[clang-tools-extra] c3e3c76 - [clang-tidy] Fix `bugprone-use-after-move` check to also consider moves in constructor initializers

[Carlos Galvez via cfe-commits]

Author: Fabian Wolff
Date: 2021-11-15T07:41:35Z
New Revision: c3e3c762098e8d425731bb40f3b8b04dac1013f3

URL: https://github.com/llvm/llvm-project/commit/c3e3c762098e8d425731bb40f3b8b04dac1013f3
DIFF: https://github.com/llvm/llvm-project/commit/c3e3c762098e8d425731bb40f3b8b04dac1013f3.diff

LOG: [clang-tidy] Fix `bugprone-use-after-move` check to also consider moves in constructor initializers

Fixes PR#38187. Constructors are actually already checked,
but only as functions, i.e. the check only looks at the
constructor body and not at the initializers, which misses
the (common) case where constructor parameters are moved
as part of an initializer expression.

One remaining false negative is when both the move //and//
the use-after-move occur in constructor initializers.
This is a lot more difficult to handle, though, because
the `bugprone-use-after-move` check is currently based on
a CFG that only takes the body into account, not the
initializers, so e.g. initialization order would have to
manually be considered. I will file a follow-up issue for
this once PR#38187 is closed.

Reviewed By: carlosgalvezp

Differential Revision: https://reviews.llvm.org/D113708

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 136d8f862b956..064b6ae19784e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -129,8 +129,12 @@ bool UseAfterMoveFinder::find(Stmt *FunctionBody, const Expr *MovingCall,
   Visited.clear();
 
   const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
-  if (!Block)
-    return false;
+  if (!Block) {
+    // This can happen if MovingCall is in a constructor initializer, which is
+    // not included in the CFG because the CFG is built only from the function
+    // body.
+    Block = &TheCFG->getEntry();
+  }
 
   return findInternal(Block, MovingCall, MovedVariable, TheUseAfterMove);
 }

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
index 73ca59ccc91bf..e26db0f6793e0 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -1338,3 +1338,15 @@ void typeId() {
   Foo Other{std::move(Bar)};
 }
 } // namespace UnevalContext
+
+class PR38187 {
+public:
+  PR38187(std::string val) : val_(std::move(val)) {
+    val.empty();
+    // CHECK-NOTES: [[@LINE-1]]:5: warning: 'val' used after it was moved
+    // CHECK-NOTES: [[@LINE-3]]:30: note: move occurred here
+  }
+
+private:
+  std::string val_;
+};