build: Require C++20 compiler #28349

maflcko · 2023-08-27T08:56:19Z

C++20 allows to write safer code, because it allows to enforce more stuff at compile time (constinit, conteval, constexpr, std::span, ...).

Also, it allows to write less verbose and easier to understand code (C++ 20 Concepts).

See #23363 and https://en.cppreference.com/w/cpp/compiler_support#cpp20

With g++-10 (#28348) and clang-13 (#28210), there is broad support for almost all features of C++20.

It should be fine to require a C++20 compiler for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month.

This pull request includes three small cleanups to make use of C++20 features. If any issues are detected before or after merge, this should be easy to revert. If no issues arise, it should be fine to make use of more involved C++20 features later on.

DrahtBot · 2023-08-27T08:56:22Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#26593 (tracing: Only prepare tracepoint arguments when actually tracing by 0xB10C)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

maflcko · 2023-08-27T09:01:44Z

Not sure why CI fails. Does ./depends/ support C++20?

src/test/fuzz/fuzz.cpp

hebasto · 2023-08-27T12:58:34Z

Not sure why CI fails. Does ./depends/ support C++20?

--- a/depends/Makefile
+++ b/depends/Makefile
@@ -49,7 +49,7 @@ NO_HARDEN ?=
 FALLBACK_DOWNLOAD_PATH ?= https://bitcoincore.org/depends-sources
 
 C_STANDARD ?= c11
-CXX_STANDARD ?= c++17
+CXX_STANDARD ?= c++20
 
 BUILD = $(shell ./config.guess)
 HOST ?= $(BUILD)

?

hebasto · 2023-08-27T13:23:02Z

https://cirrus-ci.com/task/5662608053764096:

g++: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?

funny...

maflcko · 2023-08-27T13:31:53Z

Ok, looks like this is passing. The remaining errors should go away once and if this is rebased on the bump pull requests.

hebasto · 2023-08-27T13:36:28Z

cirrus-ci.com/task/5662608053764096:

g++: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?

funny...

It seems we should require GCC >= 10 first. See: https://gcc.gnu.org/projects/cxx-status.html.

maflcko · 2023-08-27T14:12:06Z

It seems we should require GCC >= 10 first. See: https://gcc.gnu.org/projects/cxx-status.html.

Yes, I am aware, see the pull request description and my previous comment.

ajtowns · 2023-08-27T15:00:52Z

FWIW, I get a configure.ac error with clang 14 and libstdc++ 13 -- configure: error: cannot figure out how to use std::atomic. Seems to be due to clang not correctly implementing consteval prior to clang++-15. Using clang's libc++ (apt-get install libc++abi-dev libc++-dev and -stdlib=libc++) seems like it might fix it.

$ head -n31 build-aux/m4/l_atomic.m4  | tail -n19  > testing.cpp
$ clang++ -std=c++17 -stdlib=libc++ -o testing -Wall -W testing.cpp
$ clang++ -std=c++20 -stdlib=libc++ -o testing -Wall -W testing.cpp
$ clang++ -std=c++17 -o testing -Wall -W testing.cpp
$ clang++ -std=c++20 -o testing -Wall -W testing.cpp
In file included from testing.cpp:3:
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/chrono:2320:48: error: call to consteval function 'std::chrono::hh_mm_ss::_S_fractional_width' is not a constant expression
        static constexpr unsigned fractional_width = {_S_fractional_width()};
                                                      ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/chrono:2320:48: note: undefined function '_S_fractional_width' cannot be used in a constant expression
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/chrono:2275:2: note: declared here
        _S_fractional_width()
        ^
1 error generated.
$ clang++-15 -std=c++20 -o testing -Wall -W testing.cpp
$ clang++ --version
Debian clang version 14.0.6
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

maflcko · 2023-08-27T15:12:57Z

FWIW, I get a configure.ac error with clang 14 and libstdc++ 13

Ugh, that seems like a blocker, given that clang-14 seems still to be the default for the current Debian stable (https://packages.debian.org/bookworm/clang) and even trixie (for now).

configure: error: cannot figure out how to use std::atomic

If this was the only error, I wonder if it will go away with the cmake transition, or if we can work around it, but I doubt it and expect almost any code to fail to compile.

ajtowns · 2023-08-27T16:19:45Z

If this was the only error, I wonder if it will go away with the cmake transition, or if we can work around it, but I doubt it and expect almost any code to fail to compile.

I think it's specific to atomic<chrono::duration> as specified in parts of gcc's libstdc++ headers bracketed by #if c++20... Could just decide we're not going to use atomic<duration>.

ajtowns · 2023-08-28T04:59:00Z

If this was the only error, I wonder if it will go away with the cmake transition, or if we can work around it, but I doubt it and expect almost any code to fail to compile.

I think it's specific to atomic<chrono::duration> as specified in parts of gcc's libstdc++ headers bracketed by #if c++20... Could just decide we're not going to use atomic<duration>.

Ah, it's worse than that -- clang++-14 can't deal with libstdc++'s chrono header at all in c++20 mode.

$ cat testing.cpp
#include <chrono>
int main() { return 0; }
$ clang++-14 -std=c++20 -o testing -Wall -W testing.cpp
In file included from testing.cpp:1:
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/chrono:2320:48: error: call to consteval function 'std::chrono::hh_mm_ss::_S_fractional_width' is not a constant expression
        static constexpr unsigned fractional_width = {_S_fractional_width()};

Using libc++ with CC=clang-14 CXX='env clang++-14 -stdlib=libc++' LDFLAGS='-stdlib=libc++' almost works, but Db::verify with clang's ostream isn't available for linking (using system libdb5.3++, not depends build):

/usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-bdb.o): in function `wallet::BerkeleyDatabase::Verify(bilingual_str&)':
./src/./src/wallet/bdb.cpp:327:(.text+0x3367): undefined reference to `Db::verify(char const*, char const*, std::__1::basic_ostream<char, std::__1::char_traits<char> >*, unsigned int)'

maflcko · 2023-08-28T05:57:04Z

Using libc++ with CC=clang-14 CXX='env clang++-14 -stdlib=libc++' LDFLAGS='-stdlib=libc++' almost works, but Db::verify with clang's ostream isn't available for linking (using system libdb5.3++, not depends build):

Yeah, that should be a known "issue", unrelated to C++20.

maflcko · 2023-08-28T07:25:27Z

Ah, it's worse than that -- clang++-14 can't deal with libstdc++'s chrono header at all in c++20 mode.

So to clarify, this is only a bug with clang++-14 and libstdc++-13? If so, the only operating system that ships with this config is Debian 13 Trixie, which is your operating system? If so, I'd suggest to just fix it upstream by bumping the default clang version in Debian.

ajtowns · 2023-08-28T07:49:51Z

Using libc++ with CC=clang-14 CXX='env clang++-14 -stdlib=libc++' LDFLAGS='-stdlib=libc++' almost works, but Db::verify with clang's ostream isn't available for linking (using system libdb5.3++, not depends build):

Yeah, that should be a known "issue", unrelated to C++20.

Yeah... I guess we could do C++20 with the following options on linux:

gcc 10 or above should be fine
clang 15 or above should be fine
clang 13/14 -- use libc++ and depends build of libdb++

Probably want an autoconf macro to check for the latter case?

maflcko · 2023-08-28T07:54:13Z

See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1050684

maflcko · 2023-08-28T07:58:23Z

clang 13/14 -- use libc++ and depends build of libdb++

Probably want an autoconf macro to check for the latter case?

Sgtm, but maybe check the status of the autoconf and bdb removal before spending time on this? 😅

ajtowns · 2023-08-28T12:14:54Z

Ah, it's worse than that -- clang++-14 can't deal with libstdc++'s chrono header at all in c++20 mode.

So to clarify, this is only a bug with clang++-14 and libstdc++-13? If so, the only operating system that ships with this config is Debian 13 Trixie, which is your operating system? If so, I'd suggest to just fix it upstream by bumping the default clang version in Debian.

Yes, this seems correct! Seems like a non-problem then.

fanquake · 2023-10-02T13:54:34Z

Our depends version of Qt is also going to be a blocker here. We'll need to move along to a more recent point release for improved C++20 support.

hebasto · 2023-10-02T17:24:33Z

Our depends version of Qt is also going to be a blocker here. We'll need to move along to a more recent point release for improved C++20 support.

Sorry if I missed something that was mentioned previously, but what is wrong in Qt 5.15.5 with C++20 support?

With this diff:

diff --git a/depends/Makefile b/depends/Makefile
index 3169117633..319c3498df 100644
--- a/depends/Makefile
+++ b/depends/Makefile
@@ -49,7 +49,7 @@ NO_HARDEN ?=
 FALLBACK_DOWNLOAD_PATH ?= https://bitcoincore.org/depends-sources
 
 C_STANDARD ?= c11
-CXX_STANDARD ?= c++17
+CXX_STANDARD ?= c++20
 
 BUILD = $(shell ./config.guess)
 HOST ?= $(BUILD)
diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk
index 136ce32579..ef30878e65 100644
--- a/depends/packages/qt.mk
+++ b/depends/packages/qt.mk
@@ -39,7 +39,7 @@ $(package)_config_opts_release += -silent
 $(package)_config_opts_debug = -debug
 $(package)_config_opts_debug += -optimized-tools
 $(package)_config_opts += -bindir $(build_prefix)/bin
-$(package)_config_opts += -c++std c++17
+$(package)_config_opts += -c++std c++2a
 $(package)_config_opts += -confirm-license
 $(package)_config_opts += -hostprefix $(build_prefix)
 $(package)_config_opts += -no-compile-examples

everything compiles. Did not test cross-compiling, however.

A quick look at https://github.com/qt/qtbase/compare/v5.15.5-lts-lgpl..v5.15.10-lts-lgpl shows that support for C++23 has been added. But we are talking about C++20, right?

fanquake · 2023-10-02T17:29:02Z

Last I looked there were fixes since 5.15.5 that we needed, and things didn't work.

Did not test cross-compiling, however.

What do you mean by "everything"? Probably also worth checking cross-compilation before claiming it works?

hebasto · 2023-10-02T17:31:22Z

Last I looked there were fixes since 5.15.5 that we needed, and things didn't work.

Why not just sharing known steps to reproduce "things didn't work"?

fanquake · 2023-10-02T17:33:26Z

Because I tested it as part of a different change, months ago, and don't have a trivial reproducer on hand.

fanquake · 2023-10-02T18:21:02Z

With this diff:
everything compiles.

The first thing I tested after applying your diff failed to compile. Master (fd8ab08) + #28543 + the diff above:

Build type: macx-clang (x86_64, CPU features: cx16 mmx sse sse2 sse3 ssse3 sse4.1)
Compiler: clang (Apple) 15.0.0
Configuration: sse2 aesni sse3 ssse3 sse4_1 sse4_2 avx avx2 avx512f avx512bw avx512cd avx512dq avx512er avx512ifma avx512pf avx512vbmi avx512vl f16c largefile precompile_header rdrnd rdseed shani silent x86SimdAlways release c++11 c++14 c++17 c++1z c++2a reduce_exports static stl
Build options:
  Mode ................................... release
  Optimize release build for size ........ no
  Building shared libraries .............. no
  Using C standard ....................... C11
  Using C++ standard ..................... C++2a
  Using ccache ........................... no
<snip>
compiling ../../corelib/io/qdir.cpp
compiling ../../corelib/io/qdiriterator.cpp
../../corelib/global/qrandom.cpp:386:30: error: no member named 'is_literal_type' in namespace 'std'
        Q_STATIC_ASSERT(std::is_literal_type<SystemAndGlobalGenerators>::value);
                        ~~~~~^
../../../include/QtCore/../../src/corelib/global/qglobal.h:120:57: note: expanded from macro 'Q_STATIC_ASSERT'
#  define Q_STATIC_ASSERT(Condition) static_assert(bool(Condition), #Condition)
                                                        ^~~~~~~~~
../../corelib/global/qrandom.cpp:386:46: error: 'SystemAndGlobalGenerators' does not refer to a value
        Q_STATIC_ASSERT(std::is_literal_type<SystemAndGlobalGenerators>::value);
                                             ^
../../corelib/global/qrandom.cpp:350:26: note: declared here
struct QRandomGenerator::SystemAndGlobalGenerators
                         ^
../../corelib/global/qrandom.cpp:386:74: error: no member named 'value' in the global namespace
        Q_STATIC_ASSERT(std::is_literal_type<SystemAndGlobalGenerators>::value);
                                                                       ~~^
../../../include/QtCore/../../src/corelib/global/qglobal.h:120:57: note: expanded from macro 'Q_STATIC_ASSERT'
#  define Q_STATIC_ASSERT(Condition) static_assert(bool(Condition), #Condition)
                                                        ^~~~~~~~~
compiling ../../corelib/io/qfile.cpp
3 errors generated.

Looks like this was fixed upstream in qt/qtbase@df08a21, which we don't yet have until updating. Am I missing something?

0xB10C · 2023-12-08T12:57:39Z

post-merge ACK. Tested this on my NixOS setup. Configure and build works fine, unit tests pass. Reverting in faa4838 makes sense.

I see a few `-Wmaybe-uninitialized` warnings not sure if they're related.

  CXX      policy/libbitcoin_common_a-feerate.o
In file included from ./hash.h:13,
                 from ./pubkey.h:10,
                 from ./addresstype.h:9,
                 from ./outputtype.h:9,
                 from outputtype.cpp:6:
In member function ‘bool prevector<N, T, Size, Diff>::is_direct() const [with unsigned int N = 28; T = unsigned char; Size = unsigned int; Diff = int]’,
    inlined from ‘prevector<N, T, Size, Diff>::~prevector() [with unsigned int N = 28; T = unsigned char; Size = unsigned int; Diff = int]’ at ./prevector.h:469:23,
    inlined from ‘CScript::~CScript()’ at ./script/script.h:413:7,
    inlined from ‘CNoDestination::~CNoDestination()’ at ./addresstype.h:18:7,
    inlined from ‘constexpr void std::destroy_at(_Tp*) [with _Tp = CNoDestination]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/stl_construct.h:88:18,
    inlined from ‘constexpr void std::_Destroy(_Tp*) [with _Tp = CNoDestination]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/stl_construct.h:149:22,
    inlined from ‘std::__detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)> mutable [with auto:19 = CNoDestination&]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:472:19,
    inlined from ‘constexpr _Res std::__invoke_impl(__invoke_other, _Fn&&, _Args&& ...) [with _Res = void; _Fn = __detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)>; _Args = {CNoDestination&}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/invoke.h:61:36,
    inlined from ‘constexpr std::enable_if_t<is_invocable_r_v<_Res, _Callable, _Args ...>, _Res> std::__invoke_r(_Callable&&, _Args&& ...) [with _Res = void; _Callable = __detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)>; _Args = {CNoDestination&}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/invoke.h:111:28,
    inlined from ‘static constexpr decltype(auto) std::__detail::__variant::__gen_vtable_impl<std::__detail::__variant::_Multi_array<_Result_type (*)(_Visitor, _Variants ...)>, std::integer_sequence<long unsigned int, __indices ...> >::__visit_invoke(_Visitor&&, _Variants ...) [with _Result_type = void; _Visitor = std::__detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)>&&; _Variants = {std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>&}; long unsigned int ...__indices = {0}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:1035:40,
    inlined from ‘constexpr decltype(auto) std::__do_visit(_Visitor&&, _Variants&& ...) [with _Result_type = void; _Visitor = __detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)>; _Variants = {variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>&}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:1783:5,
    inlined from ‘constexpr decltype(auto) std::__do_visit(_Visitor&&, _Variants&& ...) [with _Result_type = void; _Visitor = __detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)>; _Variants = {variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>&}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:1729:5,
    inlined from ‘constexpr void std::__detail::__variant::_Variant_storage<false, _Types ...>::_M_reset() [with _Types = {CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:470:23,
    inlined from ‘constexpr std::__detail::__variant::_Variant_storage<false, _Types ...>::~_Variant_storage() [with _Types = {CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:480:17,
    inlined from ‘constexpr std::__detail::__variant::_Copy_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::~_Copy_ctor_base()’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:554:12,
    inlined from ‘constexpr std::__detail::__variant::_Move_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::~_Move_ctor_base()’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:591:12,
    inlined from ‘constexpr std::__detail::__variant::_Copy_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::~_Copy_assign_base()’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:629:12,
    inlined from ‘constexpr std::__detail::__variant::_Move_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::~_Move_assign_base()’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:681:12,
    inlined from ‘constexpr std::__detail::__variant::_Variant_base<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::~_Variant_base()’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:735:12,
    inlined from ‘constexpr std::variant<_Types>::~variant() [with _Types = {CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:1407:28,
    inlined from ‘std::vector<std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown> > GetAllDestinationsForKey(const CPubKey&)’ at outputtype.cpp:79:5:
./prevector.h:170:37: warning: ‘*(const prevector<28, unsigned char, unsigned int, int>*)((char*)&p2sh + offsetof(std::CTxDestination, std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Variant_base<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Move_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Copy_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Move_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Copy_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_u)).prevector<28, unsigned char>::_size’ may be used uninitialized [-Wmaybe-uninitialized]
  170 |     bool is_direct() const { return _size <= N; }
      |                                     ^~~~~
outputtype.cpp: In function ‘std::vector<std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown> > GetAllDestinationsForKey(const CPubKey&)’:
outputtype.cpp:77:24: note: ‘p2sh’ declared here
   77 |         CTxDestination p2sh = ScriptHash(GetScriptForDestination(segwit));
      |                        ^~~~
In file included from /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/vector:64,
                 from ./serialize.h:25,
                 from ./hash.h:14:
In destructor ‘constexpr std::_Vector_base<_Tp, _Alloc>::~_Vector_base() [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’,
    inlined from ‘constexpr std::vector<_Tp, _Alloc>::~vector() [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/stl_vector.h:733:7,
    inlined from ‘constexpr WitnessUnknown::~WitnessUnknown()’ at ./addresstype.h:94:8,
    inlined from ‘constexpr void std::destroy_at(_Tp*) [with _Tp = WitnessUnknown]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/stl_construct.h:88:18,
    inlined from ‘constexpr void std::_Destroy(_Tp*) [with _Tp = WitnessUnknown]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/stl_construct.h:149:22,
    inlined from ‘std::__detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)> mutable [with auto:19 = WitnessUnknown&]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:472:19,
    inlined from ‘constexpr _Res std::__invoke_impl(__invoke_other, _Fn&&, _Args&& ...) [with _Res = void; _Fn = __detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)>; _Args = {WitnessUnknown&}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/invoke.h:61:36,
    inlined from ‘constexpr std::enable_if_t<is_invocable_r_v<_Res, _Callable, _Args ...>, _Res> std::__invoke_r(_Callable&&, _Args&& ...) [with _Res = void; _Callable = __detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)>; _Args = {WitnessUnknown&}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/invoke.h:111:28,
    inlined from ‘static constexpr decltype(auto) std::__detail::__variant::__gen_vtable_impl<std::__detail::__variant::_Multi_array<_Result_type (*)(_Visitor, _Variants ...)>, std::integer_sequence<long unsigned int, __indices ...> >::__visit_invoke(_Visitor&&, _Variants ...) [with _Result_type = void; _Visitor = std::__detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)>&&; _Variants = {std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>&}; long unsigned int ...__indices = {7}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:1035:40,
    inlined from ‘constexpr decltype(auto) std::__do_visit(_Visitor&&, _Variants&& ...) [with _Result_type = void; _Visitor = __detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_reset()::<lambda(auto:19&&)>; _Variants = {variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>&}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:1790:5,
    inlined from ‘constexpr void std::__detail::__variant::_Variant_storage<false, _Types ...>::_M_reset() [with _Types = {CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:470:23,
    inlined from ‘constexpr std::__detail::__variant::_Variant_storage<false, _Types ...>::~_Variant_storage() [with _Types = {CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:480:17,
    inlined from ‘constexpr std::__detail::__variant::_Copy_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::~_Copy_ctor_base()’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:554:12,
    inlined from ‘constexpr std::__detail::__variant::_Move_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::~_Move_ctor_base()’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:591:12,
    inlined from ‘constexpr std::__detail::__variant::_Copy_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::~_Copy_assign_base()’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:629:12,
    inlined from ‘constexpr std::__detail::__variant::_Move_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::~_Move_assign_base()’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:681:12,
    inlined from ‘constexpr std::__detail::__variant::_Variant_base<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::~_Variant_base()’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:735:12,
    inlined from ‘constexpr std::variant<_Types>::~variant() [with _Types = {CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown}]’ at /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/variant:1407:28,
    inlined from ‘std::vector<std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown> > GetAllDestinationsForKey(const CPubKey&)’ at outputtype.cpp:79:5:
/nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/stl_vector.h:367:31: warning: ‘*(std::_Vector_base<unsigned char, std::allocator<unsigned char> >*)((char*)&p2sh + offsetof(std::CTxDestination, std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Variant_base<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Move_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Copy_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Move_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Copy_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown>::_M_u) + 8).std::_Vector_base<unsigned char, std::allocator<unsigned char> >::_M_impl.std::_Vector_base<unsigned char, std::allocator<unsigned char> >::_Vector_impl::<anonymous>.std::_Vector_base<unsigned char, std::allocator<unsigned char> >::_Vector_impl_data::_M_end_of_storage’ may be used uninitialized [-Wmaybe-uninitialized]
  367 |                       _M_impl._M_end_of_storage - _M_impl._M_start);
      |                       ~~~~~~~~^~~~~~~~~~~~~~~~~
outputtype.cpp: In function ‘std::vector<std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, WitnessUnknown> > GetAllDestinationsForKey(const CPubKey&)’:
outputtype.cpp:77:24: note: ‘p2sh’ declared here
   77 |         CTxDestination p2sh = ScriptHash(GetScriptForDestination(segwit));
      |                        ^~~~

TheCharlatan · 2023-12-08T15:23:45Z

Post-merge ACK.

hebasto · 2023-12-08T20:53:03Z

Looks like configure doesn't detect g++-10 as a C++20 compiler...

I presume this will be fixed by cmake...

It does :)

fanquake · 2023-12-08T21:02:55Z

Isn't it just applying the exact same workaround (internally) that we are using? There's no other way to know that GCC supports C++20, because of the bug we discussed above.

hebasto · 2023-12-08T21:14:22Z

Isn't it just applying the exact same workaround (internally) that we are using? There's no other way to know that GCC supports C++20, because of the bug we discussed above.

I didn't look into CMake code for that. I only tested it, including the minimum supported CMake 3.16.

theuni · 2024-03-01T16:33:28Z

This needs a release note imo.

fanquake · 2024-03-04T11:57:48Z

Added rel-note todo to the draft wiki.

This option has ceased to exist since bitcoin#28349.

64722e4 ci: Drop `--enable-c++20` option (Hennadii Stepanov) Pull request description: This option has ceased to exist since #28349. ACKs for top commit: maflcko: ACK 64722e4 Tree-SHA512: bd392c331f775605615e1b236682269b83a1e6363a4d3f09c4d8d54495cf3d22973a921ebf6b8a9f813ba6c024d3324761f3291aaf7f473995f5eaa4c195bc43

It is unclear what the goal of this check is, given that the value may need to be set lower for the mimimum supported version of compilers that forgot to bump the value, see bitcoin/bitcoin#28349 (comment) . The minimum supported compiler versions are already documented in doc/dependencies.md and using an older compiler will already result in a compile failure, so this check can be removed as redundant. Especially given that it is only included in one file, where iwyu suggests to remove it.

fa6e50d fuzz: Use C++20 starts_with in rpc.cpp (MarcoFalke) faa4838 Revert "tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings" (MarcoFalke) fae3b77 refactor: Drop unused _Pragma to ignore -Wgnu-zero-variadic-macro-arguments (MarcoFalke) fa02fc0 refactor: modernize-use-default-member-init for bit-fields (C++20) (MarcoFalke) fa67f09 build: Require C++20 compiler (MarcoFalke) Pull request description: C++20 allows to write safer code, because it allows to enforce more stuff at compile time (`constinit`, `conteval`, `constexpr`, `std::span`, ...). Also, it allows to write less verbose and easier to understand code (C++ 20 Concepts). See bitcoin#23363 and https://en.cppreference.com/w/cpp/compiler_support#cpp20 With g++-10 (bitcoin#28348) and clang-13 (bitcoin#28210), there is broad support for almost all features of C++20. It should be fine to require a C++20 compiler for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month. This pull request includes three small cleanups to make use of C++20 features. If any issues are detected before or after merge, this should be easy to revert. If no issues arise, it should be fine to make use of more involved C++20 features later on. ACKs for top commit: fanquake: ACK fa6e50d Tree-SHA512: 244d79bfb0b750a4bdd713f40573b9ca33816fb84b6c84a58f027b9d7d4bb0cc4f18642959e4cf3d094808a69e5b8a327ca8521d7c0c08af27dacb5da3e78e71

This reverts commit 27a386e.

This option has ceased to exist since bitcoin/bitcoin#28349.

fa6e50d fuzz: Use C++20 starts_with in rpc.cpp (MarcoFalke) faa4838 Revert "tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings" (MarcoFalke) fae3b77 refactor: Drop unused _Pragma to ignore -Wgnu-zero-variadic-macro-arguments (MarcoFalke) fa02fc0 refactor: modernize-use-default-member-init for bit-fields (C++20) (MarcoFalke) fa67f09 build: Require C++20 compiler (MarcoFalke) Pull request description: C++20 allows to write safer code, because it allows to enforce more stuff at compile time (`constinit`, `conteval`, `constexpr`, `std::span`, ...). Also, it allows to write less verbose and easier to understand code (C++ 20 Concepts). See bitcoin#23363 and https://en.cppreference.com/w/cpp/compiler_support#cpp20 With g++-10 (bitcoin#28348) and clang-13 (bitcoin#28210), there is broad support for almost all features of C++20. It should be fine to require a C++20 compiler for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month. This pull request includes three small cleanups to make use of C++20 features. If any issues are detected before or after merge, this should be easy to revert. If no issues arise, it should be fine to make use of more involved C++20 features later on. ACKs for top commit: fanquake: ACK fa6e50d Tree-SHA512: 244d79bfb0b750a4bdd713f40573b9ca33816fb84b6c84a58f027b9d7d4bb0cc4f18642959e4cf3d094808a69e5b8a327ca8521d7c0c08af27dacb5da3e78e71

Reverts part of fa67f09, now that we require a minimum of GCC 11. See also: bitcoin#28349 (comment).

232928b build: no-longer allow GCC-10 in C++20 check (fanquake) Pull request description: Reverts part of fa67f09, now that we require a minimum of GCC 11. See also: #28349 (comment). ACKs for top commit: maflcko: utACK 232928b theuni: utACK 232928b Tree-SHA512: 10e0adac2dd6e455aaf97ebfe73c7586430349fc27ac435bc6c0d99a4934a380398d14467aacd9cedf371345da291366b3ab2c3be7db5d97e21ad6212b2c7890

Reverts part of fa67f096bdea9db59dd20c470c9e32f3dac5be94, now that we require a minimum of GCC 11. See also: bitcoin/bitcoin#28349 (comment).

DrahtBot changed the title ~~build: Require C++20 compiler~~ build: Require C++20 compiler Aug 27, 2023

DrahtBot added the Build system label Aug 27, 2023

maflcko commented Aug 27, 2023

View reviewed changes

src/test/fuzz/fuzz.cpp Outdated Show resolved Hide resolved

DrahtBot added the CI failed label Aug 27, 2023

maflcko added the Needs rebase label Aug 27, 2023

DrahtBot removed the Needs rebase label Aug 27, 2023

fanquake added this to the 27.0 milestone Sep 26, 2023

hebasto mentioned this pull request Dec 8, 2023

cmake: Rebase and backport C++20 changes hebasto/bitcoin#60

Closed

fanquake mentioned this pull request Dec 12, 2023

Discussion: Upgrading to C++20 #23363

Closed

MPins mentioned this pull request Jan 13, 2024

Topicos Janeiro saopaulobitdevs/saopaulobitdevs.org#17

Closed

fanquake added Needs release note and removed Needs release note labels Mar 1, 2024

hebasto added a commit to hebasto/bitcoin that referenced this pull request Mar 17, 2024

ci: Drop --enable-c++20 option

64722e4

This option has ceased to exist since bitcoin#28349.

hebasto mentioned this pull request Mar 17, 2024

ci: Drop --enable-c++20 option #29669

Merged

PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Apr 4, 2024

Revert "Merge bitcoin#28349: build: Require C++20 compiler"

c97df1e

This reverts commit 27a386e.

janus pushed a commit to BitgesellOfficial/bitgesell that referenced this pull request Apr 6, 2024

ci: Drop --enable-c++20 option

cc9b017

This option has ceased to exist since bitcoin/bitcoin#28349.

ajtowns mentioned this pull request May 26, 2024

doc: note that you can assume C++20. #30136

Closed

fanquake added a commit to fanquake/bitcoin that referenced this pull request Jun 5, 2024

build: no-longer allow GCC-10 in C++20 check

232928b

Reverts part of fa67f09, now that we require a minimum of GCC 11. See also: bitcoin#28349 (comment).

fanquake mentioned this pull request Jun 5, 2024

build: no-longer allow GCC-10 in C++20 check #30228

Merged

hebasto mentioned this pull request Sep 17, 2024

GUI event loop should be block free #17145

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Require C++20 compiler #28349

build: Require C++20 compiler #28349

maflcko commented Aug 27, 2023

DrahtBot commented Aug 27, 2023 •

edited

Loading

maflcko commented Aug 27, 2023

hebasto commented Aug 27, 2023

hebasto commented Aug 27, 2023

maflcko commented Aug 27, 2023

hebasto commented Aug 27, 2023 •

edited

Loading

maflcko commented Aug 27, 2023

ajtowns commented Aug 27, 2023

maflcko commented Aug 27, 2023

ajtowns commented Aug 27, 2023

ajtowns commented Aug 28, 2023

maflcko commented Aug 28, 2023

maflcko commented Aug 28, 2023

ajtowns commented Aug 28, 2023

maflcko commented Aug 28, 2023

maflcko commented Aug 28, 2023

ajtowns commented Aug 28, 2023

fanquake commented Oct 2, 2023

hebasto commented Oct 2, 2023

fanquake commented Oct 2, 2023

hebasto commented Oct 2, 2023

fanquake commented Oct 2, 2023

fanquake commented Oct 2, 2023

0xB10C commented Dec 8, 2023

TheCharlatan commented Dec 8, 2023

hebasto commented Dec 8, 2023

fanquake commented Dec 8, 2023

hebasto commented Dec 8, 2023

theuni commented Mar 1, 2024

fanquake commented Mar 4, 2024

build: Require C++20 compiler #28349

build: Require C++20 compiler #28349

Conversation

maflcko commented Aug 27, 2023

DrahtBot commented Aug 27, 2023 • edited Loading

Code Coverage

Reviews

Conflicts

maflcko commented Aug 27, 2023

hebasto commented Aug 27, 2023

hebasto commented Aug 27, 2023

maflcko commented Aug 27, 2023

hebasto commented Aug 27, 2023 • edited Loading

maflcko commented Aug 27, 2023

ajtowns commented Aug 27, 2023

maflcko commented Aug 27, 2023

ajtowns commented Aug 27, 2023

ajtowns commented Aug 28, 2023

maflcko commented Aug 28, 2023

maflcko commented Aug 28, 2023

ajtowns commented Aug 28, 2023

maflcko commented Aug 28, 2023

maflcko commented Aug 28, 2023

ajtowns commented Aug 28, 2023

fanquake commented Oct 2, 2023

hebasto commented Oct 2, 2023

fanquake commented Oct 2, 2023

hebasto commented Oct 2, 2023

fanquake commented Oct 2, 2023

fanquake commented Oct 2, 2023

0xB10C commented Dec 8, 2023

TheCharlatan commented Dec 8, 2023

hebasto commented Dec 8, 2023

fanquake commented Dec 8, 2023

hebasto commented Dec 8, 2023

theuni commented Mar 1, 2024

fanquake commented Mar 4, 2024

DrahtBot commented Aug 27, 2023 •

edited

Loading

hebasto commented Aug 27, 2023 •

edited

Loading