From 593b3363de0f7fd4ec378ac183bf1f160cb9e613 Mon Sep 17 00:00:00 2001
From: %username% <%username%@google.com>
Date: Thu, 4 Jul 2024 22:32:19 +0200
Subject: [PATCH] Allow unnecessary-value-param to match templated functions
 including lambdas with auto.

Clang-Tidy unnecessary-value-param value param will be triggered for
templated functions if at least one instantiontion with expensive to
copy type is present in translation unit.

It is relatively common mistake to write lambda functions with auto
arguments for expensive to copy types.

Example:
Copy of the vectors will happen on every comparison.

void SortBySize(std::vector<std::vector<int>>& a) {
  std::sort(
      a.begin(), a.end(),
      [](auto x, auto y) { return a.size() < b.size()});
}
---
 .../UnnecessaryValueParamCheck.cpp            |  7 +--
 .../performance/unnecessary-value-param.cpp   | 45 -------------------
 2 files changed, 2 insertions(+), 50 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index c507043c367a86..fe8cc11018e1e9 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -75,7 +75,7 @@ void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
           functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()),
                        unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
                        has(typeLoc(forEach(ExpensiveValueParamDecl))),
-                       unless(isInstantiated()), decl().bind("functionDecl"))),
+                       decl().bind("functionDecl"))),
       this);
 }
 
@@ -133,12 +133,9 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
   // 2. the function is virtual as it might break overrides
   // 3. the function is referenced outside of a call expression within the
   //    compilation unit as the signature change could introduce build errors.
-  // 4. the function is a primary template or an explicit template
-  // specialization.
   const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Function);
   if (Param->getBeginLoc().isMacroID() || (Method && Method->isVirtual()) ||
-      isReferencedOutsideOfCallExpr(*Function, *Result.Context) ||
-      (Function->getTemplatedKind() != FunctionDecl::TK_NonTemplate))
+      isReferencedOutsideOfCallExpr(*Function, *Result.Context))
     return;
   for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
        FunctionDecl = FunctionDecl->getPreviousDecl()) {
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 d578eedd94a390..0dffaefa213a45 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
@@ -107,19 +107,6 @@ struct PositiveConstValueConstructor {
   // CHECK-FIXES: PositiveConstValueConstructor(const ExpensiveToCopyType& ConstCopy) {}
 };
 
-template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType S, T V) {
-  // CHECK-MESSAGES: [[@LINE-1]]:90: warning: the const qualified parameter 'S'
-  // CHECK-FIXES-NOT: template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType& S, T V) {
-}
-
-void instantiated() {
-  templateWithNonTemplatizedParameter(ExpensiveToCopyType(), ExpensiveToCopyType());
-  templateWithNonTemplatizedParameter(ExpensiveToCopyType(), 5);
-}
-
-template <typename T> void negativeTemplateType(const T V) {
-}
-
 void negativeArray(const ExpensiveToCopyType[]) {
 }
 
@@ -370,35 +357,3 @@ void fun() {
   ExpensiveToCopyType E;
   NegativeUsingConstructor S(E);
 }
-
-template<typename T>
-void templateFunction(T) {
-}
-
-template<>
-void templateFunction<ExpensiveToCopyType>(ExpensiveToCopyType E) {
-  // CHECK-MESSAGES: [[@LINE-1]]:64: warning: the parameter 'E' is copied
-  // CHECK-FIXES: void templateFunction<ExpensiveToCopyType>(ExpensiveToCopyType E) {
-  E.constReference();
-}
-
-template <class T>
-T templateSpecializationFunction(ExpensiveToCopyType E) {
-  // CHECK-MESSAGES: [[@LINE-1]]:54: warning: the parameter 'E' is copied
-  // CHECK-FIXES-NOT: T templateSpecializationFunction(const ExpensiveToCopyType& E) {
-  return T();
-}
-
-template <>
-bool templateSpecializationFunction(ExpensiveToCopyType E) {
-  // CHECK-MESSAGES: [[@LINE-1]]:57: warning: the parameter 'E' is copied
-  // CHECK-FIXES-NOT: bool templateSpecializationFunction(const ExpensiveToCopyType& E) {
-  return true;
-}
-
-template <>
-int templateSpecializationFunction(ExpensiveToCopyType E) {
-  // CHECK-MESSAGES: [[@LINE-1]]:56: warning: the parameter 'E' is copied
-  // CHECK-FIXES-NOT: int templateSpecializationFunction(const ExpensiveToCopyType& E) {
-  return 0;
-}