https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/189638
>From 01cb1f70ce87d59dc618b7b0e94c6c7fa63f4d25 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Tue, 31 Mar 2026 13:41:59 +0200 Subject: [PATCH 1/5] [clang-tidy] Prevent false-positive in presence of derived-to-base cast in bugprone.use-after-move The following scenario is quite common, but was reported as a use-after-move: struct Base { Base(Base&&); }; struct C : Base { int field; C(C&& c) : Base(std::move(c)), // only moves through the base type field(c.field) // << this is a valid use-after-move {} }; Fix this by checking field origin when the moved value is immediately cast to base. --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 28 +++++++++++++++---- .../use-after-move-derived-to-base.cpp | 26 +++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 9e9dfdc5de11e..9f36e6819ca93 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -28,6 +28,7 @@ using namespace clang::tidy::utils; namespace clang::tidy::bugprone { +using ast_matchers::optionally; using matchers::hasUnevaluatedContext; namespace { @@ -52,7 +53,8 @@ class UseAfterMoveFinder { public: UseAfterMoveFinder(ASTContext *TheContext, llvm::ArrayRef<StringRef> InvalidationFunctions, - llvm::ArrayRef<StringRef> ReinitializationFunctions); + llvm::ArrayRef<StringRef> ReinitializationFunctions, + const CXXRecordDecl *MovedAs); // Within the given code block, finds the first use of 'MovedVariable' that // occurs after 'MovingCall' (the expression that performs the move). If a @@ -77,6 +79,7 @@ class UseAfterMoveFinder { ASTContext *Context; llvm::ArrayRef<StringRef> InvalidationFunctions; llvm::ArrayRef<StringRef> ReinitializationFunctions; + const CXXRecordDecl *MovedAs; std::unique_ptr<ExprSequence> Sequence; std::unique_ptr<StmtToBlockMap> BlockMap; llvm::SmallPtrSet<const CFGBlock *, 8> Visited; @@ -193,9 +196,10 @@ static StatementMatcher inDecltypeOrTemplateArg() { UseAfterMoveFinder::UseAfterMoveFinder( ASTContext *TheContext, llvm::ArrayRef<StringRef> InvalidationFunctions, - llvm::ArrayRef<StringRef> ReinitializationFunctions) + llvm::ArrayRef<StringRef> ReinitializationFunctions, + const CXXRecordDecl *MovedAs) : Context(TheContext), InvalidationFunctions(InvalidationFunctions), - ReinitializationFunctions(ReinitializationFunctions) {} + ReinitializationFunctions(ReinitializationFunctions), MovedAs(MovedAs) {} std::optional<UseAfterMove> UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall, @@ -405,7 +409,13 @@ void UseAfterMoveFinder::getDeclRefs( DeclRefs](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"); + // Non-moved member as the move only implies a base class. + if (Member && MovedAs && + !MovedAs->hasMemberName(Member->getMemberDecl()->getIdentifier())) { + continue; + } if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block) { // Ignore uses of a standard smart pointer or classes annotated as // "null_after_move" (smart-pointer-like behavior) that don't @@ -420,7 +430,8 @@ void UseAfterMoveFinder::getDeclRefs( declRefExpr(hasDeclaration(equalsNode(MovedVariable)), unless(inDecltypeOrTemplateArg()), unless(hasParentIgnoringParenImpCasts( - memberExpr(hasDeclaration(cxxDestructorDecl()))))) + memberExpr(hasDeclaration(cxxDestructorDecl())))), + optionally(hasParent(memberExpr().bind("member-expr")))) .bind("declref"); AddDeclRefs(match(traverse(TK_AsIs, findAll(DeclRefMatcher)), *S->getStmt(), @@ -548,6 +559,7 @@ void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) { hasArgument(0, Arg))), unless(inDecltypeOrTemplateArg()), unless(hasParent(TryEmplaceMatcher)), expr().bind("call-move"), + optionally(hasParent(implicitCastExpr().bind("optional-cast"))), anyOf(hasAncestor(compoundStmt( hasParent(lambdaExpr().bind("containing-lambda")))), hasAncestor(functionDecl(anyOf( @@ -593,6 +605,8 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) { const auto *MovingCall = Result.Nodes.getNodeAs<Expr>("moving-call"); const auto *Arg = Result.Nodes.getNodeAs<DeclRefExpr>("arg"); const auto *MoveDecl = Result.Nodes.getNodeAs<FunctionDecl>("move-decl"); + const auto *ParentCast = + Result.Nodes.getNodeAs<ImplicitCastExpr>("optional-cast"); if (!MovingCall || !MovingCall->getExprLoc().isValid()) MovingCall = CallMove; @@ -623,9 +637,13 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) { CodeBlocks.push_back(ContainingFunc->getBody()); } + const CXXRecordDecl *MovedAs = nullptr; + if (ParentCast && ParentCast->getCastKind() == CK_DerivedToBase) + MovedAs = ParentCast->getType()->getAsCXXRecordDecl(); + for (Stmt *CodeBlock : CodeBlocks) { UseAfterMoveFinder Finder(Result.Context, InvalidationFunctions, - ReinitializationFunctions); + ReinitializationFunctions, 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/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp new file mode 100644 index 0000000000000..16366a0265a1a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp @@ -0,0 +1,26 @@ +// RUN: %check_clang_tidy -std=c++11,c++14 %s bugprone-use-after-move %t + +#include <utility> + +struct A { + int a; +}; + +struct B : A { + int b; + B(B&& other) : + A(std::move(other)), + b(std::move(other.b)) // No error raised + {} +}; + +struct C : A { + int c; + C(C&& other) : + A(std::move(other)), + { + std::move(other.a) + // CHECK-NOTES: [[@LINE-1]]:17: warning: 'other' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here + } +}; >From 8a0ae23403a839bcb7a175d96f94219482bbe173 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Tue, 31 Mar 2026 14:02:37 +0200 Subject: [PATCH 2/5] fixup! [clang-tidy] Prevent false-positive in presence of derived-to-base cast in bugprone.use-after-move --- .../checkers/bugprone/use-after-move-derived-to-base.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp index 16366a0265a1a..a088054f17544 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp @@ -17,10 +17,10 @@ struct B : A { struct C : A { int c; C(C&& other) : - A(std::move(other)), + A(std::move(other)) { - std::move(other.a) - // CHECK-NOTES: [[@LINE-1]]:17: warning: 'other' used after it was moved - // CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here + other.a; + // CHECK-NOTES: [[@LINE-1]]:7: warning: 'other' used after it was moved + // CHECK-NOTES: [[@LINE-4]]:5: note: move occurred here } }; >From 1c7d10b82a5d42a6d3e1d55fdd4d4b87521d2b1e Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Tue, 31 Mar 2026 17:29:26 +0200 Subject: [PATCH 3/5] fixup! fixup! [clang-tidy] Prevent false-positive in presence of derived-to-base cast in bugprone.use-after-move --- clang-tools-extra/docs/ReleaseNotes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ad2ee2ae4b35e..354948f3693a9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -277,6 +277,9 @@ Changes in existing checks - Do not report explicit call to destructor after move as an invalid use. + - Avoid false positives when moving object to a base type then accessing + non-base members. + - Improved :doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines <clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines>` check by adding the `AllowExplicitObjectParameters` option. When enabled, >From a60474dd5fca7256c52a333a6ba3e95b79d0dc55 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Wed, 1 Apr 2026 10:03:47 +0200 Subject: [PATCH 4/5] fixup! fixup! fixup! [clang-tidy] Prevent false-positive in presence of derived-to-base cast in bugprone.use-after-move --- .../checkers/bugprone/use-after-move-derived-to-base.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp index a088054f17544..64983eb0b5e2c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp @@ -10,7 +10,8 @@ struct B : A { int b; B(B&& other) : A(std::move(other)), - b(std::move(other.b)) // No error raised + b(std::move(other.b)) + // CHECK-NOTES-NOT: [[@LINE-1]]:7: warning: 'other' used after it was moved {} }; >From 8df6a98f143007512f190916552c32297a56ea4b Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Thu, 2 Apr 2026 11:08:06 +0200 Subject: [PATCH 5/5] fixup! fixup! fixup! fixup! [clang-tidy] Prevent false-positive in presence of derived-to-base cast in bugprone.use-after-move --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 83 ++++++++++--------- .../use-after-move-derived-to-base.cpp | 6 ++ 2 files changed, 49 insertions(+), 40 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 9f36e6819ca93..e3100e03a156b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -28,9 +28,26 @@ using namespace clang::tidy::utils; namespace clang::tidy::bugprone { -using ast_matchers::optionally; using matchers::hasUnevaluatedContext; +namespace { +AST_MATCHER_P(Expr, hasParentIgnoringParenImpCasts, + ast_matchers::internal::Matcher<Expr>, InnerMatcher) { + const Expr *E = &Node; + do { + const DynTypedNodeList Parents = Finder->getASTContext().getParents(*E); + if (Parents.size() != 1) + return false; + E = Parents[0].get<Expr>(); + if (!E) + return false; + } while (isa<ImplicitCastExpr, ParenExpr>(E)); + + return InnerMatcher.matches(*E, Finder, Builder); +} + +} // namespace + namespace { /// Contains information about a use-after-move. @@ -85,21 +102,6 @@ class UseAfterMoveFinder { llvm::SmallPtrSet<const CFGBlock *, 8> Visited; }; -AST_MATCHER_P(Expr, hasParentIgnoringParenImpCasts, - ast_matchers::internal::Matcher<Expr>, InnerMatcher) { - const Expr *E = &Node; - do { - const DynTypedNodeList Parents = Finder->getASTContext().getParents(*E); - if (Parents.size() != 1) - return false; - E = Parents[0].get<Expr>(); - if (!E) - return false; - } while (isa<ImplicitCastExpr, ParenExpr>(E)); - - return InnerMatcher.matches(*E, Finder, Builder); -} - } // namespace static auto getNameMatcher(llvm::ArrayRef<StringRef> InvalidationFunctions) { @@ -431,7 +433,8 @@ void UseAfterMoveFinder::getDeclRefs( unless(inDecltypeOrTemplateArg()), unless(hasParentIgnoringParenImpCasts( memberExpr(hasDeclaration(cxxDestructorDecl())))), - optionally(hasParent(memberExpr().bind("member-expr")))) + optionally(hasParentIgnoringParenImpCasts( + memberExpr().bind("member-expr")))) .bind("declref"); AddDeclRefs(match(traverse(TK_AsIs, findAll(DeclRefMatcher)), *S->getStmt(), @@ -551,26 +554,27 @@ void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) { cxxMemberCallExpr(callee(cxxMethodDecl(hasName("try_emplace")))); auto Arg = declRefExpr().bind("arg"); auto IsMemberCallee = callee(functionDecl(unless(isStaticStorageClass()))); - auto CallMoveMatcher = - callExpr(callee(functionDecl(getNameMatcher(InvalidationFunctions)) - .bind("move-decl")), - anyOf(cxxMemberCallExpr(IsMemberCallee, on(Arg)), - callExpr(unless(cxxMemberCallExpr(IsMemberCallee)), - hasArgument(0, Arg))), - unless(inDecltypeOrTemplateArg()), - unless(hasParent(TryEmplaceMatcher)), expr().bind("call-move"), - optionally(hasParent(implicitCastExpr().bind("optional-cast"))), - anyOf(hasAncestor(compoundStmt( - hasParent(lambdaExpr().bind("containing-lambda")))), - hasAncestor(functionDecl(anyOf( - cxxConstructorDecl( - hasAnyConstructorInitializer(withInitializer( - expr(anyOf(equalsBoundNode("call-move"), - hasDescendant(expr( - equalsBoundNode("call-move"))))) - .bind("containing-ctor-init")))) - .bind("containing-ctor"), - functionDecl().bind("containing-func")))))); + auto CallMoveMatcher = callExpr( + callee(functionDecl(getNameMatcher(InvalidationFunctions)) + .bind("move-decl")), + anyOf(cxxMemberCallExpr(IsMemberCallee, on(Arg)), + callExpr(unless(cxxMemberCallExpr(IsMemberCallee)), + hasArgument(0, Arg))), + unless(inDecltypeOrTemplateArg()), unless(hasParent(TryEmplaceMatcher)), + expr().bind("call-move"), + optionally(hasParent(implicitCastExpr(hasCastKind(CK_DerivedToBase)) + .bind("optional-cast"))), + anyOf(hasAncestor(compoundStmt( + hasParent(lambdaExpr().bind("containing-lambda")))), + hasAncestor(functionDecl( + anyOf(cxxConstructorDecl( + hasAnyConstructorInitializer(withInitializer( + expr(anyOf(equalsBoundNode("call-move"), + hasDescendant(expr( + equalsBoundNode("call-move"))))) + .bind("containing-ctor-init")))) + .bind("containing-ctor"), + functionDecl().bind("containing-func")))))); Finder->addMatcher( traverse( @@ -637,9 +641,8 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) { CodeBlocks.push_back(ContainingFunc->getBody()); } - const CXXRecordDecl *MovedAs = nullptr; - if (ParentCast && ParentCast->getCastKind() == CK_DerivedToBase) - MovedAs = ParentCast->getType()->getAsCXXRecordDecl(); + const CXXRecordDecl *MovedAs = + ParentCast ? ParentCast->getType()->getAsCXXRecordDecl() : nullptr; for (Stmt *CodeBlock : CodeBlocks) { UseAfterMoveFinder Finder(Result.Context, InvalidationFunctions, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp index 64983eb0b5e2c..9e10228b6a9a8 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move-derived-to-base.cpp @@ -25,3 +25,9 @@ struct C : A { // CHECK-NOTES: [[@LINE-4]]:5: note: move occurred here } }; + +struct D { int d; }; +struct E : A, D { + E(E&& other) : A(std::move(other)) { other.d; } +}; + _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
