diff options
author | merge-script <fanquake@gmail.com> | 2024-06-10 09:36:07 +0100 |
---|---|---|
committer | merge-script <fanquake@gmail.com> | 2024-06-10 09:36:07 +0100 |
commit | 7fd4905c403ccb233f489e2f6a6aa3cd53b033ed (patch) | |
tree | 33ea03b3cb3d0b2500de6ab266e2e13e209449c7 /src | |
parent | ea88a7596e37c41ad5c3c73265eb9c54ae211da5 (diff) | |
parent | 15796d4b61342f75548b20a18c670ed21d102ba8 (diff) | |
download | bitcoin-7fd4905c403ccb233f489e2f6a6aa3cd53b033ed.tar.xz |
Merge bitcoin/bitcoin#30235: build: warn on self-assignment
15796d4b61342f75548b20a18c670ed21d102ba8 build: warn on self-assignment (Cory Fields)
53372f21767be449bb452fc3f5fe7f16286ae371 refactor: disable self-assign warning for tests (Cory Fields)
Pull request description:
Belt-and suspenders after #30234. Self-assignment should be safe _and_ discouraged.
We used to opt out of this warning because something deep in our serialization/byteswapping code could self-assign, but that doesn't appear to be the case anymore.
ACKs for top commit:
maflcko:
ACK 15796d4b61342f75548b20a18c670ed21d102ba8
fanquake:
ACK 15796d4b61342f75548b20a18c670ed21d102ba8 - not a huge fan of inline pragma usage, but this seems fine, given it's to work around an already-fixed compiler bug, and we'll only be carrying it for a shortish time in any case.
Tree-SHA512: 1f95f7c730b974ad1da55ebd381040bac312f2f380fff9d569ebab91d7c1963592a84d1613d81d96238c6f5a66aa40deebba68a76f6b24b02150d0a77c769654
Diffstat (limited to 'src')
-rw-r--r-- | src/test/fuzz/muhash.cpp | 12 | ||||
-rw-r--r-- | src/test/uint256_tests.cpp | 19 |
2 files changed, 31 insertions, 0 deletions
diff --git a/src/test/fuzz/muhash.cpp b/src/test/fuzz/muhash.cpp index 8304e6fdb8..dd34c465ed 100644 --- a/src/test/fuzz/muhash.cpp +++ b/src/test/fuzz/muhash.cpp @@ -43,7 +43,19 @@ FUZZ_TARGET(muhash) }, [&] { // Test that dividing a MuHash by itself brings it back to it's initial state + + // See note about clang + self-assignment in test/uint256_tests.cpp + #if defined(__clang__) + # pragma clang diagnostic push + # pragma clang diagnostic ignored "-Wself-assign-overloaded" + #endif + muhash /= muhash; + + #if defined(__clang__) + # pragma clang diagnostic pop + #endif + muhash.Finalize(out); out2 = uint256S(initial_state_hash); }, diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp index 5746961550..b7892a2139 100644 --- a/src/test/uint256_tests.cpp +++ b/src/test/uint256_tests.cpp @@ -267,6 +267,22 @@ BOOST_AUTO_TEST_CASE( conversion ) BOOST_AUTO_TEST_CASE( operator_with_self ) { + +/* Clang 16 and earlier detects v -= v and v /= v as self-assignments + to 0 and 1 respectively. + See: https://github.com/llvm/llvm-project/issues/42469 + and the fix in commit c5302325b2a62d77cf13dd16cd5c19141862fed0 . + + This makes some sense for arithmetic classes, but could be considered a bug + elsewhere. Disable the warning here so that the code can be tested, but the + warning should remain on as there will likely always be a better way to + express this. +*/ + +#if defined(__clang__) +# pragma clang diagnostic push +# pragma clang diagnostic ignored "-Wself-assign-overloaded" +#endif arith_uint256 v = UintToArith256(uint256S("02")); v *= v; BOOST_CHECK(v == UintToArith256(uint256S("04"))); @@ -276,6 +292,9 @@ BOOST_AUTO_TEST_CASE( operator_with_self ) BOOST_CHECK(v == UintToArith256(uint256S("02"))); v -= v; BOOST_CHECK(v == UintToArith256(uint256S("0"))); +#if defined(__clang__) +# pragma clang diagnostic pop +#endif } BOOST_AUTO_TEST_CASE(parse) |