util: Treat Assume as Assert when evaluating at compile-time #31150

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2410-assume-stronger changing 1 files +2 −2
  1. maflcko commented at 3:05 pm on October 24, 2024: member

    There is no downside or cost of treating an Assume at compile-time as an Assert and it may even help to find bugs while compiling without ABORT_ON_FAILED_ASSUME.

    This is also required for https://github.com/bitcoin/bitcoin/pull/31093

  2. DrahtBot commented at 3:05 pm on October 24, 2024: contributor

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

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK dergoegge, brunoerg, marcofleon

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

  3. DrahtBot added the label Utils/log/libs on Oct 24, 2024
  4. maflcko commented at 3:07 pm on October 24, 2024: member

    Can be tested by adding a failing Assume at compile-time and observing that it passes on master, but fails to compile with this commit:

     0diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
 1index ba4ba6c3b6..4f8815e179 100644
 2--- a/src/txorphanage.cpp
 3+++ b/src/txorphanage.cpp
 4@@ -99,6 +99,7 @@ void TxOrphanage::EraseForPeer(NodeId peer)
 5 
 6 void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
 7 {
 8+constexpr int FOO{Assume(0)};
 9     unsigned int nEvicted = 0;
10     auto nNow{Now<NodeSeconds>()};
11     if (m_next_sweep <= nNow) {

    Master:

    0src/txorphanage.cpp:102:15: warning: unused variable 'FOO' [-Wunused-variable]
1  102 | constexpr int FOO{Assume(0)};
2      |               ^~~
31 warning generated.

    This pull:

    0src/txorphanage.cpp:102:15: error: constexpr variable 'FOO' must be initialized by a constant expression
1  102 | constexpr int FOO{Assume(0)};
2      |               ^  ~~~~~~~~~~~
31 error generated.
  5. dergoegge approved
  6. dergoegge commented at 3:12 pm on October 24, 2024: member
    ACK fa824ea44c7b1d55f20e4941d78641cd129e1ea3
  7. util: Treat Assume as Assert when evaluating at compile-time fa69a5f4b7
  8. maflcko force-pushed on Oct 24, 2024
  9. DrahtBot commented at 3:19 pm on October 24, 2024: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/32016288650

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  10. DrahtBot added the label CI failed on Oct 24, 2024
  11. maflcko commented at 3:25 pm on October 24, 2024: member
    Force pushed to fix a true warning that didn’t show locally.
  12. dergoegge approved
  13. dergoegge commented at 4:02 pm on October 24, 2024: member
    ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd
  14. DrahtBot removed the label CI failed on Oct 24, 2024
  15. fanquake requested review from brunoerg on Oct 25, 2024
  16. fanquake requested review from marcofleon on Oct 25, 2024
  17. maflcko added the label DrahtBot Guix build requested on Oct 25, 2024
  18. brunoerg approved
  19. brunoerg commented at 10:01 am on October 25, 2024: contributor
    ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd
  20. marcofleon approved
  21. marcofleon commented at 10:25 am on October 25, 2024: contributor
    ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd
  22. fanquake commented at 12:07 pm on October 25, 2024: member

    Guix Build:

     07a44582701c541810939dcc31608f04f52fdde4d60281d3432c246df1df2b903  guix-build-fa69a5f4b76a/output/aarch64-linux-gnu/SHA256SUMS.part
 1688a63d66f5d194f84403dd193a69db76a44c767abdcb2a1ad1ffb20a5e8ecbc  guix-build-fa69a5f4b76a/output/aarch64-linux-gnu/bitcoin-fa69a5f4b76a-aarch64-linux-gnu-debug.tar.gz
 25357d07cb97dd44e870c83863cb1bf5db963a32d25dd87add96474f48cb3d74d  guix-build-fa69a5f4b76a/output/aarch64-linux-gnu/bitcoin-fa69a5f4b76a-aarch64-linux-gnu.tar.gz
 341318dc317d5aeb245306c3017954bb736d8ad5eb99bd3d5f7820a1bba9a1b85  guix-build-fa69a5f4b76a/output/arm-linux-gnueabihf/SHA256SUMS.part
 4ef68e0c188aea33a73599e0dd73997b5545ed5e10ee55914c869bea0f11c1bd5  guix-build-fa69a5f4b76a/output/arm-linux-gnueabihf/bitcoin-fa69a5f4b76a-arm-linux-gnueabihf-debug.tar.gz
 55c1adbc0d5d25f9c4d92a12564e227cc055bb4bf1b795d1d3ef7cfbd5fe00596  guix-build-fa69a5f4b76a/output/arm-linux-gnueabihf/bitcoin-fa69a5f4b76a-arm-linux-gnueabihf.tar.gz
 6e1181f1b6abd53d12449577b9f93fef3ff9d3020d4278d474422d946dc92ea83  guix-build-fa69a5f4b76a/output/arm64-apple-darwin/SHA256SUMS.part
 77078fc7ae690aad5e465e299cfa64ab7590e210bc4318489f2e4bc6577a2fddb  guix-build-fa69a5f4b76a/output/arm64-apple-darwin/bitcoin-fa69a5f4b76a-arm64-apple-darwin-unsigned.tar.gz
 86862e21738651e7a71d88448223e189b5781e006efed2cc2f9441a3ae4b5ce47  guix-build-fa69a5f4b76a/output/arm64-apple-darwin/bitcoin-fa69a5f4b76a-arm64-apple-darwin-unsigned.zip
 9f4c74904148dd838be8eddb6e24401f69799328bb644b35f0038b332bda5dc41  guix-build-fa69a5f4b76a/output/arm64-apple-darwin/bitcoin-fa69a5f4b76a-arm64-apple-darwin.tar.gz
10cdf253c991863f3d6ea42ee87bf7c3fb076c7911c6828356aeb839db3badecf6  guix-build-fa69a5f4b76a/output/dist-archive/bitcoin-fa69a5f4b76a.tar.gz
111334aa50d75b395e9ad7becc978fdb9ae7d0629250fb90e08467a340b21875f7  guix-build-fa69a5f4b76a/output/powerpc64-linux-gnu/SHA256SUMS.part
129b2d3d61e0ce6e548f41a8eb2b3e41e29b6a79bafa8262fcb0df512d666fba04  guix-build-fa69a5f4b76a/output/powerpc64-linux-gnu/bitcoin-fa69a5f4b76a-powerpc64-linux-gnu-debug.tar.gz
136bd64e745d1291c0f5df8a80f10160af8fbb062158211f53714e7e98d6b4b339  guix-build-fa69a5f4b76a/output/powerpc64-linux-gnu/bitcoin-fa69a5f4b76a-powerpc64-linux-gnu.tar.gz
14472ad6a262d0f54c4d179bc725a25b97799349ca6fa0c8b76a874bf929caa28b  guix-build-fa69a5f4b76a/output/riscv64-linux-gnu/SHA256SUMS.part
15055f2ae19a2c45d7e04108428869c8b1d288738c58934a3dea0f73ff62a66a79  guix-build-fa69a5f4b76a/output/riscv64-linux-gnu/bitcoin-fa69a5f4b76a-riscv64-linux-gnu-debug.tar.gz
16679a1de292ba51a97e289debbb798db8e9ac1343f7524c02156a5c94ff4e21a1  guix-build-fa69a5f4b76a/output/riscv64-linux-gnu/bitcoin-fa69a5f4b76a-riscv64-linux-gnu.tar.gz
17cb49ae504cbf7d94065515776a5c2107b106aa5fdeae23eab7c470332e816628  guix-build-fa69a5f4b76a/output/x86_64-apple-darwin/SHA256SUMS.part
18e973965dc03ae0f52e898c199e32f8f7fe0d372c9c3fdbd8ca059d46fb0ce261  guix-build-fa69a5f4b76a/output/x86_64-apple-darwin/bitcoin-fa69a5f4b76a-x86_64-apple-darwin-unsigned.tar.gz
19c8349576edfc7dcc07a340b57fb388e320c65f15a22405ba5316e317eb92d604  guix-build-fa69a5f4b76a/output/x86_64-apple-darwin/bitcoin-fa69a5f4b76a-x86_64-apple-darwin-unsigned.zip
202f98dc6b9243e8336b8d290df6d0dd4bb4b44fba2b19b655714cdda134627f90  guix-build-fa69a5f4b76a/output/x86_64-apple-darwin/bitcoin-fa69a5f4b76a-x86_64-apple-darwin.tar.gz
21602178ad962d162fc464130f6db47732c43e5f4b1b2cd54b0cdf66565f3a08d4  guix-build-fa69a5f4b76a/output/x86_64-linux-gnu/SHA256SUMS.part
2255406fe0d3a7d346d89f1d77935ee42743f8e5fa1504b1f834656cd94d3c86d7  guix-build-fa69a5f4b76a/output/x86_64-linux-gnu/bitcoin-fa69a5f4b76a-x86_64-linux-gnu-debug.tar.gz
23d5e6bc5e69dff3502ccd03cd87dcb9dc7a081e455a33d4f02491ea5390641805  guix-build-fa69a5f4b76a/output/x86_64-linux-gnu/bitcoin-fa69a5f4b76a-x86_64-linux-gnu.tar.gz
2463ef52c3b126d20ba0ddb47b44faab48399b475ab7055ca32f4e789212459695  guix-build-fa69a5f4b76a/output/x86_64-w64-mingw32/SHA256SUMS.part
25bc566b080621728d3a832a00a56261080687bd21523a53fced06e9b8acee17d3  guix-build-fa69a5f4b76a/output/x86_64-w64-mingw32/bitcoin-fa69a5f4b76a-win64-debug.zip
2680ae4adb8a8698cf65ca49dc0156fabdc8cf15dbde021b918e5720b84709d066  guix-build-fa69a5f4b76a/output/x86_64-w64-mingw32/bitcoin-fa69a5f4b76a-win64-setup-unsigned.exe
2741225019a960f9bfe8f6dc6993b725e56e2c9bfc48c5c40851231e42484e883f  guix-build-fa69a5f4b76a/output/x86_64-w64-mingw32/bitcoin-fa69a5f4b76a-win64-unsigned.tar.gz
288fcc575e8718566e12f5f2decd512060c033458dd7b641a6728e6fc2d2a03b36  guix-build-fa69a5f4b76a/output/x86_64-w64-mingw32/bitcoin-fa69a5f4b76a-win64.zip
  23. fanquake merged this on Oct 25, 2024
  24. fanquake closed this on Oct 25, 2024
  25. maflcko removed the label DrahtBot Guix build requested on Oct 25, 2024
  26. maflcko deleted the branch on Oct 25, 2024
  27. stickies-v commented at 2:11 pm on October 25, 2024: contributor
    Post-merge ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd - nice!


maflcko DrahtBot dergoegge brunoerg marcofleon fanquake stickies-v

Labels
Utils/log/libs

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-26 12:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me