https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/172219
>From b1bca2439ca86b2593df1917cc36def691389662 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Mon, 15 Dec 2025 01:03:53 +0800 Subject: [PATCH 1/2] [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 <[email protected]> Date: Mon, 15 Dec 2025 09:10:01 +0800 Subject: [PATCH 2/2] 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
