Author: Peiqi Li Date: 2026-06-10T10:53:49+03:00 New Revision: daa49cfc2e01ff9df3400321ee308ec82b23ff34
URL: https://github.com/llvm/llvm-project/commit/daa49cfc2e01ff9df3400321ee308ec82b23ff34 DIFF: https://github.com/llvm/llvm-project/commit/daa49cfc2e01ff9df3400321ee308ec82b23ff34.diff LOG: [clang-tidy] Fix false positive in bugprone-use-after-move with std::forward on derived classes (#199905) 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 uses `traverse(TK_AsIs, expr(hasParent(...)))` on the first argument to navigate bottom-up, reliably capturing the hidden `ImplicitCastExpr`. This ensures both partial moves and forwards are consistently recognized, eliminating the false positive. Assisted by AI to check code. Fixes #63202 Added: Modified: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp index 361be321185df..d3cf2923eaf83 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 99fe37f4145dd..8e83ef7feb670 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -463,6 +463,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
