[clang-tidy] Fix some false positive in bugprone-move-forwarding-reference (#191435)

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.

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();
+}
+
+} // namespace
+
 static void replaceMoveWithForward(const UnresolvedLookupExpr *Callee,
                                    const ParmVarDecl *ParmVar,
                                    const TemplateTypeParmDecl *TypeParmDecl,
@@ -80,13 +88,15 @@ void MoveForwardingReferenceCheck::registerMatchers(MatchFinder *Finder) {
           .bind("parm-var");
 
   Finder->addMatcher(
-      callExpr(callee(unresolvedLookupExpr(
+      callExpr(
-                          hasAnyDeclaration(namedDecl(
+          callee(unresolvedLookupExpr(
-                              hasUnderlyingDecl(hasName("::std::move")))))
+                     hasAnyDeclaration(
-                          .bind("lookup")),
+                         namedDecl(hasUnderlyingDecl(hasName("::std::move")))))
-               argumentCountIs(1),
+                     .bind("lookup")),
-               hasArgument(0, ignoringParenImpCasts(declRefExpr(
+          argumentCountIs(1),
-                                  to(ForwardingReferenceParmMatcher)))))
+          hasArgument(0, ignoringParenImpCasts(declRefExpr(
+                             to(ForwardingReferenceParmMatcher),
+                             unless(refersToEnclosingVariableOrCapture())))))
           .bind("call-move"),
       this);
 }

clang-tools-extra/docs/ReleaseNotes.rst

@@ -272,6 +272,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 fixing 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.

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)); };
+}
+
+// FIXME: Do not ignore the case of captured variables by reference. Explicit capture version.
+template <typename T, typename U> void f15(U&& SomeU) {
+  [&SomeU] () { T SomeT(std::move(SomeU)); };
+}
+
+// FIXME: Do not ignore the case of captured variables by reference. Implicit capture version.
+template <typename T, typename U> void f16(U&& SomeU) {
+  [&] () { T SomeT(std::move(SomeU)); };
+}