-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Compiler error with latest canary #277
Comments
Here's a test on my fork to see if the GCC version makes a difference: https://github.com/targos/node-v8/actions/runs/7275899259 |
Well, they all failed with the same error! Now I suspect it might be change that requires C++20 support. |
Again with nodejs/node#45427 applied on top: https://github.com/targos/node-v8/actions/runs/7277425679 |
Same error with C++20 😞 |
/cc @LeszekSwirski this seems to have been introduced by https://chromium-review.googlesource.com/c/v8/v8/+/5077926. Maybe you have an idea for why the V8 GCC buildbot didn't fail with that error? |
Interesting, looks like this was a change made in gcc 11 (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95942) -- there seems to be some disagreement/lack of clarity in the C/C++ specs on whether an array member element access is a "member designator" or not, and looks like other compilers (and gcc up to 10) relaxed the spec requirement that the result is an integral constant. The V8 GCC buildbot runs with gcc 9, so it won't have picked this up. I guess we have to rewrite this the old fashioned way -- shame, the offsetof with runtime index is much neater. |
Fixing this in https://chromium-review.googlesource.com/c/v8/v8/+/5143447 |
Thank you! |
That fix landed, can you try patching it in and trying again? |
Build passed! |
https://github.com/nodejs/node-v8/actions/runs/7271603973/job/19812421974
The text was updated successfully, but these errors were encountered: