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

[Fabian Wolff via Phabricator via cfe-commits]

fwolff created this revision.
fwolff added reviewers: alexfh, carlosgalvezp.
fwolff added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.
fwolff requested review of this revision.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113708

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


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -1338,3 +1338,26 @@
   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_;
+};
+
+class UseAfterMoveInCtorInit {
+public:
+  // TODO: warn here
+  UseAfterMoveInCtorInit(std::string val) : s(std::move(val)), b(val.empty()) {
+  }
+
+private:
+  std::string s;
+  bool b;
+};
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -129,8 +129,12 @@
   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);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D113708.386633.patch
Type: text/x-patch
Size: 1576 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211111/efb63c1a/attachment.bin>