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

refactor: Add missing includes to fix gcc-13 compile error #26924

Merged
merged 1 commit into from
Jan 20, 2023

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jan 19, 2023

On current master:

  CXX      support/libbitcoin_util_a-lockedpool.o
support/lockedpool.cpp: In member function ‘void Arena::free(void*)’:
support/lockedpool.cpp:99:20: error: ‘runtime_error’ is not a member of ‘std’
   99 |         throw std::runtime_error("Arena: invalid or double free");
      |                    ^~~~~~~~~~~~~
support/lockedpool.cpp:22:1: note: ‘std::runtime_error’ is defined in header ‘<stdexcept>’; did you forget to ‘#include <stdexcept>’?
   21 | #include <algorithm>
  +++ |+#include <stdexcept>
   22 | #ifdef ARENA_DEBUG
support/lockedpool.cpp: In member function ‘void LockedPool::free(void*)’:
support/lockedpool.cpp:320:16: error: ‘runtime_error’ is not a member of ‘std’
  320 |     throw std::runtime_error("LockedPool: invalid address not pointing to any arena");
      |                ^~~~~~~~~~~~~
support/lockedpool.cpp:320:16: note: ‘std::runtime_error’ is defined in header ‘<stdexcept>’; did you forget to ‘#include <stdexcept>’?

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 19, 2023

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK hebasto, fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK fadeb6b.

While touching headers, the unused #include <config/bitcoin-config.h> can be removed as well.

And IWYU suggests to drop #include <limits.h>. But there could be some platform-specific pitfalls. FWIW, on my machine, the limits.h does not contain #define PAGESIZE.

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK fadeb6b - tested this fixes compilation with GCC 13. I don't think theres a need to do anything else here, and that'd also just potentially complicate backporting.

@fanquake fanquake merged commit 392dc68 into bitcoin:master Jan 20, 2023
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 20, 2023
@fanquake fanquake mentioned this pull request Jan 20, 2023
@fanquake
Copy link
Member

Added to #26878 for backporting to 24.x.

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 20, 2023
@fanquake fanquake mentioned this pull request Jan 20, 2023
@fanquake
Copy link
Member

Added to #26921 for 23.x.

fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jan 20, 2023
@fanquake fanquake mentioned this pull request Jan 20, 2023
@fanquake
Copy link
Member

Opened #26927 for 22.x.

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 20, 2023
…pile error

fadeb6b Add missing includes to fix gcc-13 compile error (MarcoFalke)

Pull request description:

  On current master:

  ```
    CXX      support/libbitcoin_util_a-lockedpool.o
  support/lockedpool.cpp: In member function ‘void Arena::free(void*)’:
  support/lockedpool.cpp:99:20: error: ‘runtime_error’ is not a member of ‘std’
     99 |         throw std::runtime_error("Arena: invalid or double free");
        |                    ^~~~~~~~~~~~~
  support/lockedpool.cpp:22:1: note: ‘std::runtime_error’ is defined in header ‘<stdexcept>’; did you forget to ‘#include <stdexcept>’?
     21 | #include <algorithm>
    +++ |+#include <stdexcept>
     22 | #ifdef ARENA_DEBUG
  support/lockedpool.cpp: In member function ‘void LockedPool::free(void*)’:
  support/lockedpool.cpp:320:16: error: ‘runtime_error’ is not a member of ‘std’
    320 |     throw std::runtime_error("LockedPool: invalid address not pointing to any arena");
        |                ^~~~~~~~~~~~~
  support/lockedpool.cpp:320:16: note: ‘std::runtime_error’ is defined in header ‘<stdexcept>’; did you forget to ‘#include <stdexcept>’?

ACKs for top commit:
  hebasto:
    ACK fadeb6b.
  fanquake:
    ACK fadeb6b - tested this fixes compilation with GCC 13. I don't think theres a need to do anything else here, and that'd also just potentially complicate backporting.

Tree-SHA512: 99f79cf385c913138a9cf9fc23be0a3a067b0a28518b8bdc033a7220b85bbc5d18f5356c5bdad2f628c22abb87c18b232724f606eba6326c031518559054be31
@maflcko
Copy link
Member Author

maflcko commented Jan 25, 2023

While touching headers, the unused #include <config/bitcoin-config.h> can be removed as well.

sgtm.

However, I wonder if we should require for preprocessor stuff that is used in #ifdef or #if to be mentioned in after the include. Otherwise, removing an include can result in a passing but broken build.

For example:

#include <config/bitcoin-config.h> // for FEATURE_FOO

@maflcko maflcko deleted the 2301-iwyu-gcc-13-📔 branch January 25, 2023 11:14
fanquake added a commit that referenced this pull request Feb 16, 2023
ea584a6 23.x Add missing includes to fix gcc-13 compile error (fanquake)
c21e6a9 Add missing includes to fix gcc-13 compile error (MarcoFalke)

Pull request description:

  Backports:
  * #26924

ACKs for top commit:
  instagibbs:
    ACK ea584a6

Tree-SHA512: e231d2e3e17f884eb9eb63f2f31829cef3e64351c9d6b90abeef421c366cef228a0a87b9c3a07c840879bd514ebe33b554fb62947c13bd05806865e59323cae7
fanquake added a commit that referenced this pull request Feb 16, 2023
52376d9 depends: fix systemtap download URL (fanquake)
af86266 23.x Add missing includes to fix gcc-13 compile error (fanquake)
3987687 Add missing includes to fix gcc-13 compile error (MarcoFalke)
412cd1a addrdb: Only call Serialize() once (Martin Zumsande)
fd94bef hash: add HashedSourceWriter (Martin Zumsande)

Pull request description:

  Backports:
  * #26909
  * #26924
  * #26944

ACKs for top commit:
  instagibbs:
    ACK 52376d9

Tree-SHA512: fa6463d5086667107b4ce4d87545e0b3f9b7841a52761a4dc6286377f958ecc026ed6694d1cf1e91cbad686309b5d637608f3991c46a20b02421318a804ffcea
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Feb 22, 2023
glozow added a commit that referenced this pull request Feb 27, 2023
784a754 wallet, rpc: Update migratewallet help text for encrypted wallets (Andrew Chow)
debcfe3 tests: Tests for migrating wallets by name, and providing passphrase (Andrew Chow)
ccc72fe wallet: Be able to unlock the wallet for migration (Andrew Chow)
50dd8b1 rpc: Allow users to specify wallet name for migratewallet (Andrew Chow)
648b062 wallet: Allow MigrateLegacyToDescriptor to take a wallet name (Andrew Chow)
ab3bd45 i2p: use consistent number of tunnels with i2pd and Java I2P (Vasil Dimov)
29cdf42 i2p: lower the number of tunnels for transient sessions (Vasil Dimov)
5027e93 i2p: reuse created I2P sessions if not used (Vasil Dimov)
a62c541 wallet: reuse change dest when recreating TX with avoidpartialspends (Matthew Zipkin)
64e7db6 Zero out wallet master key upon lock (John Moffett)
b7e242e Correctly limit overview transaction list (John Moffett)
cff6718 depends: fix systemtap download URL (fanquake)
7cf73df Add missing includes to fix gcc-13 compile error (MarcoFalke)
07397cd addrdb: Only call Serialize() once (Martin Zumsande)
91f83db hash: add HashedSourceWriter (Martin Zumsande)
5c824ac For feebump, ignore abandoned descendant spends (John Moffett)
428dcd5 wallet: Skip rescanning if wallet is more recent than tip (Andrew Chow)
cbcdafa test: wallet: check that labels are migrated to watchonly wallet (Sebastian Falbesoner)
342abfb wallet: fully migrate address book entries for watchonly/solvable wallets (Sebastian Falbesoner)

Pull request description:

  Backports:
  * #26595
  * #26675
  * #26679
  * #26761
  * #26837
  * #26909
  * #26924
  * #26944
  * bitcoin-core/gui#704
  * #27053
  * #27080

ACKs for top commit:
  instagibbs:
    ACK 784a754
  achow101:
    ACK 784a754
  hebasto:
    ACK 784a754, I've made backporting locally and got a diff between my branch and this PR as follows:

Tree-SHA512: 8ea84aa02d7907ff1e202e1302b441ce9ed2198bf383620ad40056a5d7e8ea88e1047abef0b92d85648016bf9b3195c974be3806ccebd85bef4f85c326869e43
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request May 4, 2023
… error

Summary:
Pull request description:

  On current master:

  ```
    CXX      support/libbitcoin_util_a-lockedpool.o
  support/lockedpool.cpp: In member function ‘void Arena::free(void*)’:
  support/lockedpool.cpp:99:20: error: ‘runtime_error’ is not a member of ‘std’
     99 |         throw std::runtime_error("Arena: invalid or double free");
        |                    ^~~~~~~~~~~~~
  support/lockedpool.cpp:22:1: note: ‘std::runtime_error’ is defined in header ‘<stdexcept>’; did you forget to ‘#include <stdexcept>’?
     21 | #include <algorithm>
    +++ |+#include <stdexcept>
     22 | #ifdef ARENA_DEBUG
  support/lockedpool.cpp: In member function ‘void LockedPool::free(void*)’:
  support/lockedpool.cpp:320:16: error: ‘runtime_error’ is not a member of ‘std’
    320 |     throw std::runtime_error("LockedPool: invalid address not pointing to any arena");
        |                ^~~~~~~~~~~~~
  support/lockedpool.cpp:320:16: note: ‘std::runtime_error’ is defined in header ‘<stdexcept>’; did you forget to ‘#include <stdexcept>’?

---

This is a backport of [[ bitcoin/bitcoin#26924 | core#26924 ]]

Test Plan:
  ninja all

Reviewers: #bitcoin_abc, PiRK

Reviewed By: #bitcoin_abc, PiRK

Differential Revision: https://reviews.bitcoinabc.org/D13860
ComputerCraftr pushed a commit to ComputerCraftr/XEP-Core that referenced this pull request Oct 29, 2023
ea584a617c6853eb1f9740600cd9db75d77948eb 23.x Add missing includes to fix gcc-13 compile error (fanquake)
c21e6a9ce210e0b0d1f4081ebd5dfefbe3c7af2d Add missing includes to fix gcc-13 compile error (MarcoFalke)

Pull request description:

  Backports:
  * bitcoin/bitcoin#26924

ACKs for top commit:
  instagibbs:
    ACK bitcoin/bitcoin@ea584a6

Tree-SHA512: e231d2e3e17f884eb9eb63f2f31829cef3e64351c9d6b90abeef421c366cef228a0a87b9c3a07c840879bd514ebe33b554fb62947c13bd05806865e59323cae7
ComputerCraftr pushed a commit to ElectraProtocol/OmniXEP that referenced this pull request Oct 29, 2023
ea584a6 23.x Add missing includes to fix gcc-13 compile error (fanquake)
c21e6a9 Add missing includes to fix gcc-13 compile error (MarcoFalke)

Pull request description:

  Backports:
  * bitcoin#26924

ACKs for top commit:
  instagibbs:
    ACK bitcoin@ea584a6

Tree-SHA512: e231d2e3e17f884eb9eb63f2f31829cef3e64351c9d6b90abeef421c366cef228a0a87b9c3a07c840879bd514ebe33b554fb62947c13bd05806865e59323cae7
@bitcoin bitcoin locked and limited conversation to collaborators Jan 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants