https://github.com/Harald-R updated https://github.com/llvm/llvm-project/pull/225804
>From e44365b2fc0b511037f27e0153356b7c6086427c Mon Sep 17 00:00:00 2001 From: Harald-R <[email protected]> Date: Tue, 4 Aug 2026 18:04:19 +0300 Subject: [PATCH] Add support for ReportAccessOnlyUseForTypes --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 93 +++++++- .../clang-tidy/bugprone/UseAfterMoveCheck.h | 1 + clang-tools-extra/docs/ReleaseNotes.md | 7 + .../checks/bugprone/use-after-move.md | 11 + .../checkers/bugprone/use-after-move.cpp | 199 +++++++++++++++++- 5 files changed, 298 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index b13367e25dcb2c..6a39591a07a9da 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -67,6 +67,7 @@ class UseAfterMoveFinder { UseAfterMoveFinder(ASTContext *TheContext, llvm::ArrayRef<StringRef> InvalidationFunctions, llvm::ArrayRef<StringRef> ReinitializationFunctions, + llvm::ArrayRef<StringRef> ReportAccessOnlyUseForTypes, const CXXRecordDecl *MovedAs); // Within the given code block, finds the first use of 'MovedVariable' that @@ -92,6 +93,7 @@ class UseAfterMoveFinder { ASTContext *Context; llvm::ArrayRef<StringRef> InvalidationFunctions; llvm::ArrayRef<StringRef> ReinitializationFunctions; + llvm::ArrayRef<StringRef> ReportAccessOnlyUseForTypes; const CXXRecordDecl *MovedAs; std::unique_ptr<ExprSequence> Sequence; std::unique_ptr<StmtToBlockMap> BlockMap; @@ -205,9 +207,12 @@ static StatementMatcher inDecltypeOrTemplateArg() { UseAfterMoveFinder::UseAfterMoveFinder( ASTContext *TheContext, llvm::ArrayRef<StringRef> InvalidationFunctions, llvm::ArrayRef<StringRef> ReinitializationFunctions, + llvm::ArrayRef<StringRef> ReportAccessOnlyUseForTypes, const CXXRecordDecl *MovedAs) : Context(TheContext), InvalidationFunctions(InvalidationFunctions), - ReinitializationFunctions(ReinitializationFunctions), MovedAs(MovedAs) {} + ReinitializationFunctions(ReinitializationFunctions), + ReportAccessOnlyUseForTypes(ReportAccessOnlyUseForTypes), + MovedAs(MovedAs) {} std::optional<UseAfterMove> UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall, @@ -404,32 +409,74 @@ static bool isSpecifiedAfterMove(const ValueDecl *VD) { return RecordDecl->getDeclContext()->isStdNamespace(); } +// Returns true if the check must report a use of `VD` only when the code +// accesses the object. An access is a member access or a dereference. The +// check does not report other references to the variable. The check does this +// when the type of `VD` is one of these: +// - a pointer to a type in the ReportAccessOnlyUseForTypes option; +// - a class that is the same as or derived from such a type. +static bool +reportAccessOnlyUse(const ValueDecl *VD, ASTContext &Context, + llvm::ArrayRef<StringRef> ReportAccessOnlyUseForTypes) { + if (ReportAccessOnlyUseForTypes.empty()) + return false; + + const auto RecordIsListed = cxxRecordDecl(isSameOrDerivedFrom( + matchers::matchesAnyListedRegexName(ReportAccessOnlyUseForTypes))); + const auto TypeMatcher = + qualType(anyOf(pointsTo(RecordIsListed), hasDeclaration(RecordIsListed))); + + const QualType QT = VD->getType().getNonReferenceType().getCanonicalType(); + return !match(TypeMatcher, QT, Context).empty(); +} + void UseAfterMoveFinder::getDeclRefs( const CFGBlock *Block, const Decl *MovedVariable, llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs) { DeclRefs->clear(); + + // For the types in the ReportAccessOnlyUseForTypes option, only an access + // counts as a use. An access is a member access or a dereference of the + // object. A different reference to the variable does not count as a use. + // Examples of a different reference are an argument, a comparison, or a copy. + const auto *MovedValueDecl = dyn_cast<ValueDecl>(MovedVariable); + const bool ReportAccessOnly = + MovedValueDecl && reportAccessOnlyUse(MovedValueDecl, *Context, + ReportAccessOnlyUseForTypes); + for (const auto &Elem : *Block) { std::optional<CFGStmt> S = Elem.getAs<CFGStmt>(); if (!S) continue; - const auto AddDeclRefs = [this, Block, - DeclRefs](const ArrayRef<BoundNodes> Matches) { + const auto AddDeclRefs = [this, Block, DeclRefs, ReportAccessOnly]( + const ArrayRef<BoundNodes> Matches) { for (const auto &Match : Matches) { const auto *DeclRef = Match.getNodeAs<DeclRefExpr>("declref"); const auto *Member = Match.getNodeAs<MemberExpr>("member-expr"); const auto *Operator = Match.getNodeAs<CXXOperatorCallExpr>("operator"); + const auto *Access = Match.getNodeAs<Expr>("access"); // Non-moved member as the move only implies a base class. if (Member && MovedAs && !isa<CXXMethodDecl>(Member->getMemberDecl()) && !MovedAs->hasMemberName(Member->getMemberDecl()->getIdentifier())) { continue; } - if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block && - (Operator || !isSpecifiedAfterMove(DeclRef->getDecl()))) - // Ignore uses of a standard smart pointer or classes annotated as - // "null_after_move" (smart-pointer-like behavior) that don't - // dereference the pointer. - DeclRefs->insert(DeclRef); + if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) { + if (ReportAccessOnly) { + // Report the use only if the code accesses the object. An access is + // one of these: + // - a member access (`op->m`, `op.m`); + // - an overloaded dereference operator; + // - a built-in dereference (`*op`) or subscript (`op[i]`). + if (Member || Operator || Access) + DeclRefs->insert(DeclRef); + } else if (Operator || !isSpecifiedAfterMove(DeclRef->getDecl())) { + // Ignore uses of a standard smart pointer, or of a class with the + // "null_after_move" annotation, that do not dereference the + // pointer. + DeclRefs->insert(DeclRef); + } + } } }; @@ -449,6 +496,24 @@ void UseAfterMoveFinder::getDeclRefs( hasArgument(0, DeclRefMatcher)) .bind("operator")), *S->getStmt(), *Context)); + // A built-in dereference (`*op`) or a subscript (`op[i]`) of the variable + // is also an access. The types in the ReportAccessOnlyUseForTypes option + // need these matchers. For example, raw pointers need them because these + // operators are not overloaded calls. Thus, the "member-expr" and + // "operator" matchers do not match these operators. + AddDeclRefs(match( + traverse(TK_AsIs, + findAll(unaryOperator(hasOperatorName("*"), + hasUnaryOperand(ignoringParenImpCasts( + DeclRefMatcher))) + .bind("access"))), + *S->getStmt(), *Context)); + AddDeclRefs(match( + traverse(TK_AsIs, + findAll(arraySubscriptExpr( + hasBase(ignoringParenImpCasts(DeclRefMatcher))) + .bind("access"))), + *S->getStmt(), *Context)); } } @@ -541,13 +606,18 @@ UseAfterMoveCheck::UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context) InvalidationFunctions(utils::options::parseStringList( Options.get("InvalidationFunctions", ""))), ReinitializationFunctions(utils::options::parseStringList( - Options.get("ReinitializationFunctions", ""))) {} + Options.get("ReinitializationFunctions", ""))), + ReportAccessOnlyUseForTypes(utils::options::parseStringList( + Options.get("ReportAccessOnlyUseForTypes", ""))) {} void UseAfterMoveCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "InvalidationFunctions", utils::options::serializeStringList(InvalidationFunctions)); Options.store(Opts, "ReinitializationFunctions", utils::options::serializeStringList(ReinitializationFunctions)); + Options.store( + Opts, "ReportAccessOnlyUseForTypes", + utils::options::serializeStringList(ReportAccessOnlyUseForTypes)); } void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) { @@ -656,7 +726,8 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) { for (Stmt *CodeBlock : CodeBlocks) { UseAfterMoveFinder Finder(Result.Context, InvalidationFunctions, - ReinitializationFunctions, MovedAs); + ReinitializationFunctions, + ReportAccessOnlyUseForTypes, MovedAs); if (auto Use = Finder.find(CodeBlock, MovingCall, Arg)) emitDiagnostic(MovingCall, Arg, *Use, this, Result.Context, determineMoveType(MoveDecl), MoveDecl); diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h index fff1c2621867db..ae7bf34511a9c8 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.h @@ -31,6 +31,7 @@ class UseAfterMoveCheck : public ClangTidyCheck { private: std::vector<StringRef> InvalidationFunctions; std::vector<StringRef> ReinitializationFunctions; + std::vector<StringRef> ReportAccessOnlyUseForTypes; }; } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 3d9e34cc4d44e7..bb0212956ce19e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -194,6 +194,13 @@ infrastructure are described first, followed by tool-specific sections. <clang-tidy/checks/bugprone/std-namespace-modification>` when checking lambda closure types used as template arguments. +- Improved {doc}`bugprone-use-after-move + <clang-tidy/checks/bugprone/use-after-move>` check by adding the + {option}`ReportAccessOnlyUseForTypes` option, which restricts diagnostics to + uses that access the object (a member access, a dereference, or a subscript) + for the configured pointer-like types, ignoring other references such as + passing, comparing, or copying the moved-from variable. + - Improved {doc}`cppcoreguidelines-missing-std-forward <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by diagnosing unforwarded `auto&&` parameters in C++20 abbreviated function templates. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.md b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.md index e766283c77ca84..f4d10f84eff49d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.md +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.md @@ -269,3 +269,14 @@ 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. ``` + +```{option} ReportAccessOnlyUseForTypes +A semicolon-separated list of regular expressions matching names of types for +which only an access of the object counts as a use. For a variable whose type +is a pointer to a listed type, or a class that is the same as or derived from a +listed type, the check reports a use only when the code accesses the object, +that is, a member access (`p->m`, `p.m`), an overloaded dereference, a built-in +dereference (`*p`), or a subscript (`p[i]`). Other references to the variable +(such as passing it as an argument, comparing it, or copying it) are not +reported. 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 1dd67941bd84cf..9611062fb1bbe1 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,15 @@ // RUN: %check_clang_tidy -std=c++11,c++14 -check-suffixes=,CXX11 %s bugprone-use-after-move %t -- \ // RUN: -config='{CheckOptions: { \ // RUN: bugprone-use-after-move.InvalidationFunctions: "::Database<>::StaticCloseConnection;Database<>::CloseConnection;FriendCloseConnection;FreeCloseConnection", \ -// RUN: bugprone-use-after-move.ReinitializationFunctions: "::Database<>::Reset;::Database<>::StaticReset;::FriendReset;::RegularReset" \ +// RUN: bugprone-use-after-move.ReinitializationFunctions: "::Database<>::Reset;::Database<>::StaticReset;::FriendReset;::RegularReset", \ +// RUN: bugprone-use-after-move.ReportAccessOnlyUseForTypes: "::report_access_only::AccessOnly;::report_access_only::HandleBase;::report_access_only::SmartHandle" \ // 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;FreeCloseConnection", \ -// RUN: bugprone-use-after-move.ReinitializationFunctions: "::Database<>::Reset;::Database<>::StaticReset;::FriendReset;::RegularReset" \ +// RUN: bugprone-use-after-move.ReinitializationFunctions: "::Database<>::Reset;::Database<>::StaticReset;::FriendReset;::RegularReset", \ +// RUN: bugprone-use-after-move.ReportAccessOnlyUseForTypes: "::report_access_only::AccessOnly;::report_access_only::HandleBase;::report_access_only::SmartHandle" \ // RUN: }}' -- \ // RUN: -fno-delayed-template-parsing @@ -1998,3 +2000,196 @@ void callPartialForwardTemplate(Derived &&d) { partialForwardTemplate<Derived>(std::forward<Derived>(d)); } } // namespace GH63202 + +//////////////////////////////////////////////////////////////////////////////// +// Tests for the ReportAccessOnlyUseForTypes option +// +// For the types in this option, only an access counts as a use. An access is a +// member access or a dereference of the object. A different reference to the +// variable does not count as a use. Examples of a different reference are an +// argument, a comparison, or a copy. This behavior applies to a pointer to a +// listed type. It also applies to a class that is the same as or derived from +// a listed type. + +namespace report_access_only { + +struct AccessOnly { + void foo() const; + int bar; +}; + +void takePointer(AccessOnly *); + +void pointerMethodAccessIsUse() { + AccessOnly *p = nullptr; + std::move(p); + p->foo(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'p' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here +} + +void pointerMemberDataAccessIsUse() { + AccessOnly *p = nullptr; + std::move(p); + int i = p->bar; + (void)i; + // CHECK-NOTES: [[@LINE-2]]:11: warning: 'p' used after it was moved + // CHECK-NOTES: [[@LINE-4]]:3: note: move occurred here +} + +void pointerDerefIsUse() { + AccessOnly *p = nullptr; + std::move(p); + *p; + // CHECK-NOTES: [[@LINE-1]]:4: warning: 'p' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here +} + +void pointerDerefThenMemberIsUse() { + AccessOnly *p = nullptr; + std::move(p); + (*p).foo(); + // CHECK-NOTES: [[@LINE-1]]:5: warning: 'p' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here +} + +void pointerSubscriptIsUse() { + AccessOnly *p = nullptr; + std::move(p); + p[0]; + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'p' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here +} + +void pointerPassIsNotUse() { + AccessOnly *p = nullptr; + std::move(p); + takePointer(p); +} + +void pointerCompareIsNotUse() { + AccessOnly *p = nullptr; + std::move(p); + if (p == nullptr) { + } +} + +void pointerCopyIsNotUse() { + AccessOnly *p = nullptr; + std::move(p); + AccessOnly *p2 = p; + (void)p2; +} + +struct DerivedAccess : AccessOnly {}; + +void derivedPointerAccessIsUse() { + DerivedAccess *p = nullptr; + std::move(p); + p->foo(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'p' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here +} + +void derivedPointerPassIsNotUse() { + DerivedAccess *p = nullptr; + std::move(p); + takePointer(p); +} + +struct HandleBase {}; +struct Handle : HandleBase { + void foo() const; +}; + +void takeHandle(Handle); + +void handleMemberAccessIsUse() { + Handle h; + std::move(h); + h.foo(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'h' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here +} + +void handlePassIsNotUse() { + Handle h; + std::move(h); + takeHandle(h); +} + +void handleCopyIsNotUse() { + Handle h; + std::move(h); + Handle h2 = h; + (void)h2; +} + +struct NotListedInConfig { + void foo() const; +}; + +void nonListedPointerCompareIsUse() { + NotListedInConfig *p = nullptr; + std::move(p); + if (p == nullptr) { + } + // CHECK-NOTES: [[@LINE-2]]:7: warning: 'p' used after it was moved + // CHECK-NOTES: [[@LINE-4]]:3: note: move occurred here +} + +// A smart-pointer-like class with overloaded dereference operators. An access +// to the pointee through an overloaded operator counts as a use. Other +// references to the variable do not count as a use. +struct Pointee { + void foo() const; +}; + +struct SmartHandle { + Pointee *operator->(); + Pointee &operator*(); + Pointee &operator[](int); +}; + +void takeSmartHandle(SmartHandle); +bool operator==(const SmartHandle &, const SmartHandle &); + +void smartHandleArrowIsUse() { + SmartHandle s; + std::move(s); + s->foo(); + // CHECK-NOTES: [[@LINE-1]]:3: warning: 's' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here +} + +void smartHandleDerefIsUse() { + SmartHandle s; + std::move(s); + (*s).foo(); + // CHECK-NOTES: [[@LINE-1]]:5: warning: 's' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here +} + +void smartHandleSubscriptIsUse() { + SmartHandle s; + std::move(s); + s[0]; + // CHECK-NOTES: [[@LINE-1]]:3: warning: 's' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here +} + +void smartHandlePassIsNotUse() { + SmartHandle s; + std::move(s); + takeSmartHandle(s); +} + +void smartHandleCompareIsNotUse() { + SmartHandle s1; + SmartHandle s2; + std::move(s1); + if (s1 == s2) { + } +} + +} // namespace report_access_only _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
