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: Require libc++-16 or later #29077
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
Draft for now to gather conceptual feedback first. |
bfb7ccd
to
faaf8b7
Compare
doc/dependencies.md
Outdated
@@ -8,7 +8,7 @@ You can find installation instructions in the `build-*.md` file for your platfor | |||
| --- | --- | | |||
| [Autoconf](https://www.gnu.org/software/autoconf/) | [2.69](https://github.com/bitcoin/bitcoin/pull/17769) | | |||
| [Automake](https://www.gnu.org/software/automake/) | [1.13](https://github.com/bitcoin/bitcoin/pull/18290) | | |||
| [Clang](https://clang.llvm.org) | [13.0](https://github.com/bitcoin/bitcoin/pull/28210) | | |||
| [Clang](https://clang.llvm.org) | [13.0](https://github.com/bitcoin/bitcoin/pull/28210) (with at least libstdc++-10, or at least [libc++ 16.0](https://github.com/bitcoin/bitcoin/pull/29077))| |
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.
Given #28687 (comment) and https://godbolt.org/z/n4oTE9sTE the libstdc++-10
requirement isn't going to work here? Looks like this really needs to be clang-16.
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.
So as long as you don't need deferred concepts instantiation, it should be fine.
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.
More missing in v10: #29081 (comment)
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.
Thanks, clarified that "libstdc++ (any version) requires at least clang-16"
So won't this break as soon as we have code that requires libstdc++-16, given that OSS-Fuzz is still pinned to 15? Are there no other projects using (some?) C++20 code on OSS-Fuzz? |
faaf8b7
to
79796a4
Compare
79796a4
to
033ee67
Compare
033ee67
to
8b3bdca
Compare
Not sure if there is much value in being overly accurate here ( So I'd say to close this and bump to 16 wholesale? |
I think generally, that would be more straightforward, then supporting varying compiler + std lib combinations, and seems possible to do at this point. |
fa90ad2 ci: Roll test-each-commit Ubuntu (MarcoFalke) fa6c82d ci: Remove clang version pin in test-each-commit (MarcoFalke) Pull request description: Needed for #29077 (comment) ACKs for top commit: hebasto: re-ACK fa90ad2. Tree-SHA512: 753a3a77d967f308b5b5dd0bc7ea9f3268fc93c5ac978da3d79b85461cb1e994c6ac481888dc472b9a08be45ad0fad66ad3fda241a8955f999b7c2cb2a2b1f58
I am not familiar with macOS, but given that #28622 prescribes a minimum of libc++-16 for macOS, doing the same on Linux could make sense?
This is needed for stuff:
std::ranges::equal
, see refactor: Remove Span operator==, Use std::ranges::equal #29071std::source_location
, see build: Enable -Wunreachable-code #28999 (comment)I don't think anyone is using
-stdlib=libc++
on Linux, except for the CI and OSS-Fuzz?The minimum required clang version remains at
clang++-15
.