https://github.com/voyager-jhk updated https://github.com/llvm/llvm-project/pull/199905
>From 0959bd6228f25e61c198e7e7c171a4527ab13b4c Mon Sep 17 00:00:00 2001 From: voyager-jhk <[email protected]> Date: Wed, 27 May 2026 16:54:08 +0800 Subject: [PATCH] [clang-tidy] Fix false positive in bugprone-use-after-move with std::forward on derived classes The `bugprone-use-after-move` check correctly identified partial moves when using `std::move` by matching the `ImplicitCastExpr` (DerivedToBase) as the parent of the call. However, when using `std::forward<Base>`, the cast occurs inside the argument, causing the matcher to miss the cast and falsely report a use-after-move. This patch manually checks the first argument of the moving call for a `DerivedToBase` cast if the parent matcher fails, ensuring both `move` and `forward` are correctly identified as partial moves. Fixes #63202 --- .../clang-tidy/bugprone/UseAfterMoveCheck.cpp | 8 ++- clang-tools-extra/docs/ReleaseNotes.rst | 3 + .../checkers/bugprone/use-after-move.cpp | 57 +++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 361be321185df..ea40ee198abbb 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp @@ -560,6 +560,8 @@ void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) { cxxMemberCallExpr(callee(cxxMethodDecl(hasName("try_emplace")))); auto Arg = declRefExpr().bind("arg"); auto IsMemberCallee = callee(functionDecl(unless(isStaticStorageClass()))); + auto DerivedToBaseCast = + implicitCastExpr(hasCastKind(CK_DerivedToBase)).bind("optional-cast"); auto CallMoveMatcher = callExpr( callee(functionDecl(getNameMatcher(InvalidationFunctions)) .bind("move-decl")), @@ -568,8 +570,10 @@ void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) { hasArgument(0, Arg))), unless(inDecltypeOrTemplateArg()), unless(hasParent(TryEmplaceMatcher)), expr().bind("call-move"), - optionally(hasParent(implicitCastExpr(hasCastKind(CK_DerivedToBase)) - .bind("optional-cast"))), + optionally(anyOf(hasParent(DerivedToBaseCast), + hasArgument(0, traverse(TK_AsIs, + expr(hasParent( + DerivedToBaseCast)))))), anyOf(hasAncestor(compoundStmt( hasParent(lambdaExpr().bind("containing-lambda")))), hasAncestor(functionDecl( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index fa53072be1eac..deddecaf65276 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -452,6 +452,9 @@ Changes in existing checks std::move(b))``). The tuple assignment writes back through the stored references, which fully reinitializes the captured variables. + - Avoid false positives when forwarding a derived object to initialize a base + class subobject. + - Improved :doc:`cert-err33-c <clang-tidy/checks/cert/err33-c>` check by not inheriting `CheckedReturnTypes` option from :doc:`bugprone-unused-return-value 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 0844aa32825f6..1dd67941bd84c 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 @@ -1941,3 +1941,60 @@ void stdTieParenthesizedPartialReinit() { // CHECK-NOTES: [[@LINE-1]]:3: warning: 'a' used after it was moved // CHECK-NOTES: [[@LINE-5]]:3: note: move occurred here } + +namespace GH63202 { + +struct Base { + std::string name; + Base() = default; + Base(Base &&) = default; + Base &operator=(Base &&) = default; +}; + +struct Derived : Base { + std::string surname; + + Derived() = default; + + Derived(Derived &&d) + : Base(std::move(d)), surname(std::move(d.surname)) {} + + Derived(Derived &&d, int) + : Base(std::forward<Base>(d)), surname(std::move(d.surname)) {} + + Derived(Derived &&d, char) : Base(std::forward<Base>(d)) { + d.name.size(); + // CHECK-NOTES: [[@LINE-1]]:5: warning: 'd' used after it was forwarded + // CHECK-NOTES: [[@LINE-3]]:32: note: forward occurred here + } +}; + +void invalidFullMove(Derived &&d) { + Derived other(std::move(d)); + d.surname = "1"; + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'd' used after it was moved + // CHECK-NOTES: [[@LINE-3]]:11: note: move occurred here +} + +void invalidFullForward(Derived &&d) { + Derived other(std::forward<Derived>(d)); + d.surname = "1"; + // CHECK-NOTES: [[@LINE-1]]:3: warning: 'd' used after it was forwarded + // CHECK-NOTES: [[@LINE-3]]:11: note: forward occurred here +} + +void partialForward(Derived &&d) { + Base other(std::forward<Base>(d)); + d.surname = "1"; +} + +template <typename T> +void partialForwardTemplate(T &&d) { + Base other(std::forward<Base>(d)); + d.surname = "1"; +} + +void callPartialForwardTemplate(Derived &&d) { + partialForwardTemplate<Derived>(std::forward<Derived>(d)); +} +} // namespace GH63202 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
