[clang-tidy] Improve bugprone-use-after-move to handle lambdas

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

@@ -79,6 +79,21 @@ class UseAfterMoveFinder {
   llvm::SmallPtrSet<const CFGBlock *, 8> Visited;
 };
 
+AST_MATCHER_P(CXXRecordDecl, hasCaptureByReference, const ValueDecl *,
+              TargetDecl) {
+  if (!Node.isLambda())
+    return false;
+
+  if (llvm::any_of(Node.captures(), [&](const auto &Capture) {
+        return Capture.capturesVariable() &&
+               Capture.getCaptureKind() == LCK_ByRef &&
+               Capture.getCapturedVar() == TargetDecl;
+      })) {
+    return true;
+  }
+  return false;
+}
+
 } // namespace
 
 static auto getNameMatcher(llvm::ArrayRef<StringRef> InvalidationFunctions) {
@@ -150,6 +165,61 @@ makeReinitMatcher(const ValueDecl *MovedVariable,
       .bind("reinit");
 }
 
+static bool
+isVariableResetInLambda(const Stmt *Body, const ValueDecl *MovedVariable,
+                        ASTContext *Context,
+                        llvm::ArrayRef<StringRef> InvalidationFunctions) {
+  assert(Body && "There should be a lambda body");
+
+  // If the variable is not mentioned at all in the lambda body,
+  // it cannot be reinitialized.
+  const auto VariableMentionMatcher = stmt(anyOf(
+      hasDescendant(declRefExpr(hasDeclaration(equalsNode(MovedVariable)))),
+      hasDescendant(memberExpr(hasDeclaration(equalsNode(MovedVariable))))));
+
+  if (match(VariableMentionMatcher, *Body, *Context).empty())
+    return false;
+
+  CFG::BuildOptions Options;
+  Options.AddImplicitDtors = true;
+  Options.AddTemporaryDtors = true;
+  Options.PruneTriviallyFalseEdges = true;
+
+  std::unique_ptr<CFG> TheCFG =
+      CFG::buildCFG(nullptr, const_cast<Stmt *>(Body), Context, Options);
+  if (!TheCFG)
+    return false;
+
+  llvm::SmallPtrSet<const CFGBlock *, 8> VisitedBlocks;
+  llvm::SmallVector<const CFGBlock *, 8> Worklist;
+
+  Worklist.push_back(&TheCFG->getEntry());
+  VisitedBlocks.insert(&TheCFG->getEntry());
+
+  const auto ReinitMatcher =
+      makeReinitMatcher(MovedVariable, InvalidationFunctions);
+
+  while (!Worklist.empty()) {
+    const CFGBlock *CurrentBlock = Worklist.pop_back_val();
+
+    // Check for reinitialization within reachable blocks.
+    for (const auto &Elem : *CurrentBlock) {
+      std::optional<CFGStmt> S = Elem.getAs<CFGStmt>();
+      if (!S)
+        continue;
+
+      if (!match(findAll(ReinitMatcher), *S->getStmt(), *Context).empty())
+        return true;
+    }
+
+    for (const auto &Succ : CurrentBlock->succs())
+      if (Succ && VisitedBlocks.insert(Succ).second)
+        Worklist.push_back(Succ);
+  }
+
+  return false;
+}
+
 // Matches nodes that are
 // - Part of a decltype argument or class template argument (we check this by
 //   seeing if they are children of a TypeLoc), or
@@ -374,6 +444,13 @@ void UseAfterMoveFinder::getReinits(
   const auto ReinitMatcher =
       makeReinitMatcher(MovedVariable, InvalidationFunctions);
 
+  // Match calls to lambdas that capture the moved variable by reference.
+  const auto LambdaCallMatcher =
+      cxxOperatorCallExpr(
+          hasOverloadedOperatorName("()"),
+          callee(cxxMethodDecl(ofClass(hasCaptureByReference(MovedVariable)))))
+          .bind("lambda-call");
+
   Stmts->clear();
   DeclRefs->clear();
   for (const auto &Elem : *Block) {
@@ -397,6 +474,30 @@ void UseAfterMoveFinder::getReinits(
           DeclRefs->insert(TheDeclRef);
       }
     }
+
+    // Check for calls to lambdas that capture the moved variable
+    // by reference and reinitialize it within their body.
+    const SmallVector<BoundNodes, 1> LambdaMatches =
+        match(findAll(LambdaCallMatcher), *S->getStmt(), *Context);
+
+    for (const auto &Match : LambdaMatches) {
+      const auto *Operator =
+          Match.getNodeAs<CXXOperatorCallExpr>("lambda-call");
+
+      if (Operator && BlockMap->blockContainingStmt(Operator) == Block) {
+        const auto *MD =
+            dyn_cast_or_null<CXXMethodDecl>(Operator->getDirectCallee());
+        if (!MD)
+          continue;
+
+        const auto *RD = MD->getParent();
+        const auto *LambdaBody = MD->getBody();
+        if (RD && RD->isLambda() && LambdaBody &&
+            isVariableResetInLambda(LambdaBody, MovedVariable, Context,
+                                    InvalidationFunctions))
+          Stmts->insert(Operator);
+      }
+    }
   }
 }
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -446,7 +446,8 @@ Changes in existing checks
 
 - Improved :doc:`bugprone-use-after-move
   <clang-tidy/checks/bugprone/use-after-move>` check by adding
-  `InvalidationFunctions` option to support custom invalidation functions.
+  `InvalidationFunctions` option to support custom invalidation functions and
+  by enabling the check to handle lambda correctly.
 
 - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check

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

@@ -1703,3 +1703,86 @@ void Run() {
 }
 
 } // namespace custom_invalidation
+
+void lambdaReinitializesVar() {
+  std::string s1;
+  auto reinit1 = [&]() { s1 = std::string(); };
+  std::move(s1);
+  reinit1();
+  s1.empty();
+
+  std::string s2;
+  auto reinit2 = [&]() { s2.clear(); };
+  std::move(s2);
+  reinit2();
+  s2.empty();
+
+  std::string s3;
+  auto reinit3 = [&]() { s3.assign(10, 'a'); };
+  std::move(s3);
+  reinit3();
+  s3.empty();
+}
+
+void lambdaReinitializesVarInLoop() {
+  std::string s;
+  auto reinit = [&]() {
+    s = std::string();
+    return true;
+  };
+  for (int i = 0; i < 10; ++i) {
+    if (reinit()) {
+      if (!s.empty()) {
+        std::move(s);
+      }
+    }
+  }
+}
+
+void lambdaDoesNotReinitializeVar() {
+  {
+    std::string s;
+    auto read = [&]() { return s.empty(); };
+    std::move(s);
+    read();
+    s.empty();
+    // CHECK-NOTES: [[@LINE-1]]:5: warning: 's' used after it was moved
+    // CHECK-NOTES: [[@LINE-4]]:5: note: move occurred here
+  }
+
+  {
+    std::string s;
+    auto reinitCopy = [s]() mutable { s = std::string(); };
+    std::move(s);
+    reinitCopy();
+    s.empty();
+    // CHECK-NOTES: [[@LINE-1]]:5: warning: 's' used after it was moved
+    // CHECK-NOTES: [[@LINE-4]]:5: note: move occurred here
+  }
+}
+
+void lambdaReinitConditional() {
+  std::string s;
+  auto reinit = [&](bool b) {
+    if (b) s = std::string();
+  };
+  std::move(s);
+  reinit(true);
+  s.empty();
+}
+
+void lambdaReinitInDeadCode() {
+  std::string s;
+  auto g = [&]() {
+    if (false) {
+      s = std::string();
+    }
+  };
+  std::move(s);
+
+  g();
+
+  s.empty();
+  // CHECK-NOTES: [[@LINE-1]]:3: warning: 's' used after it was moved
+  // CHECK-NOTES: [[@LINE-6]]:3: note: move occurred here
+}