build: compile with C++20 support
Closes: https://github.com/nodejs/node/issues/45402
Review requested:
- [ ] @nodejs/gyp
I see a lot of [-Wambiguous-reversed-operator] in ICU. For example:
[308/3813] CXX obj/deps/icu-small/source/i18n/icui18n.pluralranges.o
In file included from ../../deps/icu-small/source/i18n/pluralranges.cpp:18:
In file included from ../../deps/icu-small/source/i18n/numrange_impl.h:15:
In file included from ../../deps/icu-small/source/i18n/number_formatimpl.h:14:
../../deps/icu-small/source/i18n/number_utils.h:53:17: warning: ISO C++20 considers use of overloaded operator '==' (with operand types 'const icu_72::MeasureUnit' and 'icu_72::MeasureUnit') to be ambiguous despite there being a unique best viable function [-Wambiguous-reversed-operator]
return unit == MeasureUnit();
~~~~ ^ ~~~~~~~~~~~~~
../../deps/icu-small/source/i18n/unicode/measunit.h:435:18: note: ambiguity is between a regular call to this operator and a call with the argument order reversed
virtual bool operator==(const UObject& other) const;
^
1 warning generated.
I think V8 built fine. There's an error in env.cc:
FAILED: obj/src/libnode.env.o
c++ -MMD -MF obj/src/libnode.env.o.d -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -D_GLIBCXX_USE_CXX11_ABI=1 -DNODE_OPENSSL_CONF_NAME=nodejs_conf -DNODE_OPENSSL_HAS_QUIC -D_DARWIN_USE_64_BIT_INODE=1 -DOPENSSL_NO_PINSHARED -DOPENSSL_THREADS '-DNODE_ARCH="arm64"' -DNODE_WANT_INTERNALS=1 -DV8_DEPRECATION_WARNINGS=1 '-DNODE_OPENSSL_SYSTEM_CERT_PATH=""' -DNODE_USE_NODE_CODE_CACHE=1 -DHAVE_INSPECTOR=1 -D__POSIX__ -DNODE_USE_V8_PLATFORM=1 -DNODE_HAVE_I18N_SUPPORT=1 '-DNODE_PLATFORM="darwin"' -DHAVE_OPENSSL=1 -DOPENSSL_API_COMPAT=0x10100000L -DBASE64_STATIC_DEFINE -DUCONFIG_NO_SERVICE=1 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION=1 -DU_HAVE_STD_STRING=1 -DUCONFIG_NO_BREAK_ITERATION=0 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DNGHTTP2_STATICLIB -DNDEBUG -DL_ENDIAN -DOPENSSL_BUILDING_OPENSSL -DECP_NISTZ256_ASM -DKECCAK1600_ASM -DOPENSSL_BN_ASM_MONT -DOPENSSL_CPUID_OBJ -DPOLY1305_ASM -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DVPAES_ASM -DOPENSSL_PIC -DNGTCP2_STATICLIB -DNGHTTP3_STATICLIB -I../../src -Igen -Igen/include -Igen/src -I../../deps/base64/base64/include -I../../deps/googletest/include -I../../deps/histogram/src -I../../deps/uvwasi/include -I../../deps/v8/include -I../../deps/icu-small/source/i18n -I../../deps/icu-small/source/common -I../../deps/zlib -I../../deps/llhttp/include -I../../deps/cares/include -I../../deps/uv/include -I../../deps/nghttp2/lib/includes -I../../deps/brotli/c/include -I../../deps/openssl/openssl/include -I../../deps/openssl/openssl/crypto/include -I../../deps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/include -I../../deps/openssl/config/archs/darwin64-arm64-cc/asm_avx2 -I../../deps/ngtcp2 -I../../deps/ngtcp2/ngtcp2/lib/includes -I../../deps/ngtcp2/ngtcp2/crypto/includes -I../../deps/ngtcp2/ngtcp2/crypto -I../../deps/ngtcp2/nghttp3/lib/includes -O3 -gdwarf-2 -mmacosx-version-min=10.15 -arch arm64 -Wall -Wendif-labels -W -Wno-unused-parameter -Werror=undefined-inline -Wall -Wendif-labels -W -Wno-unused-parameter -Werror -std=gnu++20 -stdlib=libc++ -fno-rtti -fno-exceptions -fno-strict-aliasing -c ../../src/env.cc -o obj/src/libnode.env.o
../../src/env.cc:1630:22: error: no matching constructor for initialization of 'node::DeserializeRequest'
DeserializeRequest request{cb, {isolate(), holder}, index, info};
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src/env.h:504:3: note: candidate constructor not viable: requires single argument 'other', but 4 arguments were provided
DeserializeRequest(DeserializeRequest&& other) = default;
^
../../src/env.h:497:8: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 4 were provided
struct DeserializeRequest {
^
1 error generated.
[3657/3813] CXX obj/gen/libnode.node_javascript.o
Any suggestions?
I see a lot of
[-Wambiguous-reversed-operator]in ICU.
If I understand https://unicode-org.atlassian.net/browse/ICU-22014 correctly, we need to build ICU with -Wno-ambiguous-reversed-operator.
I think V8 built fine. There's an error in
env.cc:FAILED: obj/src/libnode.env.o c++ -MMD -MF obj/src/libnode.env.o.d -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -D_GLIBCXX_USE_CXX11_ABI=1 -DNODE_OPENSSL_CONF_NAME=nodejs_conf -DNODE_OPENSSL_HAS_QUIC -D_DARWIN_USE_64_BIT_INODE=1 -DOPENSSL_NO_PINSHARED -DOPENSSL_THREADS '-DNODE_ARCH="arm64"' -DNODE_WANT_INTERNALS=1 -DV8_DEPRECATION_WARNINGS=1 '-DNODE_OPENSSL_SYSTEM_CERT_PATH=""' -DNODE_USE_NODE_CODE_CACHE=1 -DHAVE_INSPECTOR=1 -D__POSIX__ -DNODE_USE_V8_PLATFORM=1 -DNODE_HAVE_I18N_SUPPORT=1 '-DNODE_PLATFORM="darwin"' -DHAVE_OPENSSL=1 -DOPENSSL_API_COMPAT=0x10100000L -DBASE64_STATIC_DEFINE -DUCONFIG_NO_SERVICE=1 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION=1 -DU_HAVE_STD_STRING=1 -DUCONFIG_NO_BREAK_ITERATION=0 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DNGHTTP2_STATICLIB -DNDEBUG -DL_ENDIAN -DOPENSSL_BUILDING_OPENSSL -DECP_NISTZ256_ASM -DKECCAK1600_ASM -DOPENSSL_BN_ASM_MONT -DOPENSSL_CPUID_OBJ -DPOLY1305_ASM -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DVPAES_ASM -DOPENSSL_PIC -DNGTCP2_STATICLIB -DNGHTTP3_STATICLIB -I../../src -Igen -Igen/include -Igen/src -I../../deps/base64/base64/include -I../../deps/googletest/include -I../../deps/histogram/src -I../../deps/uvwasi/include -I../../deps/v8/include -I../../deps/icu-small/source/i18n -I../../deps/icu-small/source/common -I../../deps/zlib -I../../deps/llhttp/include -I../../deps/cares/include -I../../deps/uv/include -I../../deps/nghttp2/lib/includes -I../../deps/brotli/c/include -I../../deps/openssl/openssl/include -I../../deps/openssl/openssl/crypto/include -I../../deps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/include -I../../deps/openssl/config/archs/darwin64-arm64-cc/asm_avx2 -I../../deps/ngtcp2 -I../../deps/ngtcp2/ngtcp2/lib/includes -I../../deps/ngtcp2/ngtcp2/crypto/includes -I../../deps/ngtcp2/ngtcp2/crypto -I../../deps/ngtcp2/nghttp3/lib/includes -O3 -gdwarf-2 -mmacosx-version-min=10.15 -arch arm64 -Wall -Wendif-labels -W -Wno-unused-parameter -Werror=undefined-inline -Wall -Wendif-labels -W -Wno-unused-parameter -Werror -std=gnu++20 -stdlib=libc++ -fno-rtti -fno-exceptions -fno-strict-aliasing -c ../../src/env.cc -o obj/src/libnode.env.o ../../src/env.cc:1630:22: error: no matching constructor for initialization of 'node::DeserializeRequest' DeserializeRequest request{cb, {isolate(), holder}, index, info}; ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../src/env.h:504:3: note: candidate constructor not viable: requires single argument 'other', but 4 arguments were provided DeserializeRequest(DeserializeRequest&& other) = default; ^ ../../src/env.h:497:8: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 4 were provided struct DeserializeRequest { ^ 1 error generated. [3657/3813] CXX obj/gen/libnode.node_javascript.oAny suggestions?
I pushed a commit try to fix, not verified
gen/node_snapshot.cc:620:16: error: no matching constructor for initialization of 'node::SnapshotData'
};SnapshotData snapshot_data {
^ ~
../../src/env.h:574:3: note: candidate constructor not viable: requires 1 argument, but 6 were provided
SnapshotData(const SnapshotData&) = delete;
Looks similar problem to DeserializeRequest error. cc @joyeecheung
hmm, what if we use designated initializers? e.g. instead of DeserializeRequest request{cb, {isolate(), holder}, index, info}, we do DeserializeRequest request{.cb = cb, .holder = v8::Global<v8::Object>(isolate(), holder), .index = index, .info = info}?
CI: https://ci.nodejs.org/job/node-test-pull-request/47884/
GCC 8 and 9 expect -std=gnu++2a
I found the cause: https://www.open-std.org/Jtc1/sc22/wg21/docs/papers/2018/p1008r1.pdf, we need to remove all the constructors of the aggregate type, even the = default and = delete ones (which are the only ones we have). This compiles for me locally with c++20
diff --git a/src/env.h b/src/env.h
index 3d44e0acbd..8fbd48d58d 100644
--- a/src/env.h
+++ b/src/env.h
@@ -499,9 +499,6 @@ struct DeserializeRequest {
v8::Global<v8::Object> holder;
int index;
InternalFieldInfoBase* info = nullptr; // Owned by the request
-
- // Move constructor
- DeserializeRequest(DeserializeRequest&& other) = default;
};
struct EnvSerializeInfo {
@@ -565,13 +562,6 @@ struct SnapshotData {
static bool FromBlob(SnapshotData* out, FILE* in);
~SnapshotData();
-
- SnapshotData(const SnapshotData&) = delete;
- SnapshotData& operator=(const SnapshotData&) = delete;
- SnapshotData(SnapshotData&&) = delete;
- SnapshotData& operator=(SnapshotData&&) = delete;
-
- SnapshotData() = default;
};
void DefaultProcessExitHandlerInternal(Environment* env, ExitCode exit_code);
CI: https://ci.nodejs.org/job/node-test-pull-request/47888/
CI: https://ci.nodejs.org/job/node-test-pull-request/47906/
Most platforms build passed. MSVC comes no surprise, v8 failed on MSVC.
15:01:24 C:\workspace\node-compile-windows\node\deps\v8\src\handles\handles.h(143,37): error C2027: use of undefined type 'v8::internal::Object' [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
Most platforms build passed. MSVC comes no surprise, v8 failed on MSVC.
15:01:24 C:\workspace\node-compile-windows\node\deps\v8\src\handles\handles.h(143,37): error C2027: use of undefined type 'v8::internal::Object' [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
See https://bugs.chromium.org/p/chromium/issues/detail?id=1377771#c4
This is fixed in recent versions of MSVC.
Most platforms build passed. MSVC comes no surprise, v8 failed on MSVC.
15:01:24 C:\workspace\node-compile-windows\node\deps\v8\src\handles\handles.h(143,37): error C2027: use of undefined type 'v8::internal::Object' [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]See bugs.chromium.org/p/chromium/issues/detail?id=1377771#c4
This is fixed in recent versions of MSVC.
So when latest MSVC released, we only have issues on freebsd and alpine. Run into less issues than I think. But I still want gcc-10 as a minimum on next major.
CI: https://ci.nodejs.org/job/node-test-pull-request/50690/
Windows build failure on both vs2019 and vs2022:
D:\a\node\node\src\node_threadsafe_cow-inl.h(10,14): error C2039: 'unique': is not a member of 'std::shared_ptr<T>' [D:\a\node\node\libnode.vcxproj]
@nodejs/cpp-reviewers
On vs2019 only:
D:\a\node\node\deps\v8\src\handles\handles.h(145,37): error C2027: use of undefined type 'v8::internal::Object' [D:\a\node\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
I suggest to try again with a more recent V8 for this one
Interesting new MSVC error:
D:\a\node\node\deps\v8\src\compiler\turboshaft\optimization-phase.h(25): fatal error C1075: '{': no matching token found [D:\a\node\node\tools\v8_gypfiles\v8_compiler.vcxproj]
Ha, msvc gets confused by the namespace foo::bar { syntax. That's a C++17-ism.
You can fix it by breaking it up into namespace foo { namespace bar { (and make sure you fix up the closing braces too) but V8 is using that syntax in a lot of places now...
Ha, msvc gets confused by the
namespace foo::bar {syntax. That's a C++17-ism.You can fix it by breaking it up into
namespace foo { namespace bar {(and make sure you fix up the closing braces too) but V8 is using that syntax in a lot of places now...
https://godbolt.org/z/e1fPba7hf Looks like MSVC supported it, not sure it's a MSVC bug or something else.
Given to https://github.com/nodejs/node/issues/43092#issuecomment-1492920069, I don't hold much hope.
But anyway upstream bugreport: https://developercommunity.visualstudio.com/t/cpp20-nested-namespace-cant-compile/10328893
Ha, msvc gets confused by the
namespace foo::bar {syntax. That's a C++17-ism.You can fix it by breaking it up into
namespace foo { namespace bar {(and make sure you fix up the closing braces too) but V8 is using that syntax in a lot of places now...
That doesn't fix it. I think it's something inside the namespace that confuses MSVC. I opened the file in Visual Studio but it doesn't show me any error in the code editor.
~This is related to precompiled headers. If I remove v8_pch.h from the arguments passed to cl.exe, the error disappears.~ edit: I'm not sure anymore...
I was able to reduce a reproduction to one preprocessed file, but it's big (14 MiB, too big for godbolt):
PS D:\Git\nodejs\node\tools\v8_gypfiles> & 'C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\bin\HostX64\x64\CL.exe' /c /Z7 /nologo /W3 /WX- /diagnostics:column /MP /O2 /Ob2 /Oi /Oy /GF /Gm- /MT /GS /Gy /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR- /Fo"pipeline.obj" /external:W3 /Gd /TP /wd4351 /wd4355 /wd4800 /wd4251 /wd4275 /wd4244 /wd4267 /wd4129 /wd4245 /wd4324 /wd4506 /wd4661 /wd4701 /wd4702 /wd4703 /wd4709 /wd4714 /wd4715 /wd4718 /wd4723 /wd4724 /FC /errorReport:prompt /utf-8 /Zc:__cplusplus -std:c++20 pipeline.i
pipeline.i
D:\Git\nodejs\node\tools\v8_gypfiles\pipeline.i(111666,31): warning C4146: opérateur moins unaire appliqué à un type non signé, le résultat sera non signé
D:\Git\nodejs\node\tools\v8_gypfiles\pipeline.i(454888,17): warning C4996: 'std::atomic_load': warning STL4029: std::atomic_*() overloads for shared_ptr are deprecated in C++20. The shared_ptr specialization of std::atomic provides superior functionality. You can define _SILENCE_CXX20_OLD_SHARED_PTR_ATOMIC_SUPPORT_DEPRECATION_WARNING or _SILENCE_ALL_CXX20_DEPRECATION_WARNINGS to suppress this warning.
D:\Git\nodejs\node\tools\v8_gypfiles\pipeline.i(454934,28): warning C4996: 'std::atomic_load': warning STL4029: std::atomic_*() overloads for shared_ptr are deprecated in C++20. The shared_ptr specialization of std::atomic provides superior functionality. You can define _SILENCE_CXX20_OLD_SHARED_PTR_ATOMIC_SUPPORT_DEPRECATION_WARNING or _SILENCE_ALL_CXX20_DEPRECATION_WARNINGS to suppress this warning.
D:\Git\nodejs\node\tools\v8_gypfiles\pipeline.i(532263): fatal error C1075: '{' : jeton correspondant introuvable
Happy to look at anything if you have suggestions.
Another version of the file without the empty lines:
> & 'C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\bin\HostX64\x64\CL.exe' /c /Z7 /nologo /W3 /WX- /diagnostics:column /MP /O2 /Ob2 /Oi /Oy /GF /Gm- /MT /GS /Gy /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR- /Fo"pipeline.obj" /external:W3 /Gd /TP /wd4351 /wd4355 /wd4800 /wd4251 /wd4275 /wd4244 /wd4267 /wd4129 /wd4245 /wd4324 /wd4506 /wd4661 /wd4701 /wd4702 /wd4703 /wd4709 /wd4714 /wd4715 /wd4718 /wd4723 /wd4724 /FC /errorReport:prompt /utf-8 /Zc:__cplusplus -std:c++20 pipeline.i
pipeline.i
D:\Git\nodejs\node\tools\v8_gypfiles\pipeline.i(57686,31): warning C4146: opérateur moins unaire appliqué à un type non signé, le résultat sera non signé
D:\Git\nodejs\node\tools\v8_gypfiles\pipeline.i(156616,17): warning C4996: 'std::atomic_load': warning STL4029: std::atomic_*() overloads for shared_ptr are deprecated in C++20. The shared_ptr specialization of std::atomic provides superior functionality. You can define _SILENCE_CXX20_OLD_SHARED_PTR_ATOMIC_SUPPORT_DEPRECATION_WARNING or _SILENCE_ALL_CXX20_DEPRECATION_WARNINGS to suppress this warning.
D:\Git\nodejs\node\tools\v8_gypfiles\pipeline.i(156654,28): warning C4996: 'std::atomic_load': warning STL4029: std::atomic_*() overloads for shared_ptr are deprecated in C++20. The shared_ptr specialization of std::atomic provides superior functionality. You can define _SILENCE_CXX20_OLD_SHARED_PTR_ATOMIC_SUPPORT_DEPRECATION_WARNING or _SILENCE_ALL_CXX20_DEPRECATION_WARNINGS to suppress this warning.
D:\Git\nodejs\node\tools\v8_gypfiles\pipeline.i(196094): fatal error C1075: '{' : jeton correspondant introuvable
The file looks well-formed to me. 🤷
Issue should be fixed in the next minor version of VS2022: https://developercommunity.visualstudio.com/t/cpp20-nested-namespace-cant-compile/10328893#T-ND10330571
CI: https://ci.nodejs.org/job/node-test-pull-request/51306/
CI: https://ci.nodejs.org/job/node-test-pull-request/51379/
CI: https://ci.nodejs.org/job/node-test-pull-request/51663/
Issue should be fixed in the next minor version of VS2022: developercommunity.visualstudio.com/t/cpp20-nested-namespace-cant-compile/10328893#T-ND10330571
17.6 is released, but not on github actions yet.