Skip to content

Commit 721393e

Browse files
committed
[clang-tidy]improve performance-unnecessary-value-param performance
Tolerate fix-it breaking compilation when functions is used as pointers. `isReferencedOutsideOfCallExpr` will visit the whole translate unit for each matched function decls. It will waste lots of cpu time in some big cpp files. But the benefits of this validation are limited. Lots of function usage are out of current translation unit. After removing this validation step, the check profiling changes from 5.7 to 1.1 in SemaExprCXX.cpp, which is similar to version 18.
1 parent ed48398 commit 721393e

File tree

3 files changed

+7
-18
lines changed

3 files changed

+7
-18
lines changed

clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "UnnecessaryValueParamCheck.h"
10-
1110
#include "../utils/DeclRefExprUtils.h"
1211
#include "../utils/FixItHintUtils.h"
1312
#include "../utils/Matchers.h"
@@ -30,14 +29,6 @@ std::string paramNameOrIndex(StringRef Name, size_t Index) {
3029
.str();
3130
}
3231

33-
bool isReferencedOutsideOfCallExpr(const FunctionDecl &Function,
34-
ASTContext &Context) {
35-
auto Matches = match(declRefExpr(to(functionDecl(equalsNode(&Function))),
36-
unless(hasAncestor(callExpr()))),
37-
Context);
38-
return !Matches.empty();
39-
}
40-
4132
bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Decl &Decl,
4233
ASTContext &Context) {
4334
auto Matches = match(
@@ -155,12 +146,9 @@ void UnnecessaryValueParamCheck::handleConstRefFix(const FunctionDecl &Function,
155146
// Do not propose fixes when:
156147
// 1. the ParmVarDecl is in a macro, since we cannot place them correctly
157148
// 2. the function is virtual as it might break overrides
158-
// 3. the function is referenced outside of a call expression within the
159-
// compilation unit as the signature change could introduce build errors.
160-
// 4. the function is an explicit template/ specialization.
149+
// 3. the function is an explicit template/ specialization.
161150
const auto *Method = llvm::dyn_cast<CXXMethodDecl>(&Function);
162151
if (Param.getBeginLoc().isMacroID() || (Method && Method->isVirtual()) ||
163-
isReferencedOutsideOfCallExpr(Function, Context) ||
164152
Function.getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
165153
return;
166154
for (const auto *FunctionDecl = &Function; FunctionDecl != nullptr;

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,11 @@ Changes in existing checks
110110
<clang-tidy/checks/misc/redundant-expression>` check by providing additional
111111
examples and fixing some macro related false positives.
112112

113+
- Improved :doc:`performance/unnecessary-value-param
114+
<clang-tidy/checks/performance/unnecessary-value-param>` check performance by
115+
tolerating fix-it breaking compilation when functions is used as pointers
116+
to avoid matching usage of functions within the current compilation unit.
117+
113118
Removed checks
114119
^^^^^^^^^^^^^^
115120

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,11 +332,7 @@ void PositiveNonConstDeclaration(const ExpensiveToCopyType A) {
332332

333333
void PositiveOnlyMessageAsReferencedInCompilationUnit(ExpensiveToCopyType A) {
334334
// CHECK-MESSAGES: [[@LINE-1]]:75: warning: the parameter 'A' is copied
335-
// CHECK-FIXES: void PositiveOnlyMessageAsReferencedInCompilationUnit(ExpensiveToCopyType A) {
336-
}
337-
338-
void ReferenceFunctionOutsideOfCallExpr() {
339-
void (*ptr)(ExpensiveToCopyType) = &PositiveOnlyMessageAsReferencedInCompilationUnit;
335+
// CHECK-FIXES: void PositiveOnlyMessageAsReferencedInCompilationUnit(const ExpensiveToCopyType& A) {
340336
}
341337

342338
void PositiveMessageAndFixAsFunctionIsCalled(ExpensiveToCopyType A) {

0 commit comments

Comments
 (0)