Merge bitcoin/bitcoin#30234: Enable clang-tidy checks for self-assignment

26a7f70 ci: enable self-assignment clang-tidy check (Cory Fields)
32b1d13 refactor: add self-assign checks to classes which violate the clang-tidy check (Cory Fields)

Pull request description:

  See comment here: bitcoin/bitcoin#30161 (comment)

  Our code failed these checks in three places, which have been fixed up here. Though these appear to have been harmless, adding the check avoids the copy in the self-assignment case so there should be no downside.

  ~Additionally, minisketch failed the check as well. See bitcoin-core/minisketch#87
  Edit: Done

  After fixing up the violations, turn on the aggressive clang-tidy check.

  Note for reviewers: `git diff -w` makes this trivial to review.

ACKs for top commit:
  hebasto:
    ACK 26a7f70, I have reviewed the code and it looks OK.
  TheCharlatan:
    ACK 26a7f70

Tree-SHA512: 74d8236a1b5a698f2f61c4740c4fc77788b7f882c4b395acc4e6bfef1ec8a4554ea8821a26b14d70cfa6c8e2e9ea305deeea3fbf323967fa19343c007a53c5ba

Author: fanquake

src/.clang-tidy

@@ -6,6 +6,7 @@ bugprone-move-forwarding-reference,
 bugprone-string-constructor,
 bugprone-use-after-move,
 bugprone-lambda-function-name,
+bugprone-unhandled-self-assignment,
 misc-unused-using-decls,
 misc-no-recursion,
 modernize-use-default-member-init,
@@ -24,8 +25,10 @@ readability-const-return-type,
 readability-redundant-declaration,
 readability-redundant-string-init,
 '
+HeaderFilterRegex: '.'
 WarningsAsErrors: '*'
 CheckOptions:
  - key: performance-move-const-arg.CheckTriviallyCopyableMove
    value: false
-HeaderFilterRegex: '.'
+ - key: bugprone-unhandled-self-assignment.WarnOnlyIfThisHasSuspiciousField
+   value: false

src/arith_uint256.h

@@ -43,8 +43,10 @@ class base_uint
 
     base_uint& operator=(const base_uint& b)
     {
-        for (int i = 0; i < WIDTH; i++)
-            pn[i] = b.pn[i];
+        if (this != &b) {
+            for (int i = 0; i < WIDTH; i++)
+                pn[i] = b.pn[i];
+        }
         return *this;
     }
 

src/key.h

@@ -75,13 +75,15 @@ class CKey
 
     CKey& operator=(const CKey& other)
     {
-        if (other.keydata) {
-            MakeKeyData();
-            *keydata = *other.keydata;
-        } else {
-            ClearKeyData();
+        if (this != &other) {
+            if (other.keydata) {
+                MakeKeyData();
+                *keydata = *other.keydata;
+            } else {
+                ClearKeyData();
+            }
+            fCompressed = other.fCompressed;
         }
-        fCompressed = other.fCompressed;
         return *this;
     }
 

src/test/util_tests.cpp

@@ -1508,8 +1508,10 @@ struct Tracker
     Tracker(Tracker&& t) noexcept : origin(t.origin), copies(t.copies) {}
     Tracker& operator=(const Tracker& t) noexcept
     {
-        origin = t.origin;
-        copies = t.copies + 1;
+        if (this != &t) {
+            origin = t.origin;
+            copies = t.copies + 1;
+        }
         return *this;
     }
 };