[clang-tools-extra] c98b3a8 - Fix `performance-unnecessary-value-param` for template specialization

[via cfe-commits]

Author: Sockke
Date: 2022-05-30T09:55:53+08:00
New Revision: c98b3a8cd9856f2fcc7ba1f9ed9896b291bbab7b

URL: https://github.com/llvm/llvm-project/commit/c98b3a8cd9856f2fcc7ba1f9ed9896b291bbab7b
DIFF: https://github.com/llvm/llvm-project/commit/c98b3a8cd9856f2fcc7ba1f9ed9896b291bbab7b.diff

LOG: Fix `performance-unnecessary-value-param` for template specialization

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

Reviewed By: aaron.ballman, flx

Differential Revision: https://reviews.llvm.org/D116593

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-delayed.cpp
    clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index ba2b89c291214..00f4b00f53819 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -51,18 +51,6 @@ bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Decl &Decl,
   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(
@@ -147,11 +135,12 @@ 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 an explicit template specialization.
+  // 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) ||
-      isExplicitTemplateSpecialization(*Function))
+      (Function->getTemplatedKind() != FunctionDecl::TK_NonTemplate))
     return;
   for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
        FunctionDecl = FunctionDecl->getPreviousDecl()) {

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 787f535dedb64..5196c53291d19 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -202,6 +202,10 @@ Changes in existing checks
   <clang-tidy/checks/readability-simplify-boolean-expr>` to simplify expressions
   using DeMorgan's Theorem.
 
+- Fixed a crash in :doc:`performance-unnecessary-value-param
+  <clang-tidy/checks/readability-suspicious-call-argument>` when the specialization
+  template has an unnecessary value paramter. Removed the fix for a template.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-delayed.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-delayed.cpp
index 623bf8b30829e..53ec8713be338 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-delayed.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-delayed.cpp
@@ -69,7 +69,7 @@ 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: template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType& S, T V) {
+  // CHECK-FIXES-NOT: template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType& S, T V) {
 }
 
 void instantiated() {

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 f801494cf0ff5..d578eedd94a39 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
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t
+// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t -- -- -fno-delayed-template-parsing
 
 // CHECK-FIXES: #include <utility>
 
@@ -109,7 +109,7 @@ 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: template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType& S, T V) {
+  // CHECK-FIXES-NOT: template <typename T> void templateWithNonTemplatizedParameter(const ExpensiveToCopyType& S, T V) {
 }
 
 void instantiated() {
@@ -381,3 +381,24 @@ void templateFunction<ExpensiveToCopyType>(ExpensiveToCopyType E) {
   // 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;
+}