[clang-tidy][NFC] Enable performance-unnecessary-value-param in the codebase (#163686)

Closes #156156.

In a few cases, instead of just applying the fix-it and making
parameters const references to owning type, I refactored them to be
non-owning types.
This commit is contained in:
Victor Chernyakin 2025-11-09 21:24:29 -07:00 committed by GitHub
parent 4d88bb6c63
commit 0d786b9a20
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
26 changed files with 59 additions and 58 deletions

View File

@ -15,7 +15,6 @@ Checks: >
performance-*,
-performance-enum-size,
-performance-no-int-to-ptr,
-performance-unnecessary-value-param,
readability-*,
-readability-avoid-nested-conditional-operator,
-readability-braces-around-statements,

View File

@ -455,8 +455,8 @@ ClangTidyASTConsumerFactory::createASTConsumer(
if (Context.canEnableModuleHeadersParsing() &&
Context.getLangOpts().Modules && OverlayFS != nullptr) {
auto ModuleExpander =
std::make_unique<ExpandModularHeadersPPCallbacks>(&Compiler, OverlayFS);
auto ModuleExpander = std::make_unique<ExpandModularHeadersPPCallbacks>(
&Compiler, *OverlayFS);
ModuleExpanderPP = ModuleExpander->getPreprocessor();
PP->addPPCallbacks(std::move(ModuleExpander));
}

View File

@ -65,8 +65,7 @@ private:
};
ExpandModularHeadersPPCallbacks::ExpandModularHeadersPPCallbacks(
CompilerInstance *CI,
IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS)
CompilerInstance *CI, llvm::vfs::OverlayFileSystem &OverlayFS)
: Recorder(std::make_unique<FileRecorder>()), Compiler(*CI),
InMemoryFs(new llvm::vfs::InMemoryFileSystem),
Sources(Compiler.getSourceManager()),
@ -76,7 +75,7 @@ ExpandModularHeadersPPCallbacks::ExpandModularHeadersPPCallbacks(
LangOpts(Compiler.getLangOpts()), HSOpts(Compiler.getHeaderSearchOpts()) {
// Add a FileSystem containing the extra files needed in place of modular
// headers.
OverlayFS->pushOverlay(InMemoryFs);
OverlayFS.pushOverlay(InMemoryFs);
Diags.setSourceManager(&Sources);
// FIXME: Investigate whatever is there better way to initialize DiagEngine

View File

@ -41,9 +41,8 @@ namespace tooling {
/// non-modular way.
class ExpandModularHeadersPPCallbacks : public PPCallbacks {
public:
ExpandModularHeadersPPCallbacks(
CompilerInstance *CI,
IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFS);
ExpandModularHeadersPPCallbacks(CompilerInstance *CI,
llvm::vfs::OverlayFileSystem &OverlayFS);
~ExpandModularHeadersPPCallbacks() override;
/// Returns the preprocessor that provides callbacks for the whole

View File

@ -36,7 +36,7 @@ static std::string buildFixMsgForStringFlag(const Expr *Arg,
}
void CloexecCheck::registerMatchersImpl(
MatchFinder *Finder, internal::Matcher<FunctionDecl> Function) {
MatchFinder *Finder, const internal::Matcher<FunctionDecl> &Function) {
// We assume all the checked APIs are C functions.
Finder->addMatcher(
callExpr(

View File

@ -29,9 +29,9 @@ public:
: ClangTidyCheck(Name, Context) {}
protected:
void
registerMatchersImpl(ast_matchers::MatchFinder *Finder,
ast_matchers::internal::Matcher<FunctionDecl> Function);
void registerMatchersImpl(
ast_matchers::MatchFinder *Finder,
const ast_matchers::internal::Matcher<FunctionDecl> &Function);
/// Currently, we have three types of fixes.
///

View File

@ -18,6 +18,7 @@
#include <initializer_list>
#include <optional>
#include <string>
#include <utility>
// FixItHint - Let the docs script know that this class does provide fixits
@ -217,11 +218,11 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
const auto AddFromStd =
[&](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
std::initializer_list<StringRef> Names) {
AddFrom(Replacer, Names, "std");
AddFrom(std::move(Replacer), Names, "std");
};
const auto AddFromBoost =
[&](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
[&](const llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> &Replacer,
std::initializer_list<
std::pair<StringRef, std::initializer_list<StringRef>>>
NamespaceAndNames) {

View File

@ -64,7 +64,8 @@ void RawMemoryCallOnNonTrivialTypeCheck::storeOptions(
void RawMemoryCallOnNonTrivialTypeCheck::registerMatchers(MatchFinder *Finder) {
using namespace ast_matchers::internal;
auto IsStructPointer = [](Matcher<CXXRecordDecl> Constraint = anything(),
auto IsStructPointer = [](const Matcher<CXXRecordDecl> &Constraint =
anything(),
bool Bind = false) {
return expr(unaryOperator(
hasOperatorName("&"),
@ -74,8 +75,8 @@ void RawMemoryCallOnNonTrivialTypeCheck::registerMatchers(MatchFinder *Finder) {
};
auto IsRecordSizeOf =
expr(sizeOfExpr(hasArgumentOfType(equalsBoundNode("Record"))));
auto ArgChecker = [&](Matcher<CXXRecordDecl> RecordConstraint,
BindableMatcher<Stmt> SecondArg = expr()) {
auto ArgChecker = [&](const Matcher<CXXRecordDecl> &RecordConstraint,
const BindableMatcher<Stmt> &SecondArg = expr()) {
return allOf(argumentCountIs(3),
hasArgument(0, IsStructPointer(RecordConstraint, true)),
hasArgument(1, SecondArg), hasArgument(2, IsRecordSizeOf));

View File

@ -435,7 +435,7 @@ void SignalHandlerCheck::check(const MatchFinder::MatchResult &Result) {
bool SignalHandlerCheck::checkFunction(
const FunctionDecl *FD, const Expr *CallOrRef,
std::function<void(bool)> ChainReporter) {
llvm::function_ref<void(bool)> ChainReporter) {
const bool FunctionIsCalled = isa<CallExpr>(CallOrRef);
if (isStandardFunction(FD)) {
@ -471,7 +471,7 @@ bool SignalHandlerCheck::checkFunction(
bool SignalHandlerCheck::checkFunctionCPP14(
const FunctionDecl *FD, const Expr *CallOrRef,
std::function<void(bool)> ChainReporter) {
llvm::function_ref<void(bool)> ChainReporter) {
if (!FD->isExternC()) {
diag(CallOrRef->getBeginLoc(),
"functions without C linkage are not allowed as signal "

View File

@ -48,10 +48,10 @@ private:
/// The bool parameter is used like \c SkipPathEnd in \c reportHandlerChain .
/// \return Returns true if a diagnostic was emitted for this function.
bool checkFunction(const FunctionDecl *FD, const Expr *CallOrRef,
std::function<void(bool)> ChainReporter);
llvm::function_ref<void(bool)> ChainReporter);
/// Similar as \c checkFunction but only check for C++14 rules.
bool checkFunctionCPP14(const FunctionDecl *FD, const Expr *CallOrRef,
std::function<void(bool)> ChainReporter);
llvm::function_ref<void(bool)> ChainReporter);
/// Returns true if a standard library function is considered
/// asynchronous-safe.
bool isStandardFunctionAsyncSafe(const FunctionDecl *FD) const;

View File

@ -37,8 +37,8 @@ void UnusedRaiiCheck::registerMatchers(MatchFinder *Finder) {
}
template <typename T>
static void reportDiagnostic(DiagnosticBuilder D, const T *Node, SourceRange SR,
bool DefaultConstruction) {
static void reportDiagnostic(const DiagnosticBuilder &D, const T *Node,
SourceRange SR, bool DefaultConstruction) {
const char *Replacement = " give_me_a_name";
// If this is a default ctor we have to remove the parens or we'll introduce a

View File

@ -93,7 +93,7 @@ struct DenseMapInfo<
"TOMBSTONE"};
}
static unsigned getHashValue(ClassDefId Val) {
static unsigned getHashValue(const ClassDefId &Val) {
assert(Val != getEmptyKey() && "Cannot hash the empty key!");
assert(Val != getTombstoneKey() && "Cannot hash the tombstone key!");

View File

@ -22,11 +22,11 @@ namespace {
class RestrictedIncludesPPCallbacks
: public portability::RestrictedIncludesPPCallbacks {
public:
explicit RestrictedIncludesPPCallbacks(
RestrictSystemLibcHeadersCheck &Check, const SourceManager &SM,
const SmallString<128> CompilerIncudeDir)
explicit RestrictedIncludesPPCallbacks(RestrictSystemLibcHeadersCheck &Check,
const SourceManager &SM,
SmallString<128> CompilerIncudeDir)
: portability::RestrictedIncludesPPCallbacks(Check, SM),
CompilerIncudeDir(CompilerIncudeDir) {}
CompilerIncudeDir(std::move(CompilerIncudeDir)) {}
void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
StringRef FileName, bool IsAngled,
@ -61,7 +61,7 @@ void RestrictSystemLibcHeadersCheck::registerPPCallbacks(
StringRef(PP->getHeaderSearchInfo().getHeaderSearchOpts().ResourceDir);
llvm::sys::path::append(CompilerIncudeDir, "include");
PP->addPPCallbacks(std::make_unique<RestrictedIncludesPPCallbacks>(
*this, SM, CompilerIncudeDir));
*this, SM, std::move(CompilerIncudeDir)));
}
} // namespace clang::tidy::llvm_libc

View File

@ -1147,16 +1147,18 @@ void RedundantExpressionCheck::checkArithmeticExpr(
}
}
static bool exprEvaluatesToZero(BinaryOperatorKind Opcode, APSInt Value) {
static bool exprEvaluatesToZero(BinaryOperatorKind Opcode,
const APSInt &Value) {
return (Opcode == BO_And || Opcode == BO_AndAssign) && Value == 0;
}
static bool exprEvaluatesToBitwiseNegatedZero(BinaryOperatorKind Opcode,
APSInt Value) {
const APSInt &Value) {
return (Opcode == BO_Or || Opcode == BO_OrAssign) && ~Value == 0;
}
static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode, APSInt Value) {
static bool exprEvaluatesToSymbolic(BinaryOperatorKind Opcode,
const APSInt &Value) {
return ((Opcode == BO_Or || Opcode == BO_OrAssign) && Value == 0) ||
((Opcode == BO_And || Opcode == BO_AndAssign) && ~Value == 0);
}

View File

@ -92,7 +92,7 @@ static StatementMatcher incrementVarMatcher() {
}
static StatementMatcher
arrayConditionMatcher(internal::Matcher<Expr> LimitExpr) {
arrayConditionMatcher(const internal::Matcher<Expr> &LimitExpr) {
return binaryOperator(
anyOf(allOf(hasOperatorName("<"), hasLHS(integerComparisonMatcher()),
hasRHS(LimitExpr)),

View File

@ -81,16 +81,17 @@ AST_MATCHER_P(clang::Expr, anyOfExhaustive, std::vector<Matcher<clang::Stmt>>,
// literals.
struct MatchBuilder {
auto
ignoreParenAndArithmeticCasting(const Matcher<clang::Expr> Matcher) const {
ignoreParenAndArithmeticCasting(const Matcher<clang::Expr> &Matcher) const {
return expr(hasType(qualType(isArithmetic())), ignoringParenCasts(Matcher));
}
auto ignoreParenAndFloatingCasting(const Matcher<clang::Expr> Matcher) const {
auto
ignoreParenAndFloatingCasting(const Matcher<clang::Expr> &Matcher) const {
return expr(hasType(qualType(isFloating())), ignoringParenCasts(Matcher));
}
auto matchMathCall(const StringRef FunctionName,
const Matcher<clang::Expr> ArgumentMatcher) const {
const Matcher<clang::Expr> &ArgumentMatcher) const {
auto HasAnyPrecisionName = hasAnyName(
FunctionName, (FunctionName + "l").str(),
(FunctionName + "f").str()); // Support long double(l) and float(f).
@ -100,7 +101,7 @@ struct MatchBuilder {
hasArgument(0, ArgumentMatcher))));
}
auto matchSqrt(const Matcher<clang::Expr> ArgumentMatcher) const {
auto matchSqrt(const Matcher<clang::Expr> &ArgumentMatcher) const {
return matchMathCall("sqrt", ArgumentMatcher);
}
@ -148,7 +149,7 @@ struct MatchBuilder {
return expr(anyOf(Int, Float, Dref));
}
auto match1Div(const Matcher<clang::Expr> Match) const {
auto match1Div(const Matcher<clang::Expr> &Match) const {
return binaryOperator(hasOperatorName("/"), hasLHS(matchValue(1)),
hasRHS(Match));
}

View File

@ -70,8 +70,8 @@ void UseStdPrintCheck::registerPPCallbacks(const SourceManager &SM,
this->PP = PP;
}
static clang::ast_matchers::StatementMatcher
unusedReturnValue(clang::ast_matchers::StatementMatcher MatchedCallExpr) {
static clang::ast_matchers::StatementMatcher unusedReturnValue(
const clang::ast_matchers::StatementMatcher &MatchedCallExpr) {
auto UnusedInCompoundStmt =
compoundStmt(forEach(MatchedCallExpr),
// The checker can't currently differentiate between the

View File

@ -23,7 +23,7 @@ namespace clang::tidy::portability {
class RestrictSystemIncludesCheck : public ClangTidyCheck {
public:
RestrictSystemIncludesCheck(StringRef Name, ClangTidyContext *Context,
std::string DefaultAllowedIncludes = "*")
StringRef DefaultAllowedIncludes = "*")
: ClangTidyCheck(Name, Context),
AllowedIncludes(Options.get("Includes", DefaultAllowedIncludes)),
AllowedIncludesGlobList(AllowedIncludes) {}
@ -36,7 +36,7 @@ public:
}
private:
std::string AllowedIncludes;
StringRef AllowedIncludes;
GlobList AllowedIncludesGlobList;
};

View File

@ -47,9 +47,8 @@ void ContainerContainsCheck::registerMatchers(MatchFinder *Finder) {
const auto StringNpos = anyOf(declRefExpr(to(varDecl(hasName("npos")))),
memberExpr(member(hasName("npos"))));
auto AddSimpleMatcher = [&](auto Matcher) {
Finder->addMatcher(
traverse(TK_IgnoreUnlessSpelledInSource, std::move(Matcher)), this);
auto AddSimpleMatcher = [&](const auto &Matcher) {
Finder->addMatcher(traverse(TK_IgnoreUnlessSpelledInSource, Matcher), this);
};
// Find membership tests which use `count()`.

View File

@ -466,10 +466,9 @@ createOptionsProvider(llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) {
}
static llvm::IntrusiveRefCntPtr<vfs::FileSystem>
getVfsFromFile(const std::string &OverlayFile,
llvm::IntrusiveRefCntPtr<vfs::FileSystem> BaseFS) {
getVfsFromFile(const std::string &OverlayFile, vfs::FileSystem &BaseFS) {
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
BaseFS->getBufferForFile(OverlayFile);
BaseFS.getBufferForFile(OverlayFile);
if (!Buffer) {
llvm::errs() << "Can't load virtual filesystem overlay file '"
<< OverlayFile << "': " << Buffer.getError().message()
@ -585,7 +584,7 @@ static llvm::IntrusiveRefCntPtr<vfs::OverlayFileSystem> createBaseFS() {
if (!VfsOverlay.empty()) {
IntrusiveRefCntPtr<vfs::FileSystem> VfsFromFile =
getVfsFromFile(VfsOverlay, BaseFS);
getVfsFromFile(VfsOverlay, *BaseFS);
if (!VfsFromFile)
return nullptr;
BaseFS->pushOverlay(std::move(VfsFromFile));

View File

@ -141,7 +141,7 @@ BraceInsertionHints getBraceInsertionsHints(const Stmt *const S,
// StartLoc points at the location of the opening brace to be inserted.
SourceLocation EndLoc;
std::string ClosingInsertion;
StringRef ClosingInsertion;
if (EndLocHint.isValid()) {
EndLoc = EndLocHint;
ClosingInsertion = "} ";

View File

@ -39,7 +39,7 @@ struct BraceInsertionHints {
/// Constructor for a hint offering fix-its for brace insertion. Both
/// positions must be valid.
BraceInsertionHints(SourceLocation OpeningBracePos,
SourceLocation ClosingBracePos, std::string ClosingBrace)
SourceLocation ClosingBracePos, StringRef ClosingBrace)
: DiagnosticPos(OpeningBracePos), OpeningBracePos(OpeningBracePos),
ClosingBracePos(ClosingBracePos), ClosingBrace(ClosingBrace) {
assert(offersFixIts());
@ -64,7 +64,7 @@ struct BraceInsertionHints {
private:
SourceLocation OpeningBracePos;
SourceLocation ClosingBracePos;
std::string ClosingBrace;
StringRef ClosingBrace;
};
/// Create fix-it hints for braces that wrap the given statement when applied.

View File

@ -192,7 +192,7 @@ static bool isFunctionPointerConvertible(QualType From, QualType To) {
//
// The function should only be called in C++ mode.
static bool isQualificationConvertiblePointer(QualType From, QualType To,
LangOptions LangOpts) {
const LangOptions &LangOpts) {
// [N4659 7.5 (1)]
// A cv-decomposition of a type T is a sequence of cv_i and P_i such that T is

View File

@ -66,7 +66,7 @@ TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name,
// we would be accessing `getLangOpts` and `Options` before the underlying
// `ClangTidyCheck` instance was properly initialized.
TransformerClangTidyCheck::TransformerClangTidyCheck(
std::function<std::optional<RewriteRuleWith<std::string>>(
llvm::function_ref<std::optional<RewriteRuleWith<std::string>>(
const LangOptions &, const OptionsView &)>
MakeRule,
StringRef Name, ClangTidyContext *Context)

View File

@ -48,8 +48,9 @@ public:
///
/// See \c setRule for constraints on the rule.
TransformerClangTidyCheck(
std::function<std::optional<transformer::RewriteRuleWith<std::string>>(
const LangOptions &, const OptionsView &)>
llvm::function_ref<
std::optional<transformer::RewriteRuleWith<std::string>>(
const LangOptions &, const OptionsView &)>
MakeRule,
StringRef Name, ClangTidyContext *Context);

View File

@ -55,7 +55,7 @@ AST_MATCHER(Expr, hasSideEffects) {
} // namespace
static auto
makeExprMatcher(ast_matchers::internal::Matcher<Expr> ArgumentMatcher,
makeExprMatcher(const ast_matchers::internal::Matcher<Expr> &ArgumentMatcher,
ArrayRef<StringRef> MethodNames,
ArrayRef<StringRef> FreeNames) {
return expr(