[clang-tidy] `bugprone-use-after-move`: Don't warn on self-moves.

[njames93]

LGTM.

I feel that this case should produce a warning akin to the no self assignment diagnostics, obviously nothing to do with this check though.

[mboehme]

Added release notes.

[mboehme]

The reviewer on https://reviews.llvm.org/D126780 reminded me to add release notes, so I've done so here as well. I assume this does not require another round of review.

In D126853#3556029, @njames93 wrote:

LGTM.

I feel that this case should produce a warning akin to the no self assignment diagnostics, obviously nothing to do with this check though.

Makes sense -- acknowledged.

FWIW, the use case that prompted me to make this change was a test for the move assignment operator where clang-tidy was erroneously flagging the self-move as a use-after-move. Self-moves shouldn't occur outside of test code though, and it would be good to have some check produce a warning for them.

clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

bool UseAfterMoveFinder::findInternal(const CFGBlock *Block,
 
   // Get all uses and reinits in the block.
   llvm::SmallVector<const DeclRefExpr *, 1> Uses;
   llvm::SmallPtrSet<const Stmt *, 1> Reinits;
   getUsesAndReinits(Block, MovedVariable, &Uses, &Reinits);
 
   // Ignore all reinitializations where the move potentially comes after the
   // reinit.
+  // If `Reinit` is identical to `MovingCall`, we're looking at a move-to-self
+  // (e.g. `a = std::move(a)`). Count these as reinitializations.
   llvm::SmallVector<const Stmt *, 1> ReinitsToDelete;
   for (const Stmt *Reinit : Reinits) {
-    if (MovingCall && Sequence->potentiallyAfter(MovingCall, Reinit))
+    if (MovingCall && Reinit != MovingCall &&
+        Sequence->potentiallyAfter(MovingCall, Reinit))
       ReinitsToDelete.push_back(Reinit);
   }
   for (const Stmt *Reinit : ReinitsToDelete) {
     Reinits.erase(Reinit);
   }
 
   // Find all uses that potentially come after the move.
   for (const DeclRefExpr *Use : Uses) {

clang-tools-extra/docs/ReleaseNotes.rst

- Fixed a false positive in :doc:`readability-non-const-parameter
   referenced by an lvalue.
 
 - Expanded :doc:`readability-simplify-boolean-expr
   <clang-tidy/checks/readability-simplify-boolean-expr>` to simplify expressions
   using DeMorgan's Theorem.
 
 - Fixed a crash in :doc:`performance-unnecessary-value-param
   <clang-tidy/checks/readability-suspicious-call-argument>` when the specialization
-  template has an unnecessary value paramter. Removed the fix for a template.
+  template has an unnecessary value parameter. Removed the fix for a template.
 
-- Fixed a bug in :doc:`bugprone-use-after-move
-  <clang-tidy/checks/bugprone-use-after-move>` where a move in a lambda capture
-  was treated as if it happened within the body of the lambda, not within the
-  function that defines the lambda.
+- Fixed bugs in :doc:`bugprone-use-after-move
+  <clang-tidy/checks/bugprone-use-after-move>`:
+  - Treat a move in a lambda capture as happening in the function that defines
+    the lambda, not within the body of the lambda (as we were previously doing
+    erroneously).
+  - Don't emit an erroneous warning on self-moves.
 
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...

clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

void simple() {
   A a;
   a.foo();
   A other_a = std::move(a);
   a.foo();
   // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved
   // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
 }
 
+// Don't flag a move-to-self.
+void selfMove() {
+  A a;
+  a = std::move(a);
+  a.foo();
+}
+
 // A warning should only be emitted for one use-after-move.
 void onlyFlagOneUseAfterMove() {
   A a;
   a.foo();
   A other_a = std::move(a);
   a.foo();
   // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved
   // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here