flx created this revision.
flx added reviewers: alexfh, sbenza.
flx added a subscriber: cfe-commits.
flx set the repository for this revision to rL LLVM.
Suppress fixes for functions that are referenced within the compilation unit
outside of a call expression as the signature change could break the code
referencing the function.
We still issue a warning in this case so that users can decide to manually
change the function signature.
Repository:
rL LLVM
https://reviews.llvm.org/D26203
Files:
clang-tidy/performance/UnnecessaryValueParamCheck.cpp
test/clang-tidy/performance-unnecessary-value-param.cpp
Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===================================================================
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -237,3 +237,21 @@
ExpensiveToCopyType B;
B = A;
}
+
+void PositiveOnlyMessageAsReferencedInCompilationUnit(ExpensiveToCopyType A) {
+ // CHECK-MESSAGES: [[@LINE-1]]:75: warning: the parameter 'A' is copied
+ // CHECK-FIXES: void
PositiveOnlyMessageAsReferencedInCompilationUnit(ExpensiveToCopyType A) {
+}
+
+void ReferenceFunctionOutsideOfCallExpr() {
+ void (*ptr)(ExpensiveToCopyType) =
&PositiveOnlyMessageAsReferencedInCompilationUnit;
+}
+
+void PositiveMessageAndFixAsFunctionIsCalled(ExpensiveToCopyType A) {
+ // CHECK-MESSAGES: [[@LINE-1]]:66: warning: the parameter 'A' is copied
+ // CHECK-FIXES: void PositiveMessageAndFixAsFunctionIsCalled(const
ExpensiveToCopyType& A) {
+}
+
+void ReferenceFunctionByCallingIt() {
+ PositiveMessageAndFixAsFunctionIsCalled(ExpensiveToCopyType());
+}
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -39,6 +39,14 @@
return true;
}
+bool isReferencedOutsideOfCallExpr(const FunctionDecl &Function,
+ ASTContext &Context) {
+ auto Matches = match(declRefExpr(to(functionDecl(equalsNode(&Function))),
+ unless(hasAncestor(callExpr()))),
+ Context);
+ return !Matches.empty();
+}
+
} // namespace
UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
@@ -118,10 +126,14 @@
"invocation but only used as a const reference; "
"consider making it a const reference")
<< paramNameOrIndex(Param->getName(), Index);
- // Do not propose fixes in macros since we cannot place them correctly, or if
- // function is virtual as it might break overrides.
+ // Do not propose fixes when:
+ // 1. in macros 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.
const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Function);
- if (Param->getLocStart().isMacroID() || (Method && Method->isVirtual()))
+ if (Param->getLocStart().isMacroID() || (Method && Method->isVirtual()) ||
+ isReferencedOutsideOfCallExpr(*Function, *Result.Context))
return;
for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
FunctionDecl = FunctionDecl->getPreviousDecl()) {
Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===================================================================
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -237,3 +237,21 @@
ExpensiveToCopyType B;
B = A;
}
+
+void PositiveOnlyMessageAsReferencedInCompilationUnit(ExpensiveToCopyType A) {
+ // CHECK-MESSAGES: [[@LINE-1]]:75: warning: the parameter 'A' is copied
+ // CHECK-FIXES: void PositiveOnlyMessageAsReferencedInCompilationUnit(ExpensiveToCopyType A) {
+}
+
+void ReferenceFunctionOutsideOfCallExpr() {
+ void (*ptr)(ExpensiveToCopyType) = &PositiveOnlyMessageAsReferencedInCompilationUnit;
+}
+
+void PositiveMessageAndFixAsFunctionIsCalled(ExpensiveToCopyType A) {
+ // CHECK-MESSAGES: [[@LINE-1]]:66: warning: the parameter 'A' is copied
+ // CHECK-FIXES: void PositiveMessageAndFixAsFunctionIsCalled(const ExpensiveToCopyType& A) {
+}
+
+void ReferenceFunctionByCallingIt() {
+ PositiveMessageAndFixAsFunctionIsCalled(ExpensiveToCopyType());
+}
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -39,6 +39,14 @@
return true;
}
+bool isReferencedOutsideOfCallExpr(const FunctionDecl &Function,
+ ASTContext &Context) {
+ auto Matches = match(declRefExpr(to(functionDecl(equalsNode(&Function))),
+ unless(hasAncestor(callExpr()))),
+ Context);
+ return !Matches.empty();
+}
+
} // namespace
UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
@@ -118,10 +126,14 @@
"invocation but only used as a const reference; "
"consider making it a const reference")
<< paramNameOrIndex(Param->getName(), Index);
- // Do not propose fixes in macros since we cannot place them correctly, or if
- // function is virtual as it might break overrides.
+ // Do not propose fixes when:
+ // 1. in macros 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.
const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Function);
- if (Param->getLocStart().isMacroID() || (Method && Method->isVirtual()))
+ if (Param->getLocStart().isMacroID() || (Method && Method->isVirtual()) ||
+ isReferencedOutsideOfCallExpr(*Function, *Result.Context))
return;
for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
FunctionDecl = FunctionDecl->getPreviousDecl()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
