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

[fwolff]

Summary

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.

[carlosgalvezp]

Looks good! I'm not sure if it's OK to write "dead tests" though (the one with the TODO comment). Would it make sense to remove it from this patch, and add it in the patch where the issue is fixed?

[fwolff]

Thanks for reviewing this @carlosgalvezp! I have removed the "dead" test and put it into a follow-up issue (PR#52502).

If you are otherwise happy with the changes, could you commit them for me?

[carlosgalvezp]

Awesome, thanks! Sure, I'm quite new here as well so I'll need to take some time to figure out how to commit on behalf of someone else, bear with me :)

[carlosgalvezp]

I will push tomorrow so I have time to keep an eye on the bots ;) Btw @fwolff could you post your name and email so I add it to the commit?

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

if (!TheCFG)
     return false;
 
   Sequence =
       std::make_unique<ExprSequence>(TheCFG.get(), FunctionBody, Context);
   BlockMap = std::make_unique<StmtToBlockMap>(TheCFG.get(), Context);
   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);
 }
 
 bool UseAfterMoveFinder::findInternal(const CFGBlock *Block,
                                       const Expr *MovingCall,
                                       const ValueDecl *MovedVariable,
                                       UseAfterMove *TheUseAfterMove) {

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

 }
 void typeId() {
   Foo Bar;
   // error: you need to include <typeinfo> before using the 'typeid' operator
   // (void) typeid(Foo{std::move(Bar)}).name();
   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_;
+};