[clang-tools-extra] [clang-tidy][NFC] Refactor `bugprone-use-after-move` check (PR #172219)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Dec 14 09:11:47 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: mitchell (zeyi2)
<details>
<summary>Changes</summary>
This change is a necessary step for a subsequent PR that will enhance the `bugprone-use-after-move` check to correctly handle cases where variables are re-initialized inside captured lambdas, which currently lead to FPs.
Part of #<!-- -->172018
---
Full diff: https://github.com/llvm/llvm-project/pull/172219.diff
1 Files Affected:
- (modified) clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp (+72-62)
``````````diff
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();
``````````
</details>
https://github.com/llvm/llvm-project/pull/172219
More information about the cfe-commits
mailing list