[clang-tools-extra] [clang-tidy] Improve `bugprone-use-after-move` to handle lambdas (PR #173192)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 22 23:02:43 PST 2025
https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/173192
>From dd6ebb87189cac5f6f6188843a98dbadced5a469 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Sun, 21 Dec 2025 18:54:09 +0800
Subject: [PATCH 01/13] [clang-tidy] Improve bugprone-use-after-move to handle
lambdas
---
.../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 201 ++++++++++++------
1 file changed, 140 insertions(+), 61 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index b2e08fe688a1b..1f42b29aa6d96 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -8,8 +8,10 @@
#include "UseAfterMoveCheck.h"
+#include "clang/AST/DeclCXX.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/AST/LambdaCapture.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h"
#include "clang/Analysis/CFG.h"
@@ -31,6 +33,112 @@ using matchers::hasUnevaluatedContext;
namespace {
+AST_MATCHER_P(CXXRecordDecl, hasCaptureByReference, const ValueDecl *,
+ TargetDecl) {
+ if (!Node.isLambda())
+ return false;
+
+ for (const auto &Capture : Node.captures()) {
+ if (Capture.capturesVariable() && Capture.getCaptureKind() == LCK_ByRef &&
+ Capture.getCapturedVar() == TargetDecl) {
+ return true;
+ }
+ }
+ return false;
+}
+
+static auto getNameMatcher(llvm::ArrayRef<StringRef> InvalidationFunctions) {
+ return anyOf(hasAnyName("::std::move", "::std::forward"),
+ matchers::matchesAnyListedName(InvalidationFunctions));
+}
+
+StatementMatcher
+makeReinitMatcher(const ValueDecl *MovedVariable,
+ llvm::ArrayRef<StringRef> InvalidationFunctions) {
+ const auto DeclRefMatcher =
+ declRefExpr(hasDeclaration(equalsNode(MovedVariable))).bind("declref");
+
+ static const auto StandardContainerTypeMatcher =
+ hasType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(cxxRecordDecl(hasAnyName(
+ "::std::basic_string", "::std::vector", "::std::deque",
+ "::std::forward_list", "::std::list", "::std::set", "::std::map",
+ "::std::multiset", "::std::multimap", "::std::unordered_set",
+ "::std::unordered_map", "::std::unordered_multiset",
+ "::std::unordered_multimap"))))));
+
+ static const auto StandardResettableOwnerTypeMatcher = hasType(
+ hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
+ hasAnyName("::std::unique_ptr", "::std::shared_ptr",
+ "::std::weak_ptr", "::std::optional", "::std::any"))))));
+
+ // Matches different types of reinitialization.
+ return stmt(
+ anyOf(
+ // Assignment. In addition to the overloaded assignment
+ // operator, test for built-in assignment as well, since
+ // template functions may be instantiated to use std::move() on
+ // built-in types.
+ binaryOperation(hasOperatorName("="), hasLHS(DeclRefMatcher)),
+ // Declaration. We treat this as a type of reinitialization
+ // too, so we don't need to treat it separately.
+ declStmt(hasDescendant(equalsNode(MovedVariable))),
+ // clear() and assign() on standard containers.
+ cxxMemberCallExpr(
+ on(expr(DeclRefMatcher, StandardContainerTypeMatcher)),
+ // To keep the matcher simple, we check for assign() calls
+ // on all standard containers, even though only vector,
+ // deque, forward_list and list have assign(). If assign()
+ // is called on any of the other containers, this will be
+ // flagged by a compile error anyway.
+ callee(cxxMethodDecl(hasAnyName("clear", "assign")))),
+ // reset() on standard smart pointers.
+ cxxMemberCallExpr(on(expr(DeclRefMatcher,
+ StandardResettableOwnerTypeMatcher)),
+ callee(cxxMethodDecl(hasName("reset")))),
+ // Methods that have the [[clang::reinitializes]] attribute.
+ cxxMemberCallExpr(on(DeclRefMatcher),
+ callee(cxxMethodDecl(
+ hasAttr(clang::attr::Reinitializes)))),
+ // Passing variable to a function as a non-const pointer.
+ callExpr(forEachArgumentWithParam(
+ unaryOperator(hasOperatorName("&"),
+ hasUnaryOperand(DeclRefMatcher)),
+ unless(
+ parmVarDecl(hasType(pointsTo(isConstQualified())))))),
+ // Passing variable to a function as a non-const lvalue
+ // reference (unless that function is std::move()).
+ callExpr(forEachArgumentWithParam(
+ traverse(TK_AsIs, DeclRefMatcher),
+ unless(parmVarDecl(hasType(
+ references(qualType(isConstQualified())))))),
+ unless(callee(functionDecl(
+ getNameMatcher(InvalidationFunctions)))))))
+ .bind("reinit");
+}
+
+
+bool isVariableResetInLambda(const Stmt *Body, const ValueDecl *MovedVariable,
+ ASTContext *Context,
+ llvm::ArrayRef<StringRef> InvalidationFunctions) {
+ if (!Body)
+ return false;
+
+ // 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;
+
+ const auto ReinitMatcher =
+ makeReinitMatcher(MovedVariable, InvalidationFunctions);
+
+ return !match(findAll(ReinitMatcher), *Body, *Context).empty();
+}
+
/// Contains information about a use-after-move.
struct UseAfterMove {
// The DeclRefExpr that constituted the use of the object.
@@ -81,11 +189,6 @@ class UseAfterMoveFinder {
} // namespace
-static auto getNameMatcher(llvm::ArrayRef<StringRef> InvalidationFunctions) {
- return anyOf(hasAnyName("::std::move", "::std::forward"),
- matchers::matchesAnyListedName(InvalidationFunctions));
-}
-
// 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
@@ -313,63 +416,15 @@ void UseAfterMoveFinder::getReinits(
const CFGBlock *Block, const ValueDecl *MovedVariable,
llvm::SmallPtrSetImpl<const Stmt *> *Stmts,
llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs) {
- auto DeclRefMatcher =
- declRefExpr(hasDeclaration(equalsNode(MovedVariable))).bind("declref");
-
- auto StandardContainerTypeMatcher = hasType(hasUnqualifiedDesugaredType(
- recordType(hasDeclaration(cxxRecordDecl(hasAnyName(
- "::std::basic_string", "::std::vector", "::std::deque",
- "::std::forward_list", "::std::list", "::std::set", "::std::map",
- "::std::multiset", "::std::multimap", "::std::unordered_set",
- "::std::unordered_map", "::std::unordered_multiset",
- "::std::unordered_multimap"))))));
+ const auto ReinitMatcher =
+ makeReinitMatcher(MovedVariable, InvalidationFunctions);
- auto StandardResettableOwnerTypeMatcher = hasType(
- hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
- hasAnyName("::std::unique_ptr", "::std::shared_ptr",
- "::std::weak_ptr", "::std::optional", "::std::any"))))));
-
- // Matches different types of reinitialization.
- auto ReinitMatcher =
- stmt(anyOf(
- // Assignment. In addition to the overloaded assignment operator,
- // test for built-in assignment as well, since template functions
- // may be instantiated to use std::move() on built-in types.
- binaryOperation(hasOperatorName("="), hasLHS(DeclRefMatcher)),
- // Declaration. We treat this as a type of reinitialization too,
- // so we don't need to treat it separately.
- declStmt(hasDescendant(equalsNode(MovedVariable))),
- // clear() and assign() on standard containers.
- cxxMemberCallExpr(
- on(expr(DeclRefMatcher, StandardContainerTypeMatcher)),
- // To keep the matcher simple, we check for assign() calls
- // on all standard containers, even though only vector,
- // deque, forward_list and list have assign(). If assign()
- // is called on any of the other containers, this will be
- // flagged by a compile error anyway.
- callee(cxxMethodDecl(hasAnyName("clear", "assign")))),
- // reset() on standard smart pointers.
- cxxMemberCallExpr(
- on(expr(DeclRefMatcher, StandardResettableOwnerTypeMatcher)),
- callee(cxxMethodDecl(hasName("reset")))),
- // Methods that have the [[clang::reinitializes]] attribute.
- cxxMemberCallExpr(
- on(DeclRefMatcher),
- callee(cxxMethodDecl(hasAttr(clang::attr::Reinitializes)))),
- // Passing variable to a function as a non-const pointer.
- callExpr(forEachArgumentWithParam(
- unaryOperator(hasOperatorName("&"),
- hasUnaryOperand(DeclRefMatcher)),
- unless(parmVarDecl(hasType(pointsTo(isConstQualified())))))),
- // Passing variable to a function as a non-const lvalue reference
- // (unless that function is std::move()).
- callExpr(forEachArgumentWithParam(
- traverse(TK_AsIs, DeclRefMatcher),
- unless(parmVarDecl(hasType(
- references(qualType(isConstQualified())))))),
- unless(callee(functionDecl(
- getNameMatcher(InvalidationFunctions)))))))
- .bind("reinit");
+ // 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();
@@ -394,6 +449,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);
+ }
+ }
}
}
>From 95a3a9ffc7ee339b617cb09f958302513cc513ae Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Sun, 21 Dec 2025 21:56:14 +0800
Subject: [PATCH 02/13] [clang-tidy] Reformatting
---
.../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 117 +++++++++---------
1 file changed, 57 insertions(+), 60 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 87941eeba2840..cd4bf400d3985 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -33,6 +33,54 @@ using matchers::hasUnevaluatedContext;
namespace {
+/// Contains information about a use-after-move.
+struct UseAfterMove {
+ // The DeclRefExpr that constituted the use of the object.
+ const DeclRefExpr *DeclRef;
+
+ // Is the order in which the move and the use are evaluated undefined?
+ bool EvaluationOrderUndefined = false;
+
+ // Does the use happen in a later loop iteration than the move?
+ //
+ // We default to false and change it to true if required in find().
+ bool UseHappensInLaterLoopIteration = false;
+};
+
+/// Finds uses of a variable after a move (and maintains state required by the
+/// various internal helper functions).
+class UseAfterMoveFinder {
+public:
+ UseAfterMoveFinder(ASTContext *TheContext,
+ llvm::ArrayRef<StringRef> InvalidationFunctions);
+
+ // Within the given code block, finds the first use of 'MovedVariable' that
+ // occurs after 'MovingCall' (the expression that performs the move). If a
+ // use-after-move is found, writes information about it to 'TheUseAfterMove'.
+ // Returns whether a use-after-move was found.
+ std::optional<UseAfterMove> find(Stmt *CodeBlock, const Expr *MovingCall,
+ const DeclRefExpr *MovedVariable);
+
+private:
+ std::optional<UseAfterMove> findInternal(const CFGBlock *Block,
+ const Expr *MovingCall,
+ const ValueDecl *MovedVariable);
+ void getUsesAndReinits(const CFGBlock *Block, const ValueDecl *MovedVariable,
+ llvm::SmallVectorImpl<const DeclRefExpr *> *Uses,
+ llvm::SmallPtrSetImpl<const Stmt *> *Reinits);
+ void getDeclRefs(const CFGBlock *Block, const Decl *MovedVariable,
+ llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs);
+ void getReinits(const CFGBlock *Block, const ValueDecl *MovedVariable,
+ llvm::SmallPtrSetImpl<const Stmt *> *Stmts,
+ llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs);
+
+ ASTContext *Context;
+ llvm::ArrayRef<StringRef> InvalidationFunctions;
+ std::unique_ptr<ExprSequence> Sequence;
+ std::unique_ptr<StmtToBlockMap> BlockMap;
+ llvm::SmallPtrSet<const CFGBlock *, 8> Visited;
+};
+
AST_MATCHER_P(CXXRecordDecl, hasCaptureByReference, const ValueDecl *,
TargetDecl) {
if (!Node.isLambda())
@@ -52,22 +100,21 @@ static auto getNameMatcher(llvm::ArrayRef<StringRef> InvalidationFunctions) {
matchers::matchesAnyListedName(InvalidationFunctions));
}
-StatementMatcher
+static StatementMatcher
makeReinitMatcher(const ValueDecl *MovedVariable,
llvm::ArrayRef<StringRef> InvalidationFunctions) {
const auto DeclRefMatcher =
declRefExpr(hasDeclaration(equalsNode(MovedVariable))).bind("declref");
- static const auto StandardContainerTypeMatcher =
- hasType(hasUnqualifiedDesugaredType(
- recordType(hasDeclaration(cxxRecordDecl(hasAnyName(
- "::std::basic_string", "::std::vector", "::std::deque",
- "::std::forward_list", "::std::list", "::std::set", "::std::map",
- "::std::multiset", "::std::multimap", "::std::unordered_set",
- "::std::unordered_map", "::std::unordered_multiset",
- "::std::unordered_multimap"))))));
+ const auto StandardContainerTypeMatcher = hasType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(cxxRecordDecl(hasAnyName(
+ "::std::basic_string", "::std::vector", "::std::deque",
+ "::std::forward_list", "::std::list", "::std::set", "::std::map",
+ "::std::multiset", "::std::multimap", "::std::unordered_set",
+ "::std::unordered_map", "::std::unordered_multiset",
+ "::std::unordered_multimap"))))));
- static const auto StandardResettableOwnerTypeMatcher = hasType(
+ const auto StandardResettableOwnerTypeMatcher = hasType(
hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
hasAnyName("::std::unique_ptr", "::std::shared_ptr",
"::std::weak_ptr", "::std::optional", "::std::any"))))));
@@ -117,7 +164,6 @@ makeReinitMatcher(const ValueDecl *MovedVariable,
.bind("reinit");
}
-
bool isVariableResetInLambda(const Stmt *Body, const ValueDecl *MovedVariable,
ASTContext *Context,
llvm::ArrayRef<StringRef> InvalidationFunctions) {
@@ -139,57 +185,8 @@ bool isVariableResetInLambda(const Stmt *Body, const ValueDecl *MovedVariable,
return !match(findAll(ReinitMatcher), *Body, *Context).empty();
}
-/// Contains information about a use-after-move.
-struct UseAfterMove {
- // The DeclRefExpr that constituted the use of the object.
- const DeclRefExpr *DeclRef;
-
- // Is the order in which the move and the use are evaluated undefined?
- bool EvaluationOrderUndefined = false;
-
- // Does the use happen in a later loop iteration than the move?
- //
- // We default to false and change it to true if required in find().
- bool UseHappensInLaterLoopIteration = false;
-};
-
-/// Finds uses of a variable after a move (and maintains state required by the
-/// various internal helper functions).
-class UseAfterMoveFinder {
-public:
- UseAfterMoveFinder(ASTContext *TheContext,
- llvm::ArrayRef<StringRef> InvalidationFunctions);
-
- // Within the given code block, finds the first use of 'MovedVariable' that
- // occurs after 'MovingCall' (the expression that performs the move). If a
- // use-after-move is found, writes information about it to 'TheUseAfterMove'.
- // Returns whether a use-after-move was found.
- std::optional<UseAfterMove> find(Stmt *CodeBlock, const Expr *MovingCall,
- const DeclRefExpr *MovedVariable);
-
-private:
- std::optional<UseAfterMove> findInternal(const CFGBlock *Block,
- const Expr *MovingCall,
- const ValueDecl *MovedVariable);
- void getUsesAndReinits(const CFGBlock *Block, const ValueDecl *MovedVariable,
- llvm::SmallVectorImpl<const DeclRefExpr *> *Uses,
- llvm::SmallPtrSetImpl<const Stmt *> *Reinits);
- void getDeclRefs(const CFGBlock *Block, const Decl *MovedVariable,
- llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs);
- void getReinits(const CFGBlock *Block, const ValueDecl *MovedVariable,
- llvm::SmallPtrSetImpl<const Stmt *> *Stmts,
- llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs);
-
- ASTContext *Context;
- llvm::ArrayRef<StringRef> InvalidationFunctions;
- std::unique_ptr<ExprSequence> Sequence;
- std::unique_ptr<StmtToBlockMap> BlockMap;
- llvm::SmallPtrSet<const CFGBlock *, 8> Visited;
-};
-
} // namespace
-
// 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
>From 63d18cddec02015bb399da487a136dee4a335d8e Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Sun, 21 Dec 2025 22:29:45 +0800
Subject: [PATCH 03/13] Add testcases
---
.../checkers/bugprone/use-after-move.cpp | 109 ++++++++++++++++++
1 file changed, 109 insertions(+)
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 b2df2638106e0..b1ed032cbb27b 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
@@ -1703,3 +1703,112 @@ void Run() {
}
} // namespace custom_invalidation
+
+void lambdaReinit() {
+ {
+ std::string s;
+ auto g = [&]() {
+ s = std::string();
+ return true;
+ };
+ for (int i = 0; i < 10; ++i) {
+ if (g()) {
+ if (!s.empty()) {
+ std::move(s);
+ }
+ }
+ }
+ }
+
+ {
+ std::string s;
+ auto g = [&]() {
+ s.clear();
+ return true;
+ };
+ for (int i = 0; i < 10; ++i) {
+ if (g()) {
+ if (!s.empty()) {
+ std::move(s);
+ }
+ }
+ }
+ }
+
+ {
+ std::string s;
+ auto g = [&]() {
+ s.assign(10, 'a');
+ return true;
+ };
+ for (int i = 0; i < 10; ++i) {
+ if (g()) {
+ if (!s.empty()) {
+ std::move(s);
+ }
+ }
+ }
+ }
+
+ {
+ std::string s;
+ auto g = [&]() {
+ return !s.empty();
+ };
+ for (int i = 0; i < 10; ++i) {
+ if (g()) {
+ std::move(s);
+ // CHECK-NOTES: [[@LINE-1]]:19: warning: 's' used after it was moved
+ // CHECK-NOTES: [[@LINE-2]]:9: note: move occurred here
+ // CHECK-NOTES: [[@LINE-3]]:19: note: the use happens in a later loop
+ }
+ }
+ }
+
+ {
+ std::string s;
+ auto g = [s]() mutable {
+ s = std::string();
+ return true;
+ };
+ for (int i = 0; i < 10; ++i) {
+ if (g()) {
+ std::move(s);
+ // CHECK-NOTES: [[@LINE-1]]:19: warning: 's' used after it was moved
+ // CHECK-NOTES: [[@LINE-2]]:9: note: move occurred here
+ // CHECK-NOTES: [[@LINE-3]]:19: note: the use happens in a later loop
+ }
+ }
+ }
+
+ {
+ std::string s;
+ while ([&]() { s = std::string(); return true; }()) {
+ // CHECK-NOTES: [[@LINE-1]]:13: warning: 's' used after it was moved
+ // CHECK-NOTES: [[@LINE+3]]:9: note: move occurred here
+ // CHECK-NOTES: [[@LINE-3]]:13: note: the use happens in a later loop
+ if (!s.empty()) {
+ std::move(s);
+ }
+ }
+ }
+}
+
+void lambdaReinitInDeadCode() {
+ // FIXME: This is a known False Negative. The check currently
+ // lacks the ability to do control flow analysis.
+ {
+ std::string s;
+ auto g = [&]() {
+ if (false) {
+ s = std::string();
+ }
+ };
+ std::move(s);
+
+ g();
+
+ s.clear();
+ // CHECK-NOTES-NOT: [[@LINE-2]]:5: warning: 's' used after it was moved
+ }
+}
>From 3180161c6d1edee95831c4d668698907948c1f89 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Sun, 21 Dec 2025 22:41:53 +0800
Subject: [PATCH 04/13] Make clang-tidy happy
---
.../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index cd4bf400d3985..1b58f83267d26 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -86,11 +86,12 @@ AST_MATCHER_P(CXXRecordDecl, hasCaptureByReference, const ValueDecl *,
if (!Node.isLambda())
return false;
- for (const auto &Capture : Node.captures()) {
- if (Capture.capturesVariable() && Capture.getCaptureKind() == LCK_ByRef &&
- Capture.getCapturedVar() == TargetDecl) {
- return true;
- }
+ if (llvm::any_of(Node.captures(), [&](const auto &Capture) {
+ return Capture.capturesVariable() &&
+ Capture.getCaptureKind() == LCK_ByRef &&
+ Capture.getCapturedVar() == TargetDecl;
+ })) {
+ return true;
}
return false;
}
@@ -164,9 +165,10 @@ makeReinitMatcher(const ValueDecl *MovedVariable,
.bind("reinit");
}
-bool isVariableResetInLambda(const Stmt *Body, const ValueDecl *MovedVariable,
- ASTContext *Context,
- llvm::ArrayRef<StringRef> InvalidationFunctions) {
+static bool
+isVariableResetInLambda(const Stmt *Body, const ValueDecl *MovedVariable,
+ ASTContext *Context,
+ llvm::ArrayRef<StringRef> InvalidationFunctions) {
if (!Body)
return false;
>From 546025e1363bf090eca7c87ab84a479900f1f374 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Sun, 21 Dec 2025 22:46:29 +0800
Subject: [PATCH 05/13] Remove unnecessary headers
---
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 1b58f83267d26..1d6104025ad6f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -8,10 +8,8 @@
#include "UseAfterMoveCheck.h"
-#include "clang/AST/DeclCXX.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
-#include "clang/AST/LambdaCapture.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h"
#include "clang/Analysis/CFG.h"
>From 92f1cbf83ec2b2de6daf377051a0288c7e2d384f Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Sun, 21 Dec 2025 23:30:36 +0800
Subject: [PATCH 06/13] ~
---
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 1d6104025ad6f..14772c3e8188d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -94,6 +94,8 @@ AST_MATCHER_P(CXXRecordDecl, hasCaptureByReference, const ValueDecl *,
return false;
}
+} // namespace
+
static auto getNameMatcher(llvm::ArrayRef<StringRef> InvalidationFunctions) {
return anyOf(hasAnyName("::std::move", "::std::forward"),
matchers::matchesAnyListedName(InvalidationFunctions));
@@ -185,8 +187,6 @@ isVariableResetInLambda(const Stmt *Body, const ValueDecl *MovedVariable,
return !match(findAll(ReinitMatcher), *Body, *Context).empty();
}
-} // namespace
-
// 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
>From 7ffa746a850680cf5b5628a8a07699da9adb20da Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Mon, 22 Dec 2025 00:05:52 +0800
Subject: [PATCH 07/13] Add CFG analysis for lambdas
---
.../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 36 ++++++++++++++++++-
.../checkers/bugprone/use-after-move.cpp | 7 ++--
2 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 14772c3e8188d..440317bb7d65c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -181,10 +181,44 @@ isVariableResetInLambda(const Stmt *Body, const ValueDecl *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);
- return !match(findAll(ReinitMatcher), *Body, *Context).empty();
+ 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
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 b1ed032cbb27b..b03eab610bc9b 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
@@ -1795,8 +1795,6 @@ void lambdaReinit() {
}
void lambdaReinitInDeadCode() {
- // FIXME: This is a known False Negative. The check currently
- // lacks the ability to do control flow analysis.
{
std::string s;
auto g = [&]() {
@@ -1808,7 +1806,8 @@ void lambdaReinitInDeadCode() {
g();
- s.clear();
- // CHECK-NOTES-NOT: [[@LINE-2]]:5: warning: 's' used after it was moved
+ s.empty();
+ // CHECK-NOTES: [[@LINE-1]]:5: warning: 's' used after it was moved
+ // CHECK-NOTES: [[@LINE-6]]:5: note: move occurred here
}
}
>From ab6eac8f8f5552bade65d7395247643498d5c986 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Mon, 22 Dec 2025 00:30:45 +0800
Subject: [PATCH 08/13] Refactor testcases
---
.../checkers/bugprone/use-after-move.cpp | 151 ++++++++----------
1 file changed, 63 insertions(+), 88 deletions(-)
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 b03eab610bc9b..4ccd8b3b5a3b5 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
@@ -1704,110 +1704,85 @@ void Run() {
} // namespace custom_invalidation
-void lambdaReinit() {
- {
- std::string s;
- auto g = [&]() {
- s = std::string();
- return true;
- };
- for (int i = 0; i < 10; ++i) {
- if (g()) {
- if (!s.empty()) {
- std::move(s);
- }
- }
- }
- }
-
- {
- std::string s;
- auto g = [&]() {
- s.clear();
- return true;
- };
- for (int i = 0; i < 10; ++i) {
- if (g()) {
- if (!s.empty()) {
- std::move(s);
- }
- }
- }
- }
-
- {
- std::string s;
- auto g = [&]() {
- s.assign(10, 'a');
- return true;
- };
- for (int i = 0; i < 10; ++i) {
- if (g()) {
- if (!s.empty()) {
- std::move(s);
- }
- }
- }
- }
+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();
+}
- {
- std::string s;
- auto g = [&]() {
- return !s.empty();
- };
- for (int i = 0; i < 10; ++i) {
- if (g()) {
+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);
- // CHECK-NOTES: [[@LINE-1]]:19: warning: 's' used after it was moved
- // CHECK-NOTES: [[@LINE-2]]:9: note: move occurred here
- // CHECK-NOTES: [[@LINE-3]]:19: note: the use happens in a later loop
}
}
}
+}
+void lambdaDoesNotReinitializeVar() {
{
std::string s;
- auto g = [s]() mutable {
- s = std::string();
- return true;
- };
- for (int i = 0; i < 10; ++i) {
- if (g()) {
- std::move(s);
- // CHECK-NOTES: [[@LINE-1]]:19: warning: 's' used after it was moved
- // CHECK-NOTES: [[@LINE-2]]:9: note: move occurred here
- // CHECK-NOTES: [[@LINE-3]]:19: note: the use happens in a later loop
- }
- }
+ 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;
- while ([&]() { s = std::string(); return true; }()) {
- // CHECK-NOTES: [[@LINE-1]]:13: warning: 's' used after it was moved
- // CHECK-NOTES: [[@LINE+3]]:9: note: move occurred here
- // CHECK-NOTES: [[@LINE-3]]:13: note: the use happens in a later loop
- if (!s.empty()) {
- std::move(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);
+ std::string s;
+ auto g = [&]() {
+ if (false) {
+ s = std::string();
+ }
+ };
+ std::move(s);
- g();
+ g();
- s.empty();
- // CHECK-NOTES: [[@LINE-1]]:5: warning: 's' used after it was moved
- // CHECK-NOTES: [[@LINE-6]]:5: note: move occurred here
- }
+ s.empty();
+ // CHECK-NOTES: [[@LINE-1]]:3: warning: 's' used after it was moved
+ // CHECK-NOTES: [[@LINE-6]]:3: note: move occurred here
}
>From cbe4dd171ddd8b5f1a4dc6ca9f516280d605fcd4 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Mon, 22 Dec 2025 00:36:10 +0800
Subject: [PATCH 09/13] Add documentation
---
clang-tools-extra/docs/ReleaseNotes.rst | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 924b2c03cfd18..aa9bb50dec198 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/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
>From ee11a0c22674a61fe28d57be415eed310f2cbfa5 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Mon, 22 Dec 2025 00:47:05 +0800
Subject: [PATCH 10/13] Use assert
---
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 440317bb7d65c..ecb063e6fe1ce 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -169,8 +169,7 @@ static bool
isVariableResetInLambda(const Stmt *Body, const ValueDecl *MovedVariable,
ASTContext *Context,
llvm::ArrayRef<StringRef> InvalidationFunctions) {
- if (!Body)
- return false;
+ assert(Body && "There should be a lambda body");
// If the variable is not mentioned at all in the lambda body,
// it cannot be reinitialized.
>From 8f2eecaa640c2fa66a8408829d123cd65edf39d9 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Mon, 22 Dec 2025 18:52:48 +0800
Subject: [PATCH 11/13] Address Review Feedback :)
---
.../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 37 ++++++++-----------
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index ecb063e6fe1ce..3db663f8f9c65 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -81,17 +81,12 @@ class UseAfterMoveFinder {
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;
+ return Node.isLambda() &&
+ llvm::any_of(Node.captures(), [&](const auto &Capture) {
+ return Capture.capturesVariable() &&
+ Capture.getCaptureKind() == LCK_ByRef &&
+ Capture.getCapturedVar() == TargetDecl;
+ });
}
} // namespace
@@ -173,9 +168,9 @@ isVariableResetInLambda(const Stmt *Body, const ValueDecl *MovedVariable,
// 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))))));
+ const auto VariableMentionMatcher =
+ stmt(hasDescendant(mapAnyOf(declRefExpr, memberExpr)
+ .with(hasDeclaration(equalsNode(MovedVariable)))));
if (match(VariableMentionMatcher, *Body, *Context).empty())
return false;
@@ -186,7 +181,7 @@ isVariableResetInLambda(const Stmt *Body, const ValueDecl *MovedVariable,
Options.PruneTriviallyFalseEdges = true;
std::unique_ptr<CFG> TheCFG =
- CFG::buildCFG(nullptr, const_cast<Stmt *>(Body), Context, Options);
+ CFG::buildCFG(/*D=*/nullptr, const_cast<Stmt *>(Body), Context, Options);
if (!TheCFG)
return false;
@@ -484,15 +479,15 @@ void UseAfterMoveFinder::getReinits(
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;
+ assert(Operator && "The lambda call should be a CXXOperatorCallExpr");
+
+ if (BlockMap->blockContainingStmt(Operator) == Block) {
+ const auto *MD = cast<CXXMethodDecl>(Operator->getDirectCallee());
const auto *RD = MD->getParent();
const auto *LambdaBody = MD->getBody();
- if (RD && RD->isLambda() && LambdaBody &&
+ assert(RD && RD->isLambda());
+ if (LambdaBody &&
isVariableResetInLambda(LambdaBody, MovedVariable, Context,
InvalidationFunctions))
Stmts->insert(Operator);
>From 6549c2df75c878db2bf701c9b568aab20d61db92 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Mon, 22 Dec 2025 19:15:28 +0800
Subject: [PATCH 12/13] Make clang-tidy happy again
---
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 3db663f8f9c65..7008626a84563 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -484,9 +484,8 @@ void UseAfterMoveFinder::getReinits(
if (BlockMap->blockContainingStmt(Operator) == Block) {
const auto *MD = cast<CXXMethodDecl>(Operator->getDirectCallee());
- const auto *RD = MD->getParent();
const auto *LambdaBody = MD->getBody();
- assert(RD && RD->isLambda());
+ assert(MD->getParent() && MD->getParent()->isLambda());
if (LambdaBody &&
isVariableResetInLambda(LambdaBody, MovedVariable, Context,
InvalidationFunctions))
>From 438fe67629ffd9abe99c7f881a1299f0826059ed Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Tue, 23 Dec 2025 15:02:12 +0800
Subject: [PATCH 13/13] Fix testcases error caused by merging
---
.../test/clang-tidy/checkers/bugprone/use-after-move.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 b841d4f3162c1..d8e0e7e253722 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
@@ -1770,7 +1770,7 @@ void lambdaReinitConditional() {
};
std::move(s);
reinit(true);
- s.empty();plicit
+ s.empty();
}
void lambdaReinitInDeadCode() {
More information about the cfe-commits
mailing list