[clang-tidy] Do not issue fixit for explicit template specializations

[flx]

Summary

Do not issue fixit in UnnecessaryValueParamCheck if the function is an explicit template specialization as this could cause build breakages.

[alexfh]

test/clang-tidy/performance-unnecessary-value-param.cpp

358

Did you mean to add a CHECK-FIXES to ensure the code stays unchanged?

[alexfh]

LG. Thanks!

clang-tidy/performance/UnnecessaryValueParamCheck.cpp

auto Matches =
       match(decl(forEachDescendant(declRefExpr(
                 equalsNode(&DeclRef),
                 unless(hasAncestor(stmt(anyOf(forStmt(), cxxForRangeStmt(),
                                               whileStmt(), doStmt()))))))),
             Decl, Context);
   return Matches.empty();
 }
 
+bool isExplicitTemplateSpecialization(const FunctionDecl &Function) {
+  if (const auto *SpecializationInfo = Function.getTemplateSpecializationInfo())
+    if (SpecializationInfo->getTemplateSpecializationKind() ==
+        TSK_ExplicitSpecialization)
+      return true;
+  if (const auto *Method = llvm::dyn_cast<CXXMethodDecl>(&Function))
+    if (Method->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization &&
+        Method->getMemberSpecializationInfo()->isExplicitSpecialization())
+      return true;
+  return false;
+}
+
 } // namespace
 
 UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
           Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
 
auto Diag =
                               "invocation but only used as a const reference; "
                               "consider making it a const reference")
       << paramNameOrIndex(Param->getName(), Index);
   // Do not propose fixes when:
   // 1. the ParmVarDecl is in a macro, since we cannot place them correctly
   // 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 an explicit template specialization.
   const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Function);
   if (Param->getLocStart().isMacroID() || (Method && Method->isVirtual()) ||
-      isReferencedOutsideOfCallExpr(*Function, *Result.Context))
+      isReferencedOutsideOfCallExpr(*Function, *Result.Context) ||
+      isExplicitTemplateSpecialization(*Function))
     return;
   for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
        FunctionDecl = FunctionDecl->getPreviousDecl()) {
     const auto &CurrentParam = *FunctionDecl->getParamDecl(Index);
     Diag << utils::fixit::changeVarDeclToReference(CurrentParam,
                                                    *Result.Context);
     // The parameter of each declaration needs to be checked individually as to
     // whether it is const or not as constness can differ between definition and

test/clang-tidy/performance-unnecessary-value-param.cpp

 struct NegativeUsingConstructor : public PositiveConstructor {
   using PositiveConstructor::PositiveConstructor;
 };
 
 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();
+}