[clang-tools-extra] [clang-tidy] Fix some false positive in bugprone-move-forwarding-reference (PR #191435)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 16 04:17:20 PDT 2026
https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/191435
>From 9c46692b5e60c2347597dea9588d221174e4be2e Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Fri, 10 Apr 2026 16:43:46 +0200
Subject: [PATCH 1/4] [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 3d126910d2e2e..9e99e7624f171 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -271,6 +271,10 @@ Changes in existing checks
<clang-tidy/checks/bugprone/narrowing-conversions>` check by fixing a false
positive when converting a ``bool`` to a signed integer type.
+- 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 dadb5e92b2f0da35c3467f94289d41036c356029 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Fri, 10 Apr 2026 21:05:58 +0200
Subject: [PATCH 2/4] 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 9e99e7624f171..6a696b23a374b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -273,7 +273,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
>From 260f52a39889eb631b8d3992f65e262177e3a66f Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Thu, 16 Apr 2026 12:38:56 +0200
Subject: [PATCH 3/4] fixup! fixup! [clang-tidy] Fix some false positive in
bugprone-move-forwarding-reference
---
.../bugprone/MoveForwardingReferenceCheck.cpp | 1 -
.../checkers/bugprone/move-forwarding-reference.cpp | 10 ----------
2 files changed, 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp
index ab8f8cdd0861e..deb17880aaf8d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/MoveForwardingReferenceCheck.cpp
@@ -96,7 +96,6 @@ void MoveForwardingReferenceCheck::registerMatchers(MatchFinder *Finder) {
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/test/clang-tidy/checkers/bugprone/move-forwarding-reference.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/move-forwarding-reference.cpp
index f100d6f1d3f3d..42807695ea039 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
@@ -123,13 +123,3 @@ template <typename T, typename U> void f13(U&& SomeU) {
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 7b50c784fa24b639f7527c0106175b66f6e6b2d5 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Thu, 16 Apr 2026 13:17:04 +0200
Subject: [PATCH 4/4] fixup! fixup! fixup! [clang-tidy] Fix some false positive
in bugprone-move-forwarding-reference
---
clang-tools-extra/docs/ReleaseNotes.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6a696b23a374b..58c486a0bbbe4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -267,14 +267,14 @@ 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-narrowing-conversions
- <clang-tidy/checks/bugprone/narrowing-conversions>` check by fixing a false
- positive when converting a ``bool`` to a signed integer type.
-
- Improved :doc:`bugprone-move-forwarding-reference
<clang-tidy/checks/bugprone/move-forwarding-reference>` check by removing some
false positives in the context of moved lambda captures.
+- Improved :doc:`bugprone-narrowing-conversions
+ <clang-tidy/checks/bugprone/narrowing-conversions>` check by fixing a false
+ positive when converting a ``bool`` to a signed integer type.
+
- 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.
More information about the cfe-commits
mailing list