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
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
