[clang-tools-extra] [clang-tidy][NFC] Refactor `bugprone-use-after-move` check (PR #172219)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 16 04:23:42 PST 2025
https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/172219
>From b1bca2439ca86b2593df1917cc36def691389662 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Mon, 15 Dec 2025 01:03:53 +0800
Subject: [PATCH 1/5] [clang-tidy][NFC] Refactor `bugprone-use-after-move`
check
---
.../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 134 ++++++++++--------
1 file changed, 72 insertions(+), 62 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index b2e08fe688a1b..295453b093ee6 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -31,6 +31,76 @@ using matchers::hasUnevaluatedContext;
namespace {
+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");
+}
+
/// Contains information about a use-after-move.
struct UseAfterMove {
// The DeclRefExpr that constituted the use of the object.
@@ -81,11 +151,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 +378,8 @@ 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"))))));
-
- 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");
+ const auto ReinitMatcher =
+ makeReinitMatcher(MovedVariable, InvalidationFunctions);
Stmts->clear();
DeclRefs->clear();
>From a26cf439f296deeb702ba3119e8eff0f1f029fba Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Mon, 15 Dec 2025 09:10:01 +0800
Subject: [PATCH 2/5] Fix
---
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 295453b093ee6..905d0a4d8c26e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -29,14 +29,12 @@ namespace clang::tidy::bugprone {
using matchers::hasUnevaluatedContext;
-namespace {
-
static auto getNameMatcher(llvm::ArrayRef<StringRef> InvalidationFunctions) {
return anyOf(hasAnyName("::std::move", "::std::forward"),
matchers::matchesAnyListedName(InvalidationFunctions));
}
-StatementMatcher
+static StatementMatcher
makeReinitMatcher(const ValueDecl *MovedVariable,
llvm::ArrayRef<StringRef> InvalidationFunctions) {
const auto DeclRefMatcher =
@@ -149,8 +147,6 @@ class UseAfterMoveFinder {
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 fe00972aaf3bac1716422cf5cbbf15ad8abb7439 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Tue, 16 Dec 2025 18:33:08 +0800
Subject: [PATCH 3/5] Remove static
---
.../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 905d0a4d8c26e..5c5be8473eb9f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -40,16 +40,15 @@ makeReinitMatcher(const ValueDecl *MovedVariable,
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(
+ 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 StandardResettableOwnerTypeMatcher = hasType(
hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
hasAnyName("::std::unique_ptr", "::std::shared_ptr",
"::std::weak_ptr", "::std::optional", "::std::any"))))));
>From 7c987fa813569fd921ceb540f8b681f47576fd5a Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Tue, 16 Dec 2025 20:09:35 +0800
Subject: [PATCH 4/5] Fix
---
.../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 105 +++++++++---------
1 file changed, 54 insertions(+), 51 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 5c5be8473eb9f..ec7f6a3886953 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -29,10 +29,56 @@ namespace clang::tidy::bugprone {
using matchers::hasUnevaluatedContext;
-static auto getNameMatcher(llvm::ArrayRef<StringRef> InvalidationFunctions) {
- return anyOf(hasAnyName("::std::move", "::std::forward"),
- matchers::matchesAnyListedName(InvalidationFunctions));
-}
+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;
+};
+} // namespace
static StatementMatcher
makeReinitMatcher(const ValueDecl *MovedVariable,
@@ -98,53 +144,10 @@ makeReinitMatcher(const ValueDecl *MovedVariable,
.bind("reinit");
}
-/// 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;
-};
+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
>From bca47c617f8a74f0f2af0ef4e5b15fd474fce49e Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Tue, 16 Dec 2025 20:23:09 +0800
Subject: [PATCH 5/5] Fix declaration order
---
.../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index ec7f6a3886953..3b8be912f6e22 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -78,8 +78,14 @@ class UseAfterMoveFinder {
std::unique_ptr<StmtToBlockMap> BlockMap;
llvm::SmallPtrSet<const CFGBlock *, 8> Visited;
};
+
} // namespace
+static auto getNameMatcher(llvm::ArrayRef<StringRef> InvalidationFunctions) {
+ return anyOf(hasAnyName("::std::move", "::std::forward"),
+ matchers::matchesAnyListedName(InvalidationFunctions));
+}
+
static StatementMatcher
makeReinitMatcher(const ValueDecl *MovedVariable,
llvm::ArrayRef<StringRef> InvalidationFunctions) {
@@ -144,11 +150,6 @@ makeReinitMatcher(const ValueDecl *MovedVariable,
.bind("reinit");
}
-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
More information about the cfe-commits
mailing list