v8: update v8 patch to avoid breaking building with clang

[zcbenz]

When building with C++20 using a very new clang (which is used by the GN build), compilation would fail caused because of the change in b9d806a.

The detailed errors can be found in:
https://github.com/photoionization/node_with_gn/actions/runs/8501717771/job/23284924676

My understanding of the error is that, after changing if constexpr to if, some code that were not supposed to be compiled are now compiled, and it is causing problems for certain compiler settings.

[nodejs-github-bot]

Review requested:

• @nodejs/security-wg
• @nodejs/v8-update

[targos]

@StefanStojanovic FYI

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/58032/

[nodejs-github-bot]

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/5900/

[nodejs-github-bot]

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/5900/

[nodejs-github-bot]

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/5900/

[targos]

@zcbenz Unfortunately this breaks compilation with MSVC.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/58055/

[nodejs-github-bot]

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/5902/

[nodejs-github-bot]

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/5902/

[nodejs-github-bot]

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/5902/

[zcbenz]

@zcbenz Unfortunately this breaks compilation with MSVC.

It turns out the V8_COMPILER_IS_MSVC macro is not defined in header. I have updated the change and it builds with MSVC now.

[gengjiawen]

Can you also try upstream those changes ?

[zcbenz]

Can you also try upstream those changes ?

This PR means to fix the breakages introduced by b9d806a, according to its commit message it is not going to be upstreamed, so neither will this change.

[StefanStojanovic]

This PR means to fix the breakages introduced by b9d806a, according to its commit message it is not going to be upstreamed, so neither will this change.

You are correct, this will not be upstreamed. @zcbenz, as a part of work on #52293 I'm porting my patch there (cannot apply cleanly because the files it modifies were changed in the meantime). Once it's ready (eg. working on all platforms), I'll let you know and we can apply your changes before landing it in the V8 update branch, so it's all there together.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/58254/

[StefanStojanovic]

@zcbenz just to let you know that neither V8, nor its patches I made starting from v12.3 have reducer_list_contains in that place, so your setup with the latest clang should work correctly out of the box as far as I can tell.

[zcbenz]

Thanks for letting me know! I'll give it a try.