https://github.com/higher-performance created https://github.com/llvm/llvm-project/pull/170346
None >From 8fbbf33787ff20fdc10df800e6b8c95935f493c7 Mon Sep 17 00:00:00 2001 From: higher-performance <[email protected]> Date: Tue, 2 Dec 2025 13:20:24 -0500 Subject: [PATCH] Extend bugprone-use-after-move check to allow custom invalidation functions --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 64 +++++++++++++------ .../clang-tidy/bugprone/UseAfterMoveCheck.h | 7 +- .../checkers/bugprone/use-after-move.cpp | 42 +++++++++++- 3 files changed, 89 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index efb5ec64689cf..404fc4de8ef3c 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -19,6 +19,7 @@ #include "../utils/ExprSequence.h" #include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" #include <optional> using namespace clang::ast_matchers; @@ -48,7 +49,8 @@ struct UseAfterMove { /// various internal helper functions). class UseAfterMoveFinder { public: - UseAfterMoveFinder(ASTContext *TheContext); + 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 @@ -71,11 +73,17 @@ class UseAfterMoveFinder { 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)); +} + } // namespace // Matches nodes that are @@ -92,8 +100,9 @@ static StatementMatcher inDecltypeOrTemplateArg() { hasAncestor(expr(hasUnevaluatedContext()))); } -UseAfterMoveFinder::UseAfterMoveFinder(ASTContext *TheContext) - : Context(TheContext) {} +UseAfterMoveFinder::UseAfterMoveFinder( + ASTContext *TheContext, llvm::ArrayRef<StringRef> InvalidationFunctions) + : Context(TheContext), InvalidationFunctions(InvalidationFunctions) {} std::optional<UseAfterMove> UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall, @@ -359,7 +368,7 @@ void UseAfterMoveFinder::getReinits( unless(parmVarDecl(hasType( references(qualType(isConstQualified())))))), unless(callee(functionDecl( - hasAnyName("::std::move", "::std::forward"))))))) + getNameMatcher(InvalidationFunctions))))))) .bind("reinit"); Stmts->clear(); @@ -389,8 +398,9 @@ void UseAfterMoveFinder::getReinits( } enum class MoveType { - Move, // std::move - Forward, // std::forward + Forward, // std::forward + Move, // std::move + Invalidation, // other }; static MoveType determineMoveType(const FunctionDecl *FuncDecl) { @@ -399,7 +409,7 @@ static MoveType determineMoveType(const FunctionDecl *FuncDecl) { if (FuncDecl->getName() == "forward") return MoveType::Forward; - llvm_unreachable("Invalid move type"); + return MoveType::Invalidation; } static void emitDiagnostic(const Expr *MovingCall, const DeclRefExpr *MoveArg, @@ -408,29 +418,40 @@ static void emitDiagnostic(const Expr *MovingCall, const DeclRefExpr *MoveArg, const SourceLocation UseLoc = Use.DeclRef->getExprLoc(); const SourceLocation MoveLoc = MovingCall->getExprLoc(); - const bool IsMove = (Type == MoveType::Move); + const int Kind = static_cast<int>(Type); - Check->diag(UseLoc, "'%0' used after it was %select{forwarded|moved}1") - << MoveArg->getDecl()->getName() << IsMove; - Check->diag(MoveLoc, "%select{forward|move}0 occurred here", + Check->diag(UseLoc, + "'%0' used after it was %select{forwarded|moved|invalidated}1") + << MoveArg->getDecl()->getName() << Kind; + Check->diag(MoveLoc, "%select{forward|move|invalidation}0 occurred here", DiagnosticIDs::Note) - << IsMove; + << Kind; if (Use.EvaluationOrderUndefined) { Check->diag( UseLoc, - "the use and %select{forward|move}0 are unsequenced, i.e. " + "the use and %select{forward|move|invalidation}0 are unsequenced, i.e. " "there is no guarantee about the order in which they are evaluated", DiagnosticIDs::Note) - << IsMove; + << Kind; } else if (Use.UseHappensInLaterLoopIteration) { Check->diag(UseLoc, "the use happens in a later loop iteration than the " - "%select{forward|move}0", + "%select{forward|move|invalidation}0", DiagnosticIDs::Note) - << IsMove; + << Kind; } } +UseAfterMoveCheck::UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + InvalidationFunctions(utils::options::parseStringList( + Options.get("InvalidationFunctions", ""))) {} + +void UseAfterMoveCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "InvalidationFunctions", + utils::options::serializeStringList(InvalidationFunctions)); +} + void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) { // try_emplace is a common maybe-moving function that returns a // bool to tell callers whether it moved. Ignore std::move inside @@ -438,11 +459,14 @@ void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) { // the bool. auto TryEmplaceMatcher = cxxMemberCallExpr(callee(cxxMethodDecl(hasName("try_emplace")))); + auto Arg = declRefExpr().bind("arg"); + auto IsMemberCallee = callee(functionDecl(unless(isStaticStorageClass()))); auto CallMoveMatcher = - callExpr(argumentCountIs(1), - callee(functionDecl(hasAnyName("::std::move", "::std::forward")) + callExpr(callee(functionDecl(getNameMatcher(InvalidationFunctions)) .bind("move-decl")), - hasArgument(0, declRefExpr().bind("arg")), + anyOf(cxxMemberCallExpr(IsMemberCallee, on(Arg)), + callExpr(unless(cxxMemberCallExpr(IsMemberCallee)), + hasArgument(0, Arg))), unless(inDecltypeOrTemplateArg()), unless(hasParent(TryEmplaceMatcher)), expr().bind("call-move"), anyOf(hasAncestor(compoundStmt( @@ -521,7 +545,7 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) { } for (Stmt *CodeBlock : CodeBlocks) { - UseAfterMoveFinder Finder(Result.Context); + UseAfterMoveFinder Finder(Result.Context, InvalidationFunctions); 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 d38b29e09fa8b..1bbf5c00785ff 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h @@ -20,13 +20,16 @@ namespace clang::tidy::bugprone { /// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/use-after-move.html class UseAfterMoveCheck : public ClangTidyCheck { public: - UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus11; } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + std::vector<StringRef> InvalidationFunctions; }; } // namespace clang::tidy::bugprone 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 87dfec4f68061..57920f3bd7657 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,5 +1,5 @@ -// RUN: %check_clang_tidy -std=c++11 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- -- -fno-delayed-template-parsing -// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -std=c++11 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- -config="{CheckOptions: {bugprone-use-after-move.InvalidationFunctions: 'Database::StaticCloseConnection;Database::CloseConnection;FriendCloseConnection'}}" -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-use-after-move %t -- -config="{CheckOptions: {bugprone-use-after-move.InvalidationFunctions: 'Database::StaticCloseConnection;Database::CloseConnection;FriendCloseConnection'}}" -- -fno-delayed-template-parsing typedef decltype(nullptr) nullptr_t; @@ -1645,3 +1645,41 @@ void create() { } } // namespace issue82023 + +namespace custom_invalidation +{ + +struct Database { + void CloseConnection(); + static void StaticCloseConnection(Database&); + friend void FriendCloseConnection(Database&); + void Query(); +}; + +void Run() { + Database db1; + db1.CloseConnection(); + db1.Query(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'db1' used after it was invalidated + // CHECK-NOTES: [[@LINE-3]]:7: note: invalidation occurred here + + Database db2; + Database::StaticCloseConnection(db2); + db2.Query(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'db2' used after it was invalidated + // CHECK-NOTES: [[@LINE-3]]:3: note: invalidation occurred here + + Database db3; + Database().StaticCloseConnection(db3); + db3.Query(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'db3' used after it was invalidated + // CHECK-NOTES: [[@LINE-3]]:3: note: invalidation occurred here + + Database db4; + FriendCloseConnection(db4); + db4.Query(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'db4' used after it was invalidated + // CHECK-NOTES: [[@LINE-3]]:3: note: invalidation occurred here +} + +} // namespace custom_invalidation _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
