[clang-tools-extra] Extend bugprone-use-after-move check to allow custom invalidation functions (PR #170346)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 2 10:22:05 PST 2025
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 <higher.performance.github at gmail.com>
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
More information about the cfe-commits
mailing list