Skip to content

Commit dd23b34

Browse files
authored
[clang-tidy][performance-unnecessary-value-param] Make handleMoveFix virtual (#99867)
... so that downstream checks can override behaviour to do additional processing. Refactor the rest of the logic to `handleConstRefFix` (which is also `virtual`). This is otherwise and NFC. This is similar to #73921 but for `performance-unnecessary-value-param`.
1 parent 5f05d5e commit dd23b34

File tree

2 files changed

+47
-34
lines changed

2 files changed

+47
-34
lines changed

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

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -119,65 +119,72 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
119119
}
120120
}
121121

122-
const size_t Index = llvm::find(Function->parameters(), Param) -
123-
Function->parameters().begin();
122+
handleConstRefFix(*Function, *Param, *Result.Context);
123+
}
124+
125+
void UnnecessaryValueParamCheck::registerPPCallbacks(
126+
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
127+
Inserter.registerPreprocessor(PP);
128+
}
129+
130+
void UnnecessaryValueParamCheck::storeOptions(
131+
ClangTidyOptions::OptionMap &Opts) {
132+
Options.store(Opts, "IncludeStyle", Inserter.getStyle());
133+
Options.store(Opts, "AllowedTypes",
134+
utils::options::serializeStringList(AllowedTypes));
135+
}
136+
137+
void UnnecessaryValueParamCheck::onEndOfTranslationUnit() {
138+
MutationAnalyzerCache.clear();
139+
}
140+
141+
void UnnecessaryValueParamCheck::handleConstRefFix(const FunctionDecl &Function,
142+
const ParmVarDecl &Param,
143+
ASTContext &Context) {
144+
const size_t Index =
145+
llvm::find(Function.parameters(), &Param) - Function.parameters().begin();
146+
const bool IsConstQualified =
147+
Param.getType().getCanonicalType().isConstQualified();
124148

125149
auto Diag =
126-
diag(Param->getLocation(),
150+
diag(Param.getLocation(),
127151
"the %select{|const qualified }0parameter %1 is copied for each "
128152
"invocation%select{ but only used as a const reference|}0; consider "
129153
"making it a %select{const |}0reference")
130-
<< IsConstQualified << paramNameOrIndex(Param->getName(), Index);
154+
<< IsConstQualified << paramNameOrIndex(Param.getName(), Index);
131155
// Do not propose fixes when:
132156
// 1. the ParmVarDecl is in a macro, since we cannot place them correctly
133157
// 2. the function is virtual as it might break overrides
134158
// 3. the function is referenced outside of a call expression within the
135159
// compilation unit as the signature change could introduce build errors.
136160
// 4. the function is an explicit template/ specialization.
137-
const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Function);
138-
if (Param->getBeginLoc().isMacroID() || (Method && Method->isVirtual()) ||
139-
isReferencedOutsideOfCallExpr(*Function, *Result.Context) ||
140-
Function->getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
161+
const auto *Method = llvm::dyn_cast<CXXMethodDecl>(&Function);
162+
if (Param.getBeginLoc().isMacroID() || (Method && Method->isVirtual()) ||
163+
isReferencedOutsideOfCallExpr(Function, Context) ||
164+
Function.getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
141165
return;
142-
for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
166+
for (const auto *FunctionDecl = &Function; FunctionDecl != nullptr;
143167
FunctionDecl = FunctionDecl->getPreviousDecl()) {
144168
const auto &CurrentParam = *FunctionDecl->getParamDecl(Index);
145-
Diag << utils::fixit::changeVarDeclToReference(CurrentParam,
146-
*Result.Context);
169+
Diag << utils::fixit::changeVarDeclToReference(CurrentParam, Context);
147170
// The parameter of each declaration needs to be checked individually as to
148171
// whether it is const or not as constness can differ between definition and
149172
// declaration.
150173
if (!CurrentParam.getType().getCanonicalType().isConstQualified()) {
151174
if (std::optional<FixItHint> Fix = utils::fixit::addQualifierToVarDecl(
152-
CurrentParam, *Result.Context, DeclSpec::TQ::TQ_const))
175+
CurrentParam, Context, DeclSpec::TQ::TQ_const))
153176
Diag << *Fix;
154177
}
155178
}
156179
}
157180

158-
void UnnecessaryValueParamCheck::registerPPCallbacks(
159-
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
160-
Inserter.registerPreprocessor(PP);
161-
}
162-
163-
void UnnecessaryValueParamCheck::storeOptions(
164-
ClangTidyOptions::OptionMap &Opts) {
165-
Options.store(Opts, "IncludeStyle", Inserter.getStyle());
166-
Options.store(Opts, "AllowedTypes",
167-
utils::options::serializeStringList(AllowedTypes));
168-
}
169-
170-
void UnnecessaryValueParamCheck::onEndOfTranslationUnit() {
171-
MutationAnalyzerCache.clear();
172-
}
173-
174-
void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Var,
181+
void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Param,
175182
const DeclRefExpr &CopyArgument,
176-
const ASTContext &Context) {
183+
ASTContext &Context) {
177184
auto Diag = diag(CopyArgument.getBeginLoc(),
178185
"parameter %0 is passed by value and only copied once; "
179186
"consider moving it to avoid unnecessary copies")
180-
<< &Var;
187+
<< &Param;
181188
// Do not propose fixes in macros since we cannot place them correctly.
182189
if (CopyArgument.getBeginLoc().isMacroID())
183190
return;

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,16 @@ class UnnecessaryValueParamCheck : public ClangTidyCheck {
3333
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
3434
void onEndOfTranslationUnit() override;
3535

36-
private:
37-
void handleMoveFix(const ParmVarDecl &Var, const DeclRefExpr &CopyArgument,
38-
const ASTContext &Context);
36+
protected:
37+
// Create diagnostics. These are virtual so that derived classes can change
38+
// behaviour.
39+
virtual void handleMoveFix(const ParmVarDecl &Param,
40+
const DeclRefExpr &CopyArgument,
41+
ASTContext &Context);
42+
virtual void handleConstRefFix(const FunctionDecl &Function,
43+
const ParmVarDecl &Param, ASTContext &Context);
3944

45+
private:
4046
ExprMutationAnalyzer::Memoized MutationAnalyzerCache;
4147
utils::IncludeInserter Inserter;
4248
const std::vector<StringRef> AllowedTypes;

0 commit comments

Comments
 (0)