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
serialization: replace char-is-int8_t autoconf detection with c++20 concept #29484
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.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. 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. |
Ping @maflcko for the concepts question. |
Concept ACK |
review ACK 064c6d3 |
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.
ACK 064c6d3
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.
Code review ACK 064c6d3
…oncept This removes the only remaining autoconf macro in our serialization code, so it can now be used trivially and safely out-of-tree.
064c6d3
to
ad7584d
Compare
Code review ACK ad7584d |
dc1a121 fixup! cmake: Check compiler features (Hennadii Stepanov) Pull request description: This PR backports build system changes from bitcoin#29484. ACKs for top commit: pablomartin4btc: ACK dc1a121 Tree-SHA512: e6005f96a9632fb4787d2dcfc558aa484be6401804ffcdfe68e6d17e39576d7fc55cbf17f0c7097f32c4639201f6b26c7098bb9a1cce801fee6bd77047cfa82d
Doesn't depend on #29263, but it's really only relevant after that one's merged.
This removes the only remaining autoconf macro in our serialization code (after #29263), so it can now be used trivially and safely out-of-tree.
Our code does not currently contain any concepts, but couldn't find any discussion or docs about avoiding them. I guess we'll see if this blows up our c-i.Edit: Ignore this. ajtowns pointed out that we're already using a few concepts.
This was introduced in #13580. Please check my logic on this as I'm unable to test on a SmartOS system. Even better would be a confirmation from someone who can build there.