[clang-tidy] fix false negative in cppcoreguidelines-missing-std-forward

[jcsxky]

Try to fix #83845
When std::forward is invoked in a function, make sure it uses correct parameter by checking if the bounded var equals the parameter.

[llvmbot]

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Qizhi Hu (jcsxky)

Changes

Try to fix #83845
When std::forward is invoked in a function, make sure it uses correct parameter by checking if the bounded var equals the parameter.

---

Full diff: https://github.com/llvm/llvm-project/pull/83987.diff

3 Files Affected:

• (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp (+6-4)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1)
• (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp (+10)

diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
index c633683570f748..87fd8adf997082 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
@@ -112,10 +112,12 @@ void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) {
 
   auto ForwardCallMatcher = callExpr(
       callExpr().bind("call"), argumentCountIs(1),
-      hasArgument(
-          0, declRefExpr(to(
-                 varDecl(optionally(equalsBoundNode("param"))).bind("var")))),
-      forCallable(anyOf(equalsBoundNode("func"), CapturedInLambda)),
+      hasArgument(0, declRefExpr(to(varDecl().bind("var")))),
+      forCallable(
+          anyOf(allOf(equalsBoundNode("func"),
+                      functionDecl(hasAnyParameter(parmVarDecl(allOf(
+                          equalsBoundNode("param"), equalsBoundNode("var")))))),
+                CapturedInLambda)),
       callee(unresolvedLookupExpr(hasAnyDeclaration(
           namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))),
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 143ae230fc443c..1abe45bae8a84d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -156,7 +156,8 @@ Changes in existing checks
 
 - Improved :doc:`cppcoreguidelines-missing-std-forward
   <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by no longer
-  giving false positives for deleted functions.
+  giving false positives for deleted functions and fix false negative when one
+  parameter is forwarded, but some other parameter isn't.
 
 - Cleaned up :doc:`cppcoreguidelines-prefer-member-initializer
   <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>`
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
index 20e43f04180ff3..29af67b60614c8 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
@@ -95,6 +95,16 @@ void lambda_value_capture_copy(T&& t) {
   [&,t]() { T other = std::forward<T>(t); };
 }
 
+template <typename X>
+void use(const X &x) {}
+
+template <typename X, typename Y>
+void foo(X &&x, Y &&y) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
+    use(std::forward<X>(x));
+    use(y);
+}
+
 } // namespace positive_cases
 
 namespace negative_cases {

[HerrCai0907]

LGTM since it is helpful to improve this check.
But I still take a negative view about use std::forward in lambda body.

[HerrCai0907]

Suggested change

	giving false positives for deleted functions and fix false negative when one
	parameter is forwarded, but some other parameter isn't.
	giving false positives for deleted functions and fix false negative when some
	parameters are forwarded, but other aren't.

[jcsxky]

Thanks for review to make the description more clear!
I think you can check

llvm-project/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp

Lines 171 to 173 in 2a1b09f

	void lambda_value_reference_auxiliary_var(T&& t) {
	[&x = t]() { T other = std::forward<T>(x); };
	}

this testcase may helpful for solving confusions.

[PiotrZSL]

LGTM