Noticed while looking at the -wno-* flags in #30235.
This was disabled in #18738 due to the combo of old gcc and qt. We no longer support the affected gcc, and the old qt should no longer be relevant to us anyway.
See old fixes in: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88136 and https://bugreports.qt.io/browse/QTBUG-75210 and https://codereview.qt-project.org/c/qt/qtbase/+/245434
This was disabled in #18738 due to the combo of old gcc and qt, neither of
which are relevant to us anymore.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See 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.
Reviewers, 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.
-Wextra for Clang and GCC.
An alternative/addition would be to have one CI task compile with C++23, but that can be done after the cmake transition (remind me)
Can you clarify how this PR is related to C++23? I don’t see CMake as a blocker, if that’s something we want to do.
An alternative/addition would be to have one CI task compile with C++23, but that can be done after the cmake transition (remind me)
Can you clarify how this PR is related to C++23? I don’t see CMake as a blocker, if that’s something we want to do.
Ah sorry, looks like they didn’t change anything about the deprecation in C++23 and it is kept as-is for now.
However, I found that this patch seems incomplete. For example, no warning is printed to catch https://eel.is/c++draft/depr.impldec, unless -Wdeprecated-copy-dtor is passed. See https://godbolt.org/z/GTGbWGEnr
For reference https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2139r2.html#3.8 said:
Strong recommendation: take decisive action early in the C++23 development cycle, so early adopters can shake out the remaining cost of updating old code. Note that this would be both an API and ABI breaking change, and it would be good to set that precedent early if we wish to allow such breakage in C++23.
But that didn’t happen.
-isystem, which is equivalent to suppress_external_warnings = yes. Therefore, the block removed in this PR has never been a part of the CMake staging branch.
unless -Wdeprecated-copy-dtor is passed.
I think we can turn on additional warning flags, if you think that is valuable.
CMake includes dependencies with -isystem, which is equivalent to suppress_external_warnings = yes.
We may still need a way to disable that functionality, so all warnings can be observed, so I think this block (or atleast the functionality) is still relevant.
unless -Wdeprecated-copy-dtor is passed.
I think we can turn on additional warning flags, if you think that is valuable.
Yeah, not sure how useful that’d be, given that the deprecation apparently won’t result in a deletion (for now).
CMake includes dependencies with -isystem, which is equivalent to suppress_external_warnings = yes.
We may still need a way to disable that functionality, so all warnings can be observed, so I think this block (or atleast the functionality) is still relevant.
Yeah, could open a tracking issue in the cmake repo (hebasto/bitcoin)?
Yeah, could open a tracking issue in the cmake repo (hebasto/bitcoin)?
Opened one here so it isn’t forgotten: https://github.com/hebasto/bitcoin/issues/223.