Allow unnecessary-value-param to match templated functions including lambdas with auto.

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,11 @@ 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.
+  // 4. the function is 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))
+      Function->getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
     return;
   for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
        FunctionDecl = FunctionDecl->getPreviousDecl()) {

clang-tools-extra/docs/ReleaseNotes.rst

@@ -418,6 +418,12 @@ Changes in existing checks
   Calls to mutable function where there exists a `const` overload are also
   handled. Fix crash in the case of a non-member operator call.
 
+- Improved :doc:`performance-unnecessary-value-param
+  <clang-tidy/checks/performance/unnecessary-value-param>` check
+  detecting more cases for template functions including lambdas with ``auto``.
+  E.g., ``std::sort(a.begin(), a.end(), [](auto x, auto y) { return a > b; });``
+  will be detected for expensive to copy types.
+
 - Improved :doc:`readability-avoid-return-with-void-value
   <clang-tidy/checks/readability/avoid-return-with-void-value>` check by adding
   fix-its.

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-delayed.cpp

@@ -69,7 +69,8 @@ struct PositiveConstValueConstructor {
 
 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) {
+  // CHECK-MESSAGES: [[@LINE-2]]:95: warning: the parameter 'V'
+  // CHECK-FIXES: template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType& S, const T& V) {
 }
 
 void instantiated() {

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param-templates.cpp

@@ -0,0 +1,98 @@
+// RUN: %check_clang_tidy  -std=c++14-or-later %s performance-unnecessary-value-param %t
+
+struct ExpensiveToCopyType {
+  virtual ~ExpensiveToCopyType();
+};
+
+template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType S, T V) {
+  // CHECK-MESSAGES: [[@LINE-1]]:90: warning: the const qualified parameter 'S'
+  // CHECK-MESSAGES: [[@LINE-2]]:95: warning: the parameter 'V'
+  // CHECK-FIXES: template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType& S, const T& V) {
+}
+
+void instantiatedWithExpensiveValue() {
+  templateWithNonTemplatizedParameter(
+      ExpensiveToCopyType(), ExpensiveToCopyType());
+  templateWithNonTemplatizedParameter(ExpensiveToCopyType(), 5);
+}
+
+template <typename T> void templateWithNonTemplatizedParameterCheapTemplate(const ExpensiveToCopyType S, T V) {
+  // CHECK-MESSAGES: [[@LINE-1]]:103: warning: the const qualified parameter 'S'
+  // CHECK-FIXES: template <typename T> void templateWithNonTemplatizedParameterCheapTemplate(const ExpensiveToCopyType& S, T V) {
+}
+
+void instantiatedWithCheapValue() {
+  templateWithNonTemplatizedParameterCheapTemplate(ExpensiveToCopyType(), 5);
+}
+
+template <typename T> void nonInstantiatedTemplateWithConstValue(const T S) {}
+template <typename T> void nonInstantiatedTemplateWithNonConstValue(T S) {}
+
+template <typename T> void instantiatedTemplateSpecialization(T NoSpecS) {}
+template <> void instantiatedTemplateSpecialization<int>(int SpecSInt) {}
+// Updating template specialization would also require to update the main
+// template and other specializations. Such specializations may be
+// spreaded across different translation units.
+// For that reason we only issue a warning, but do not propose fixes.
+template <>
+void instantiatedTemplateSpecialization<ExpensiveToCopyType>(
+    ExpensiveToCopyType SpecSExpensiveToCopy) {
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: the parameter 'SpecSExpensiveToCopy'
+  // CHECK-FIXES-NOT: const T& NoSpecS
+  // CHECK-FIXES-NOT: const int& SpecSInt
+  // CHECK-FIXES-NOT: const ExpensiveToCopyType& SpecSExpensiveToCopy
+}
+
+void instantiatedTemplateSpecialization() {
+  instantiatedTemplateSpecialization(ExpensiveToCopyType());
+}
+
+template <typename T> void instantiatedTemplateWithConstValue(const T S) {
+  // CHECK-MESSAGES: [[@LINE-1]]:71: warning: the const qualified parameter 'S'
+  // CHECK-FIXES: template <typename T> void instantiatedTemplateWithConstValue(const T& S) {
+}
+
+void instantiatedConstValue() {
+  instantiatedTemplateWithConstValue(ExpensiveToCopyType());
+}
+
+template <typename T> void instantiatedTemplateWithNonConstValue(T S) {
+  // CHECK-MESSAGES: [[@LINE-1]]:68: warning: the parameter 'S'
+  // CHECK-FIXES: template <typename T> void instantiatedTemplateWithNonConstValue(const T& S) {
+}
+
+void instantiatedNonConstValue() {
+  instantiatedTemplateWithNonConstValue(ExpensiveToCopyType());
+}
+
+void lambdaConstValue() {
+  auto fn = [](const ExpensiveToCopyType S) {
+    // CHECK-MESSAGES: [[@LINE-1]]:42: warning: the const qualified parameter 'S'
+    // CHECK-FIXES: auto fn = [](const ExpensiveToCopyType& S) {
+  };
+  fn(ExpensiveToCopyType());
+}
+
+void lambdaNonConstValue() {
+  auto fn = [](ExpensiveToCopyType S) {
+    // CHECK-MESSAGES: [[@LINE-1]]:36: warning: the parameter 'S'
+    // CHECK-FIXES: auto fn = [](const ExpensiveToCopyType& S) {
+  };
+  fn(ExpensiveToCopyType());
+}
+
+void lambdaConstAutoValue() {
+  auto fn = [](const auto S) {
+    // CHECK-MESSAGES: [[@LINE-1]]:27: warning: the const qualified parameter 'S'
+    // CHECK-FIXES: auto fn = [](const auto& S) {
+  };
+  fn(ExpensiveToCopyType());
+}
+
+void lambdaNonConstAutoValue() {
+  auto fn = [](auto S) {
+    // CHECK-MESSAGES: [[@LINE-1]]:21: warning: the parameter 'S'
+    // CHECK-FIXES: auto fn = [](const auto& S) {
+  };
+  fn(ExpensiveToCopyType());
+}

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;
-}