https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/191435
>From 9acd0c37d2b5887d71866707bba892126acc9efa Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Fri, 10 Apr 2026 16:43:46 +0200 Subject: [PATCH 1/2] [clang-tidy] Fix some false positive in bugprone-move-forwarding-reference In the following case: template <typename T, typename U> void shocase(U&& SomeU) { [SomeU] () { T SomeT(std::move(SomeU)); }; } We use to flag the move as a forward, while the lambda captures SomeU by copy, which makes the move valid. --- .../bugprone/MoveForwardingReferenceCheck.cpp | 12 +++++++++- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++ .../bugprone/move-forwarding-reference.cpp | 22 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp index e182df75b1d9a..7cc3d33da1a78 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp @@ -13,6 +13,14 @@ using namespace clang::ast_matchers; namespace clang::tidy::bugprone { +namespace { + +AST_MATCHER(DeclRefExpr, refersToEnclosingVariableOrCapture) { + return Node.refersToEnclosingVariableOrCapture(); +} + +} + static void replaceMoveWithForward(const UnresolvedLookupExpr *Callee, const ParmVarDecl *ParmVar, const TemplateTypeParmDecl *TypeParmDecl, @@ -86,7 +94,9 @@ void MoveForwardingReferenceCheck::registerMatchers(MatchFinder *Finder) { .bind("lookup")), argumentCountIs(1), hasArgument(0, ignoringParenImpCasts(declRefExpr( - to(ForwardingReferenceParmMatcher))))) + to(ForwardingReferenceParmMatcher), + // FIXME: allow capture by reference + unless(refersToEnclosingVariableOrCapture()))))) .bind("call-move"), this); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index caf0275035064..8a2dbfca8384b 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -256,6 +256,10 @@ Changes in existing checks <clang-tidy/checks/bugprone/macro-parentheses>` check by printing the macro definition in the warning message if the macro is defined on command line. +- Improved :doc:`bugprone-move-forwarding-reference + <clang-tidy/checks/bugprone/move-forwarding-reference>` check by removing some + false positive in the context of moved lambda captures. + - Improved :doc:`bugprone-pointer-arithmetic-on-polymorphic-object <clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object>` check by fixing a false positive when ``operator[]`` is used in a dependent context. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/move-forwarding-reference.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/move-forwarding-reference.cpp index c9f40668f449a..f100d6f1d3f3d 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/move-forwarding-reference.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/move-forwarding-reference.cpp @@ -111,3 +111,25 @@ template <typename T> void f12() { // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: forwarding reference passed to // CHECK-FIXES: [] (auto&& x) { T SomeT(std::forward<decltype(x)>(x)); }; } + +// Ignore the case of captured variables where an implicit copy already +// happened. Explicit capture version. +template <typename T, typename U> void f13(U&& SomeU) { + [SomeU] () { T SomeT(std::move(SomeU)); }; +} + +// Ignore the case of captured variables where an implicit copy already +// happened. Implicit capture version. +template <typename T, typename U> void f14(U&& SomeU) { + [=] () { T SomeT(std::move(SomeU)); }; +} + +#if 0 +// FIXME: we currently ignore that case. +// +// Handle the case of captured variables where no copy already +// happened. +template <typename T, typename U> void f15(U&& SomeU) { + [&SomeU] () { T SomeT(std::move(SomeU)); }; +} +#endif >From a4465b21ffc9fb38fa5339315d6387c108e69b28 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Fri, 10 Apr 2026 21:05:58 +0200 Subject: [PATCH 2/2] fixup! [clang-tidy] Fix some false positive in bugprone-move-forwarding-reference --- .../bugprone/MoveForwardingReferenceCheck.cpp | 21 ++++++++++--------- clang-tools-extra/docs/ReleaseNotes.rst | 2 +- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp index 7cc3d33da1a78..ab8f8cdd0861e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp @@ -19,7 +19,7 @@ AST_MATCHER(DeclRefExpr, refersToEnclosingVariableOrCapture) { return Node.refersToEnclosingVariableOrCapture(); } -} +} // namespace static void replaceMoveWithForward(const UnresolvedLookupExpr *Callee, const ParmVarDecl *ParmVar, @@ -88,15 +88,16 @@ void MoveForwardingReferenceCheck::registerMatchers(MatchFinder *Finder) { .bind("parm-var"); Finder->addMatcher( - callExpr(callee(unresolvedLookupExpr( - hasAnyDeclaration(namedDecl( - hasUnderlyingDecl(hasName("::std::move"))))) - .bind("lookup")), - argumentCountIs(1), - hasArgument(0, ignoringParenImpCasts(declRefExpr( - to(ForwardingReferenceParmMatcher), - // FIXME: allow capture by reference - unless(refersToEnclosingVariableOrCapture()))))) + callExpr( + callee(unresolvedLookupExpr( + hasAnyDeclaration( + namedDecl(hasUnderlyingDecl(hasName("::std::move"))))) + .bind("lookup")), + argumentCountIs(1), + hasArgument(0, ignoringParenImpCasts(declRefExpr( + to(ForwardingReferenceParmMatcher), + // FIXME: allow capture by reference + unless(refersToEnclosingVariableOrCapture()))))) .bind("call-move"), this); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8a2dbfca8384b..171945be3af7a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -258,7 +258,7 @@ Changes in existing checks - Improved :doc:`bugprone-move-forwarding-reference <clang-tidy/checks/bugprone/move-forwarding-reference>` check by removing some - false positive in the context of moved lambda captures. + false positives in the context of moved lambda captures. - Improved :doc:`bugprone-pointer-arithmetic-on-polymorphic-object <clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object>` check _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
