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.poluk...@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); }`

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to