nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.6k stars 29.07k forks source link

V8 build error with 22.7.0 #54576

Open bnoordhuis opened 3 weeks ago

bnoordhuis commented 3 weeks ago

Like https://github.com/nodejs/node/issues/53633 which was for gcc 12 and fixed in commit f4a7ac5e1842c0f4629a0bebfda38f2502a2ee41 but with clang 15.0.7 on x86_64 linux I get the exact same build error:

../deps/v8/src/base/small-vector.h:25:3: error: static assertion failed due to requirement '::v8::base::is_trivially_copyable<std::pair<const v8::internal::com
piler::turboshaft::PhiOp *, const v8::internal::compiler::turboshaft::OpIndex>>::value': T should be trivially copyable                                        
  ASSERT_TRIVIALLY_COPYABLE(T);                                                                                                                                
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                                                 
../deps/v8/src/base/macros.h:211:3: note: expanded from macro 'ASSERT_TRIVIALLY_COPYABLE'                                                                      
  static_assert(::v8::base::is_trivially_copyable<T>::value, \                                                                                                 
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                    
../deps/v8/src/compiler/turboshaft/loop-unrolling-reducer.h:431:65: note: in instantiation of template class 'v8::base::SmallVector<std::pair<const v8::interna
l::compiler::turboshaft::PhiOp *, const v8::internal::compiler::turboshaft::OpIndex>, 16>' requested here                                                      
  base::SmallVector<std::pair<const PhiOp*, const OpIndex>, 16> phis;

Maybe just remove that ASSERT_TRIVIALLY_COPYABLE? Upstream already tests for correctness and to us it's just a recurring source of build breakage.

joyeecheung commented 3 weeks ago

Another option would be to pile the affected clang versions onto the ifdef mixture. According to the minimal repro this seems specific to clang 15 (doesn't reproduce on neither clang 14.0.0 nor 16.0.0)

joyeecheung commented 3 weeks ago

Actually I remember we are using clang 15 for macOS in the canary CI? @targos

targos commented 3 weeks ago

There's nothing special about the canary CI. It's using the same machines and config as main

targos commented 3 weeks ago

According to https://en.wikipedia.org/wiki/Xcode#Xcode_11.0_-_14.x_(since_SwiftUI_framework)_2, clang 15 wasn't included for a long time in Xcode (it's the LLVM column):

CleanShot 2024-08-27 at 16 22 45

bnoordhuis commented 2 hours ago

I've seen this bug with both clang 14 and 15. What do our buildbots use? 16?

bnoordhuis commented 2 hours ago

Forgot to mention, I have a patch ready for upstreaming but I'd like to narrow down the range of broken clangs.

targos commented 1 hour ago

In Jenkins, the buildbots use Clang 12 In GitHub actions, it seems to be Clang 15.

(on macOS)

targos commented 1 hour ago

on Linux, I think we only use Clang in GitHub actions, and it's on version 18.