build: Add --enable-c++20 option #24169

maflcko · 2022-01-26T19:04:13Z

This is for CI and devs only and doesn't change that C++17 is the standard we are currently using. The option --enable-c++20 allows CI to check that the C++17 code in the repo is also valid C++20. (There are some cases where valid C++17 doesn't compile under C++20).

Also, it allows developers to easily play with C++20 in the codebase.

PastaPastaPasta · 2022-01-27T08:28:18Z

You may or may not be interested in one of my PRs to dash that did this dashpay#4600

I was planning on doing the equivalent here, but didn't want to until we could merge #23340 which if I recall correctly is required to fix cxx20 builds

jonatack · 2022-01-27T16:34:21Z

Concept ACK

fanquake · 2022-02-11T10:02:57Z

Support for c++20 has now been merged into the ax_cxx_compile_stdcxx.m4 macro upstream: autoconf-archive/autoconf-archive#244.

DrahtBot · 2022-02-15T08:01:41Z

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

Conflicts

No conflicts as of last run.

fanquake · 2022-02-23T11:49:56Z

If you want to split the macro update off, happy to merge that earlier, given it's a mechanical change.

maflcko · 2022-03-17T20:56:28Z

Thx, done. (No code changes)

ryanofsky

Code review ACK fab63ca. Thanks for the descriptions, I understand this much better now!

fanquake · 2022-03-18T10:54:35Z

@theuni you might also be interested here

theStack

Concept and code-review ACK fab63ca 2️⃣ 0️⃣

Tested --enable-c++20 compilation with clang 11.1.0 (OpenBSD 7.0).

hebasto · 2022-03-21T07:22:09Z

Concept ACK.

hebasto

Approach ACK fab63ca, except for the fa08f8b "Set -Wno-deprecated-enum-enum-conversion" commit. An alternative has been suggested in #24624.

maflcko · 2022-03-22T08:10:03Z

I'd say the two pull requests are mostly orthogonal. If one is merged before the other, the one merged second can drop the temporary -Wno commit.

hebasto

ACK fab63ca

I'd say the two pull requests are mostly orthogonal. If one is merged before the other, the one merged second can drop the temporary -Wno commit.

Agree.

…um-conversion warnings acd98ad qt: Avoid potential -Wdeprecated-enum-enum-conversion warning (Hennadii Stepanov) d8641f0 qt: Use human-readable strings in preference to hard-coded integers (Hennadii Stepanov) Pull request description: This PR is related to bitcoin/bitcoin#24169. It adjusts code in order to avoid `-Wdeprecated-enum-enum-conversion` warnings instead of disabling them. Could be tested with gcc 11.2. ACKs for top commit: MarcoFalke: Approach ACK acd98ad fanquake: untested ACK acd98ad - thanks. promag: Code review ACK acd98ad. Tree-SHA512: e8043d997d85f8dba0f37ca02f1c60eb756a1732cf76a75908b01eb2cf7a4c6d4aaf6007271a929c213de37a0c1d96bc25280f0ee9eca488f370904461222ede

maflcko · 2022-03-22T12:44:46Z

Removed a commit. Should be trivial to re-ACK.

hebasto

~~re-ACK fa6bc3b~~

CI failure?

maflcko · 2022-03-22T15:39:20Z

~~Please review #24641 first, while this fails to compile.~~

Without the changes, g++ will warn to compile under C++20: scheduler.cpp:114:21: warning: implicit capture of ‘this’ via ‘[=]’ is deprecated in C++20 [-Wdeprecated] 114 | scheduleFromNow([=] { Repeat(*this, f, delta); }, delta); | ^ scheduler.cpp:114:21: note: add explicit ‘this’ or ‘*this’ capture

Without the changes, the file will fail to compile under C++20 because char8_t can not be converted to char implicitly.

This makes code that uses the helper less verbose. Moreover, this makes net_processing C++20 compliant. Otherwise, it would lead to a compile error (see below). C++20 disables aggregate initialization when any constructor is declared. See http://open-std.org/JTC1/SC22/WG21/docs/papers/2018/p1008r1.pdf net_processing.cpp:1627:42: error: no matching constructor for initialization of 'CSerializedNetMsg' m_connman.PushMessage(pnode, CSerializedNetMsg{ser_cmpctblock.data, ser_cmpctblock.m_type}); ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

ryanofsky

Code review ACK 999982b. Since last review was rebased, and enum-conversion change was dropped, and CSerializedNetMsg copy workaround was added

fanquake

utACK 999982b

Sjors · 2022-03-26T09:26:08Z

Nice! Though macOS trips over a deprecation (warning), see #24682.

maflcko added the Build system label Jan 26, 2022

maflcko added this to the Future milestone Jan 26, 2022

luke-jr mentioned this pull request Jan 31, 2022

Autotools dependency version undocumented #24209

Closed

maflcko force-pushed the 2201-ciCpp20 branch from ee0ff0f to fabb931 Compare February 11, 2022 11:22

maflcko changed the title ~~[WIP DRAFT] build: Add --enable-c++20 option~~ build: Add --enable-c++20 option Feb 11, 2022

maflcko marked this pull request as ready for review February 11, 2022 11:22

maflcko removed this from the Future milestone Feb 11, 2022

maflcko marked this pull request as draft February 11, 2022 12:01

DrahtBot mentioned this pull request Feb 15, 2022

util: Revert back MoveFileExW call for MinGW-w64 #24331

Merged

DrahtBot mentioned this pull request Feb 17, 2022

util: Work around libstdc++ create_directories issue #24338

Merged

DrahtBot added the Needs rebase label Feb 17, 2022

maflcko force-pushed the 2201-ciCpp20 branch 3 times, most recently from 4433cf5 to 8293846 Compare February 17, 2022 16:32

maflcko mentioned this pull request Feb 17, 2022

bench: Avoid deprecated use of volatile += #24372

Merged

DrahtBot removed the Needs rebase label Feb 17, 2022

maflcko force-pushed the 2201-ciCpp20 branch 3 times, most recently from b4c9519 to 50c40d2 Compare February 21, 2022 16:17

This was referenced Feb 21, 2022

test: Fix Wambiguous-reversed-operator compiler warnings #24406

Merged

refactor: Remove confusing P1008R1 violation in ATMPArgs #24404

Merged

maflcko force-pushed the 2201-ciCpp20 branch 2 times, most recently from 7f3fb74 to 4f64dd4 Compare February 23, 2022 11:32

maflcko force-pushed the 2201-ciCpp20 branch from 4f64dd4 to 60fbb9c Compare March 2, 2022 12:08

maflcko mentioned this pull request Mar 2, 2022

build: update ax_cxx_compile_stdcxx to serial 14 #24460

Merged

ryanofsky approved these changes Mar 17, 2022

View reviewed changes

theStack approved these changes Mar 19, 2022

View reviewed changes

hebasto mentioned this pull request Mar 21, 2022

qt: Avoid potential -Wdeprecated-enum-enum-conversion warnings #24624

Merged

hebasto reviewed Mar 21, 2022

View reviewed changes

hebasto approved these changes Mar 22, 2022

View reviewed changes

maflcko force-pushed the 2201-ciCpp20 branch from fab63ca to fa6bc3b Compare March 22, 2022 12:44

hebasto approved these changes Mar 22, 2022

View reviewed changes

maflcko marked this pull request as draft March 22, 2022 15:40

MarcoFalke added 2 commits March 24, 2022 11:36

Make fs.h C++20 compliant

fabb7c4

Without the changes, the file will fail to compile under C++20 because char8_t can not be converted to char implicitly.

maflcko marked this pull request as ready for review March 24, 2022 10:37

MarcoFalke added 2 commits March 24, 2022 11:37

build: Add --enable-c++20 option

999982b

maflcko force-pushed the 2201-ciCpp20 branch from fa6bc3b to 999982b Compare March 24, 2022 10:38

ryanofsky approved these changes Mar 24, 2022

View reviewed changes

maflcko mentioned this pull request Mar 24, 2022

Re-enable C++20 aggregate initialization for CSerializedNetMsg #24641

Closed

fanquake approved these changes Mar 24, 2022

View reviewed changes

fanquake merged commit 213e98c into bitcoin:master Mar 24, 2022

maflcko deleted the 2201-ciCpp20 branch March 24, 2022 13:28

Sjors mentioned this pull request Mar 26, 2022

u8path<std::string> deprecation with libc++ and c++20 #24682

Closed

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 2, 2022

Merge bitcoin#24169: build: Add --enable-c++20 option

aad46d4

bitcoin locked and limited conversation to collaborators Mar 26, 2023

build: Add --enable-c++20 option #24169

build: Add --enable-c++20 option #24169

Uh oh!

Conversation

maflcko commented Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PastaPastaPasta commented Jan 27, 2022

Uh oh!

jonatack commented Jan 27, 2022

Uh oh!

fanquake commented Feb 11, 2022

Uh oh!

DrahtBot commented Feb 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

fanquake commented Feb 23, 2022

Uh oh!

maflcko commented Mar 17, 2022

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

fanquake commented Mar 18, 2022

Uh oh!

theStack left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto commented Mar 21, 2022

Uh oh!

hebasto left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented Mar 22, 2022

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko commented Mar 22, 2022

Uh oh!

hebasto left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented Mar 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

Sjors commented Mar 26, 2022

Uh oh!

Uh oh!

maflcko commented Jan 26, 2022 •

edited

Loading

DrahtBot commented Feb 15, 2022 •

edited

Loading

hebasto left a comment •

edited

Loading

hebasto left a comment •

edited

Loading

maflcko commented Mar 22, 2022 •

edited

Loading