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
This commit is contained in:
merge-script 2024-06-10 09:36:07 +01:00
commit 7fd4905c40
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1
3 changed files with 32 additions and 1 deletions

View File

@ -411,12 +411,12 @@ AX_CHECK_COMPILE_FLAG([-Wsuggest-override], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wsug
AX_CHECK_COMPILE_FLAG([-Wimplicit-fallthrough], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wimplicit-fallthrough"], [], [$CXXFLAG_WERROR])
AX_CHECK_COMPILE_FLAG([-Wunreachable-code], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunreachable-code"], [], [$CXXFLAG_WERROR])
AX_CHECK_COMPILE_FLAG([-Wdocumentation], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdocumentation"], [], [$CXXFLAG_WERROR])
AX_CHECK_COMPILE_FLAG([-Wself-assign], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wself-assign"], [], [$CXXFLAG_WERROR])
dnl Some compilers (gcc) ignore unknown -Wno-* options, but warn about all
dnl unknown options if any other warning is produced. Test the -Wfoo case, and
dnl set the -Wno-foo case if it works.
AX_CHECK_COMPILE_FLAG([-Wunused-parameter], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-unused-parameter"], [], [$CXXFLAG_WERROR])
AX_CHECK_COMPILE_FLAG([-Wself-assign], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-self-assign"], [], [$CXXFLAG_WERROR])
dnl Don't allow extended (non-ASCII) symbols in identifiers. This is easier for code review.
AX_CHECK_COMPILE_FLAG([-fno-extended-identifiers], [CORE_CXXFLAGS="$CORE_CXXFLAGS -fno-extended-identifiers"], [], [$CXXFLAG_WERROR])

View File

@ -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);
},

View File

@ -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)