Skip to content
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

doc: add C++17 release note for 0.21.0 #19305

Merged
merged 1 commit into from Jun 22, 2020
Merged

Conversation

fanquake
Copy link
Member

TLDR: Mention that the codebase is now compatible with C++17, and that the
intention is to require C++17 starting with 0.22.0.

Following some discussion with Cory/Carl, and in #16684, I think this is the next step in the C++17 migration.

While #16684 mentions a gitian/Guix release with C++17, it's not yet clear how that would be done. Are we just going to pass --enable-c++17 in gitian/Guix?. Are we changing our default in configure.ac?

According to the last comment in #16684, we wouldn't be changing anything in depends:

No, everything (including depends) will stay at C++11.

However I don't think we want to be mixing C++11 built dependencies, with a C++17 built bitcoind, if there is any potential for compatibility issues.

Instead, I'd suggest we build the 0.21.0 release as C++11, and do a complete switch to C++17 for 0.22.0. Also, if we actually wanted to use C++17 in depends for 0.21.0, we couldn't without breaking C++11 compat (Qt). See below.

Here is a potential timeline/TODOs for the migration:

Potential Timeline

One thing worth noting, is that we cannot bump our Qt to a newer LTS for 0.21.0, without breaking C++11 compatibility. Qt 5.12 is not compilable in C++11 mode, as the project has started using C++14 features throughout at least the macOS portions of it's codebase, and seemingly "forgotten" that the release is meant to be C++11 compatible.
Upstream bug here: https://bugreports.qt.io/browse/QTBUG-77310.

Building Qt requires C+11, at a minimum, but in practice we use later features, usually under a feature define, or with a fallback of some kind. On platforms that support > C11, we've (apparently) not considered the fallback necessary, under the assumption C+14 is always available.

Mention that the codebase is now compatible with C++17, and that the
intention is to require C++17 starting with 0.22.0.
@fanquake fanquake added the Docs label Jun 17, 2020
@fanquake fanquake added this to the 0.21.0 milestone Jun 17, 2020
@maflcko
Copy link
Member

maflcko commented Jun 17, 2020

ACK f1d21ef can't hurt to give an advance warning

@maflcko
Copy link
Member

maflcko commented Jun 17, 2020

Some time prior to split-off:

Confirm that compiling with C++17 works.
Confirm that C++11 compatibility has not been broken.

our current ci configs should take care of this already

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK f1d21ef

@Sjors
Copy link
Member

Sjors commented Jun 19, 2020

ACK f1d21ef

One thing worth noting, is that we cannot bump our Qt to a newer LTS for 0.21.0, without breaking C++11 compatibility.

Perusing all open GUI issues old repo / new repo I don't see any burning issues that need a version bump. In other words, happy to wait for 0.22. Regardless, I look forward to the 0.21.0 branch-off.

Bump Qt from 5.9.x (no c++17 mode) to, likely, 5.15.x (LTS)

Unfortunately QT retroactively dropped LTS for 5.15 (see #18580), i.e. it's commercial support only. But as @MarcoFalke pointed out:

I don't think we have a choice. Now that they dropped LTS, all we can do is follow their latest open source release

It's certainly doesn't seem like a reason to stick to a c++11 version of QT, since those aren't maintained at all, making it mostly an orthogonal concern.

@JeremyRubin
Copy link
Contributor

This seems consistent with #16684 (comment)

I would add a note that 0.21 backports and minor versions may begin requiring C++17 as well, depending on what they are (e.g., if consensus code gets added using a C++17 feature, the soft fork rules would require C++17). As such, it's reasonable to say that 0.21.0 is the last version a user should expect to be compatible with C++11.

@laanwj
Copy link
Member

laanwj commented Jun 22, 2020

ACK f1d21ef

@laanwj laanwj merged commit e3fa3c7 into bitcoin:master Jun 22, 2020
@fanquake fanquake deleted the c++17_0_21_0 branch June 23, 2020 07:52
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants