-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
build: compile with C++20 support on Windows #52594
Conversation
Our Linux build infra is not ready for it yet, but V8 is making it difficult to compile on Windows without it. Refs: #45402
Review requested:
|
/cc @nodejs/build @nodejs/platform-windows |
@nodejs/tsc I'm labeling semver-major out of caution (IIRC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought Windows was the remaining blocker for #45402? I think we have gcc 10 on all the Linux machines now but I'm wary of making this kind of switch across-the-board days before the Node.js 22 release.
Approving subject to passing CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM given that Windows CI passes. I'll use this change locally until it lands 👍
@richardlau the last tentative on #45427 showed issues with Debian 12: #45427 (comment) |
OTOH you're right. This fails on Windows 🤔 |
@StefanStojanovic There are a few errors. Here's one of them:
|
If it works with V8 12.4, I can add the commit to the other PR. |
I'm seeing them too (locally added change in V8 v12.4 update branch)
Since v12.4 is the first one where I saw c++20 errors, I'll work on it with the changes from this PR. Once it is ready I'll open a PR to your branch as usual and will add this change either as part of that PR. I'll make it so there are 2 commits, one for c++20 and the other for V8, you can later decide to squash them or leave them both. Does this make sense? |
Perfect, thank you! |
Will be included in #52465 |
Our Linux build infra is not ready for it yet,
but V8 is making it difficult to compile on Windows
without it.
Refs: #45402