https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/172784
>From 8ba6d09854cb8f245fdbd5f2b2aad5c6ee23e829 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Wed, 17 Dec 2025 23:35:35 +0800 Subject: [PATCH 1/3] [clang-tidy] Add `ReinitializationFunctions` option to `bugprone-use-after-move` --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 24 +++++++++++--- .../clang-tidy/bugprone/UseAfterMoveCheck.h | 1 + clang-tools-extra/docs/ReleaseNotes.rst | 4 ++- .../checks/bugprone/use-after-move.rst | 6 ++++ .../checkers/bugprone/use-after-move.cpp | 32 +++++++++++++++++-- 5 files changed, 59 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index b2e08fe688a1b..3d74df4725577 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -50,7 +50,8 @@ struct UseAfterMove { class UseAfterMoveFinder { public: UseAfterMoveFinder(ASTContext *TheContext, - llvm::ArrayRef<StringRef> InvalidationFunctions); + llvm::ArrayRef<StringRef> InvalidationFunctions, + llvm::ArrayRef<StringRef> ReinitializationFunctions); // Within the given code block, finds the first use of 'MovedVariable' that // occurs after 'MovingCall' (the expression that performs the move). If a @@ -74,6 +75,7 @@ class UseAfterMoveFinder { ASTContext *Context; llvm::ArrayRef<StringRef> InvalidationFunctions; + llvm::ArrayRef<StringRef> ReinitializationFunctions; std::unique_ptr<ExprSequence> Sequence; std::unique_ptr<StmtToBlockMap> BlockMap; llvm::SmallPtrSet<const CFGBlock *, 8> Visited; @@ -101,8 +103,10 @@ static StatementMatcher inDecltypeOrTemplateArg() { } UseAfterMoveFinder::UseAfterMoveFinder( - ASTContext *TheContext, llvm::ArrayRef<StringRef> InvalidationFunctions) - : Context(TheContext), InvalidationFunctions(InvalidationFunctions) {} + ASTContext *TheContext, llvm::ArrayRef<StringRef> InvalidationFunctions, + llvm::ArrayRef<StringRef> ReinitializationFunctions) + : Context(TheContext), InvalidationFunctions(InvalidationFunctions), + ReinitializationFunctions(ReinitializationFunctions) {} std::optional<UseAfterMove> UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall, @@ -356,6 +360,11 @@ void UseAfterMoveFinder::getReinits( cxxMemberCallExpr( on(DeclRefMatcher), callee(cxxMethodDecl(hasAttr(clang::attr::Reinitializes)))), + // Methods that are specified in ReinitializationFunctions option. + cxxMemberCallExpr( + on(DeclRefMatcher), + callee(cxxMethodDecl( + matchers::matchesAnyListedName(ReinitializationFunctions)))), // Passing variable to a function as a non-const pointer. callExpr(forEachArgumentWithParam( unaryOperator(hasOperatorName("&"), @@ -445,11 +454,15 @@ static void emitDiagnostic(const Expr *MovingCall, const DeclRefExpr *MoveArg, UseAfterMoveCheck::UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), InvalidationFunctions(utils::options::parseStringList( - Options.get("InvalidationFunctions", ""))) {} + Options.get("InvalidationFunctions", ""))), + ReinitializationFunctions(utils::options::parseStringList( + Options.get("ReinitializationFunctions", ""))) {} void UseAfterMoveCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "InvalidationFunctions", utils::options::serializeStringList(InvalidationFunctions)); + Options.store(Opts, "ReinitializationFunctions", + utils::options::serializeStringList(ReinitializationFunctions)); } void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) { @@ -545,7 +558,8 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) { } for (Stmt *CodeBlock : CodeBlocks) { - UseAfterMoveFinder Finder(Result.Context, InvalidationFunctions); + UseAfterMoveFinder Finder(Result.Context, InvalidationFunctions, + ReinitializationFunctions); if (auto Use = Finder.find(CodeBlock, MovingCall, Arg)) emitDiagnostic(MovingCall, Arg, *Use, this, Result.Context, determineMoveType(MoveDecl)); diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h index 1bbf5c00785ff..fff1c2621867d 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h @@ -30,6 +30,7 @@ class UseAfterMoveCheck : public ClangTidyCheck { private: std::vector<StringRef> InvalidationFunctions; + std::vector<StringRef> ReinitializationFunctions; }; } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 924b2c03cfd18..fc3246adf9633 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -446,7 +446,9 @@ 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 `ReinitializationFunctions` option to support custom reinitialization + functions. - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst index 77424d3d620bb..bb8d0631f2673 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst @@ -263,3 +263,9 @@ Options arguments to be invalidated (e.g., closing a handle). For member functions, the first argument is considered to be the implicit object argument (``this``). Default value is an empty string. + +.. option:: ReinitializationFunctions + + A semicolon-separated list of names of functions that reinitialize the + object. For member functions, the implicit object argument (``this``) is + considered to be reinitialized. Default value is an empty string. 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..11e242ef9ab35 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 @@ -1,11 +1,13 @@ // RUN: %check_clang_tidy -std=c++11 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- \ // RUN: -config='{CheckOptions: { \ -// RUN: bugprone-use-after-move.InvalidationFunctions: "::Database<>::StaticCloseConnection;Database<>::CloseConnection;FriendCloseConnection" \ +// RUN: bugprone-use-after-move.InvalidationFunctions: "::Database<>::StaticCloseConnection;Database<>::CloseConnection;FriendCloseConnection", \ +// RUN: bugprone-use-after-move.ReinitializationFunctions: "::custom_reinitialization::Database<>::Reset" \ // RUN: }}' -- \ // RUN: -fno-delayed-template-parsing // RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- \ // RUN: -config='{CheckOptions: { \ -// RUN: bugprone-use-after-move.InvalidationFunctions: "::Database<>::StaticCloseConnection;Database<>::CloseConnection;FriendCloseConnection" \ +// RUN: bugprone-use-after-move.InvalidationFunctions: "::Database<>::StaticCloseConnection;Database<>::CloseConnection;FriendCloseConnection", \ +// RUN: bugprone-use-after-move.ReinitializationFunctions: "::custom_reinitialization::Database<>::Reset" \ // RUN: }}' -- \ // RUN: -fno-delayed-template-parsing @@ -1703,3 +1705,29 @@ void Run() { } } // namespace custom_invalidation + +namespace custom_reinitialization { + +template <class T = int> +struct Database { + void Reset(T = T()) {} + void Query(T = T()) {} +}; + +void Run() { + using DB = Database<>; + + DB db1; + std::move(db1); + db1.Reset(); + db1.Query(); + + DB db2; + std::move(db2); + db2.Query(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'db2' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here + db2.Reset(); +} + +} // namespace custom_reinitialization >From e1d22ed9c0999d1c9cf4b68ac73da5ff692e24f5 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Thu, 18 Dec 2025 11:45:23 +0800 Subject: [PATCH 2/3] Make clang-format happy --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 87 ++++++++++--------- 1 file changed, 44 insertions(+), 43 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 3d74df4725577..83b0f65da1e53 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -335,49 +335,50 @@ void UseAfterMoveFinder::getReinits( // 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)))), - // Methods that are specified in ReinitializationFunctions option. - cxxMemberCallExpr( - on(DeclRefMatcher), - callee(cxxMethodDecl( - matchers::matchesAnyListedName(ReinitializationFunctions)))), - // 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))))))) + 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)))), + // Methods that are specified in ReinitializationFunctions option. + cxxMemberCallExpr( + on(DeclRefMatcher), + callee(cxxMethodDecl(matchers::matchesAnyListedName( + ReinitializationFunctions)))), + // 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"); Stmts->clear(); >From 23c4b1964428c5dfbfc8ae06f0d586a881bb2f7b Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Thu, 18 Dec 2025 21:08:14 +0800 Subject: [PATCH 3/3] Support non-member functions --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 89 ++++++++++--------- .../checks/bugprone/use-after-move.rst | 6 +- .../checkers/bugprone/use-after-move.cpp | 27 +++++- 3 files changed, 72 insertions(+), 50 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 83b0f65da1e53..f853c3726e298 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -335,50 +335,51 @@ void UseAfterMoveFinder::getReinits( // 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)))), - // Methods that are specified in ReinitializationFunctions option. - cxxMemberCallExpr( - on(DeclRefMatcher), - callee(cxxMethodDecl(matchers::matchesAnyListedName( - ReinitializationFunctions)))), - // 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))))))) + 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)))), + // Functions that are specified in ReinitializationFunctions + // option. + callExpr(callee(functionDecl(matchers::matchesAnyListedName( + ReinitializationFunctions))), + anyOf(cxxMemberCallExpr(on(DeclRefMatcher)), + callExpr(unless(cxxMemberCallExpr()), + hasArgument(0, DeclRefMatcher)))), + // 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"); Stmts->clear(); diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst index bb8d0631f2673..95a752e9399a9 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst @@ -267,5 +267,7 @@ Options .. option:: ReinitializationFunctions A semicolon-separated list of names of functions that reinitialize the - object. For member functions, the implicit object argument (``this``) is - considered to be reinitialized. Default value is an empty string. + object. For member functions, the implicit object argument (``*this``) is + considered to be reinitialized. For non-member or static member functions, + the first argument is considered to be reinitialized. Default value is an + empty string. 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 11e242ef9ab35..d6c361fd0d0ad 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 @@ -1,13 +1,13 @@ // RUN: %check_clang_tidy -std=c++11 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- \ // RUN: -config='{CheckOptions: { \ // RUN: bugprone-use-after-move.InvalidationFunctions: "::Database<>::StaticCloseConnection;Database<>::CloseConnection;FriendCloseConnection", \ -// RUN: bugprone-use-after-move.ReinitializationFunctions: "::custom_reinitialization::Database<>::Reset" \ +// RUN: bugprone-use-after-move.ReinitializationFunctions: "::Database<>::Reset;::Database<>::StaticReset;::FriendReset" \ // RUN: }}' -- \ // RUN: -fno-delayed-template-parsing // RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- \ // RUN: -config='{CheckOptions: { \ // RUN: bugprone-use-after-move.InvalidationFunctions: "::Database<>::StaticCloseConnection;Database<>::CloseConnection;FriendCloseConnection", \ -// RUN: bugprone-use-after-move.ReinitializationFunctions: "::custom_reinitialization::Database<>::Reset" \ +// RUN: bugprone-use-after-move.ReinitializationFunctions: "::Database<>::Reset;::Database<>::StaticReset;::FriendReset" \ // RUN: }}' -- \ // RUN: -fno-delayed-template-parsing @@ -1710,7 +1710,12 @@ namespace custom_reinitialization { template <class T = int> struct Database { - void Reset(T = T()) {} + template <class... Args> + void Reset(T = T(), Args &&...) {} + template <class... Args> + static void StaticReset(Database &, T = T(), Args &&...) {} + template <class... Args> + friend void FriendReset(Database &, T = T(), Args &&...) {} void Query(T = T()) {} }; @@ -1728,6 +1733,20 @@ void Run() { // CHECK-NOTES: [[@LINE-1]]:3: warning: 'db2' used after it was moved // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here db2.Reset(); -} + DB db3; + std::move(db3); + DB::StaticReset(db3); + db3.Query(); + + DB db4; + std::move(db4); + FriendReset(db4); + db4.Query(); + + DB db5; + std::move(db5); + db5.Reset(0, 1.5, "extra"); + db5.Query(); +} } // namespace custom_reinitialization _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
