[PATCH] D116593: Fix `performance-unnecessary-value-param` for template specialization

[gehry via Phabricator via cfe-commits]

Sockke created this revision.
Sockke added reviewers: aaron.ballman, flx, njames93.
Herald added a subscriber: carlosgalvezp.
Sockke requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The checker missed a check for parameter type of primary template of specialization template and this could cause build breakages.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116593

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp
@@ -381,3 +381,24 @@
   // 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: 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: 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: int templateSpecializationFunction(const ExpensiveToCopyType& E) {
+  return 0;
+}
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -51,6 +51,21 @@
   return Matches.empty();
 }
 
+bool isTemplateTypeParmOfPrimaryTemplate(const FunctionDecl &Function,
+                                         int Index) {
+  // Check the specific parameter type of primary template whether is template
+  // type.
+  if (const auto *PrimaryTemplateInfo = Function.getPrimaryTemplate()) {
+    if (const auto *PrimaryTemplateFunction =
+            PrimaryTemplateInfo->getTemplatedDecl()) {
+      const auto *ParamInfo = PrimaryTemplateFunction->getParamDecl(Index);
+      if (ParamInfo && ParamInfo->getOriginalType()->isTemplateTypeParmType())
+        return true;
+    }
+  }
+  return false;
+}
+
 bool isExplicitTemplateSpecialization(const FunctionDecl &Function) {
   if (const auto *SpecializationInfo = Function.getTemplateSpecializationInfo())
     if (SpecializationInfo->getTemplateSpecializationKind() ==
@@ -146,15 +161,21 @@
   // 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.
+  // 4. the function is an explicit template specialization and the specific
+  //    parameter of primary template is template type.
   const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Function);
+  bool IsExplicitTemplateSpecialization =
+      isExplicitTemplateSpecialization(*Function);
   if (Param->getBeginLoc().isMacroID() || (Method && Method->isVirtual()) ||
       isReferencedOutsideOfCallExpr(*Function, *Result.Context) ||
-      isExplicitTemplateSpecialization(*Function))
+      (IsExplicitTemplateSpecialization &&
+       isTemplateTypeParmOfPrimaryTemplate(*Function, Index)))
     return;
   for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
        FunctionDecl = FunctionDecl->getPreviousDecl()) {
     const auto &CurrentParam = *FunctionDecl->getParamDecl(Index);
+    if (IsExplicitTemplateSpecialization && Function != FunctionDecl)
+      continue;
     Diag << utils::fixit::changeVarDeclToReference(CurrentParam,
                                                    *Result.Context);
     // The parameter of each declaration needs to be checked individually as to


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116593.397259.patch
Type: text/x-patch
Size: 3827 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220104/5ad5729f/attachment-0001.bin>