[clang] [clang-tools-extra] [clang-tidy] Fix false in unnecessary-value-param inside templates (PR #98488)

[Dmitry Polukhin via cfe-commits]

https://github.com/dmpolukhin created https://github.com/llvm/llvm-project/pull/98488

Summary:
If callExpr is type dependent, there is no way to analyze individual arguments until template specialization. Before this diff only calls with dependent callees were skipped so unnecessary-value-param was processing arguments that had non-dependent type that gave false positives because the call was not fully resolved till specialization. So now instead of checking type dependent callee, the whole expression will be checked for type dependent.

Test Plan: check-clang-tools

>From a05c4ca2c2e61653e7bd8d3a2f5faa6b86daa615 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.polukhin at gmail.com>
Date: Thu, 11 Jul 2024 07:01:49 -0700
Subject: [PATCH] [clang-tidy] Fix false in unnecessary-value-param inside
 templates

Summary:
If callExpr is type dependent, there is no way to analyze individual arguments
until template specialization. Before this diff only calls with dependent
callees were skipped so unnecessary-value-param was processing arguments that
had non-dependent type that gave false positives because the call was not fully
resolved till specialization. So now instead of checking type dependent callee,
the whole expression will be checked for type dependent.

Test Plan: check-clang-tools
---
 .../performance/unnecessary-value-param.cpp   | 34 +++++++++++++++++++
 clang/lib/Analysis/ExprMutationAnalyzer.cpp   |  9 +++--
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
index d578eedd94a39..b1696ad3c3d59 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
@@ -2,6 +2,31 @@
 
 // CHECK-FIXES: #include <utility>
 
+namespace std {
+template <typename>
+struct remove_reference;
+
+template <typename _Tp>
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template <typename _Tp>
+struct remove_reference<_Tp &> {
+  typedef _Tp type;
+};
+
+template <typename _Tp>
+struct remove_reference<_Tp &&> {
+  typedef _Tp type;
+};
+
+template <typename _Tp>
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) {
+  return static_cast<typename std::remove_reference<_Tp>::type &&>(__t);
+}
+} // namespace std
+
 struct ExpensiveToCopyType {
   const ExpensiveToCopyType & constReference() const {
     return *this;
@@ -402,3 +427,12 @@ int templateSpecializationFunction(ExpensiveToCopyType E) {
   // CHECK-FIXES-NOT: int templateSpecializationFunction(const ExpensiveToCopyType& E) {
   return 0;
 }
+
+struct B {
+  static void bar(ExpensiveMovableType a, ExpensiveMovableType b);
+};
+
+template <typename T>
+void NegativeCallWithDependentAndNondependentArgs(ExpensiveMovableType a, T b) {
+    B::bar(std::move(a), b);
+}
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index 3b3782fa1db9a..70a1d7b52ffe9 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -404,15 +404,14 @@ ExprMutationAnalyzer::Analyzer::findDirectMutation(const Expr *Exp) {
             memberExpr(hasObjectExpression(canResolveToExpr(Exp)))),
       nonConstReferenceType());
   const auto NotInstantiated = unless(hasDeclaration(isInstantiated()));
-  const auto TypeDependentCallee =
-      callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(),
-                        cxxDependentScopeMemberExpr(),
-                        hasType(templateTypeParmType()), isTypeDependent())));
 
   const auto AsNonConstRefArg = anyOf(
       callExpr(NonConstRefParam, NotInstantiated),
       cxxConstructExpr(NonConstRefParam, NotInstantiated),
-      callExpr(TypeDependentCallee, hasAnyArgument(canResolveToExpr(Exp))),
+      // If the call is type-dependent, we can't properly process any
+      // argument because required type conversions and implicit casts
+      // will be inserted only after specialization.
+      callExpr(isTypeDependent(), hasAnyArgument(canResolveToExpr(Exp))),
       cxxUnresolvedConstructExpr(hasAnyArgument(canResolveToExpr(Exp))),
       // Previous False Positive in the following Code:
       // `template <typename T> void f() { int i = 42; new Type<T>(i); }`