-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[clang-tidy][NFC] Enable 'readability-named-parameter' check #158774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[clang-tidy][NFC] Enable 'readability-named-parameter' check #158774
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
✅ With the latest revision this PR passed the C/C++ code formatter. |
5ea20be
to
023d215
Compare
023d215
to
f32f0e6
Compare
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: None (capitan-davide) ChangesCloses #156159 Patch is 25.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/158774.diff 16 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/.clang-tidy b/clang-tools-extra/clang-tidy/.clang-tidy
index d290901730405..c0401a7da8578 100644
--- a/clang-tools-extra/clang-tidy/.clang-tidy
+++ b/clang-tools-extra/clang-tidy/.clang-tidy
@@ -27,7 +27,6 @@ Checks: >
-readability-implicit-bool-conversion,
-readability-isolate-declaration,
-readability-magic-numbers,
- -readability-named-parameter,
-readability-qualified-auto,
-readability-simplify-boolean-expr,
-readability-static-definition-in-anonymous-namespace,
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index e59f157b468bc..ec24eb3e688fb 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -66,14 +66,14 @@ template <> struct MappingTraits<ClangTidyOptions::StringPair> {
};
struct NOptionMap {
- NOptionMap(IO &) {}
- NOptionMap(IO &, const ClangTidyOptions::OptionMap &OptionMap) {
+ NOptionMap(IO & /*unused*/) {}
+ NOptionMap(IO & /*unused*/, const ClangTidyOptions::OptionMap &OptionMap) {
Options.reserve(OptionMap.size());
for (const auto &KeyValue : OptionMap)
Options.emplace_back(std::string(KeyValue.getKey()),
KeyValue.getValue().Value);
}
- ClangTidyOptions::OptionMap denormalize(IO &) {
+ ClangTidyOptions::OptionMap denormalize(IO & /*unused*/) {
ClangTidyOptions::OptionMap Map;
for (const auto &KeyValue : Options)
Map[KeyValue.first] = ClangTidyOptions::ClangTidyValue(KeyValue.second);
@@ -83,7 +83,7 @@ struct NOptionMap {
};
template <>
-void yamlize(IO &IO, ClangTidyOptions::OptionMap &Val, bool,
+void yamlize(IO &IO, ClangTidyOptions::OptionMap &Val, bool /*unused*/,
EmptyContext &Ctx) {
if (IO.outputting()) {
// Ensure check options are sorted
@@ -130,7 +130,8 @@ struct ChecksVariant {
std::optional<std::vector<std::string>> AsVector;
};
-template <> void yamlize(IO &IO, ChecksVariant &Val, bool, EmptyContext &Ctx) {
+template <>
+void yamlize(IO &IO, ChecksVariant &Val, bool /*unused*/, EmptyContext &Ctx) {
if (!IO.outputting()) {
// Special case for reading from YAML
// Must support reading from both a string or a list
diff --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
index 487e5e299d132..f43dde7154fc7 100644
--- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -181,65 +181,63 @@ void ExpandModularHeadersPPCallbacks::EndOfMainFile() {
// Handle all other callbacks.
// Just parse to the corresponding location to generate the same callback for
// the PPCallbacks registered in our custom preprocessor.
-void ExpandModularHeadersPPCallbacks::Ident(SourceLocation Loc, StringRef) {
+void ExpandModularHeadersPPCallbacks::Ident(SourceLocation Loc,
+ StringRef /*str*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::PragmaDirective(SourceLocation Loc,
- PragmaIntroducerKind) {
+void ExpandModularHeadersPPCallbacks::PragmaDirective(
+ SourceLocation Loc, PragmaIntroducerKind /*Introducer*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::PragmaComment(SourceLocation Loc,
- const IdentifierInfo *,
- StringRef) {
+void ExpandModularHeadersPPCallbacks::PragmaComment(
+ SourceLocation Loc, const IdentifierInfo * /*Kind*/, StringRef /*Str*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::PragmaDetectMismatch(SourceLocation Loc,
- StringRef,
- StringRef) {
+void ExpandModularHeadersPPCallbacks::PragmaDetectMismatch(
+ SourceLocation Loc, StringRef /*Name*/, StringRef /*Value*/) {
parseToLocation(Loc);
}
void ExpandModularHeadersPPCallbacks::PragmaDebug(SourceLocation Loc,
- StringRef) {
+ StringRef /*DebugType*/) {
parseToLocation(Loc);
}
void ExpandModularHeadersPPCallbacks::PragmaMessage(SourceLocation Loc,
- StringRef,
- PragmaMessageKind,
- StringRef) {
+ StringRef /*Namespace*/,
+ PragmaMessageKind /*Kind*/,
+ StringRef /*Str*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::PragmaDiagnosticPush(SourceLocation Loc,
- StringRef) {
+void ExpandModularHeadersPPCallbacks::PragmaDiagnosticPush(
+ SourceLocation Loc, StringRef /*Namespace*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::PragmaDiagnosticPop(SourceLocation Loc,
- StringRef) {
+void ExpandModularHeadersPPCallbacks::PragmaDiagnosticPop(
+ SourceLocation Loc, StringRef /*Namespace*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::PragmaDiagnostic(SourceLocation Loc,
- StringRef,
- diag::Severity,
- StringRef) {
+void ExpandModularHeadersPPCallbacks::PragmaDiagnostic(
+ SourceLocation Loc, StringRef /*Namespace*/, diag::Severity /*mapping*/,
+ StringRef /*Str*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::HasInclude(SourceLocation Loc, StringRef,
- bool, OptionalFileEntryRef,
- SrcMgr::CharacteristicKind) {
+void ExpandModularHeadersPPCallbacks::HasInclude(
+ SourceLocation Loc, StringRef /*FileName*/, bool /*IsAngled*/,
+ OptionalFileEntryRef /*File*/, SrcMgr::CharacteristicKind /*FileType*/) {
parseToLocation(Loc);
}
void ExpandModularHeadersPPCallbacks::PragmaOpenCLExtension(
- SourceLocation NameLoc, const IdentifierInfo *, SourceLocation StateLoc,
- unsigned) {
+ SourceLocation NameLoc, const IdentifierInfo * /*Name*/,
+ SourceLocation StateLoc, unsigned /*State*/) {
// FIXME: Figure out whether it's the right location to parse to.
parseToLocation(NameLoc);
}
-void ExpandModularHeadersPPCallbacks::PragmaWarning(SourceLocation Loc,
- PragmaWarningSpecifier,
- ArrayRef<int>) {
+void ExpandModularHeadersPPCallbacks::PragmaWarning(
+ SourceLocation Loc, PragmaWarningSpecifier /*WarningSpec*/,
+ ArrayRef<int> /*Ids*/) {
parseToLocation(Loc);
}
void ExpandModularHeadersPPCallbacks::PragmaWarningPush(SourceLocation Loc,
- int) {
+ int /*Level*/) {
parseToLocation(Loc);
}
void ExpandModularHeadersPPCallbacks::PragmaWarningPop(SourceLocation Loc) {
@@ -253,10 +251,9 @@ void ExpandModularHeadersPPCallbacks::PragmaAssumeNonNullEnd(
SourceLocation Loc) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::MacroExpands(const Token &MacroNameTok,
- const MacroDefinition &,
- SourceRange Range,
- const MacroArgs *) {
+void ExpandModularHeadersPPCallbacks::MacroExpands(
+ const Token &MacroNameTok, const MacroDefinition & /*MD*/,
+ SourceRange Range, const MacroArgs * /*Args*/) {
// FIXME: Figure out whether it's the right location to parse to.
parseToLocation(Range.getBegin());
}
@@ -265,12 +262,13 @@ void ExpandModularHeadersPPCallbacks::MacroDefined(const Token &MacroNameTok,
parseToLocation(MD->getLocation());
}
void ExpandModularHeadersPPCallbacks::MacroUndefined(
- const Token &, const MacroDefinition &, const MacroDirective *Undef) {
+ const Token & /*MacroNameTok*/, const MacroDefinition & /*MD*/,
+ const MacroDirective *Undef) {
if (Undef)
parseToLocation(Undef->getLocation());
}
void ExpandModularHeadersPPCallbacks::Defined(const Token &MacroNameTok,
- const MacroDefinition &,
+ const MacroDefinition & /*MD*/,
SourceRange Range) {
// FIXME: Figure out whether it's the right location to parse to.
parseToLocation(Range.getBegin());
@@ -280,27 +278,32 @@ void ExpandModularHeadersPPCallbacks::SourceRangeSkipped(
// FIXME: Figure out whether it's the right location to parse to.
parseToLocation(EndifLoc);
}
-void ExpandModularHeadersPPCallbacks::If(SourceLocation Loc, SourceRange,
- ConditionValueKind) {
+void ExpandModularHeadersPPCallbacks::If(
+ SourceLocation Loc, SourceRange /*ConditionRange*/,
+ ConditionValueKind /*ConditionValue*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::Elif(SourceLocation Loc, SourceRange,
- ConditionValueKind, SourceLocation) {
+void ExpandModularHeadersPPCallbacks::Elif(
+ SourceLocation Loc, SourceRange /*ConditionRange*/,
+ ConditionValueKind /*ConditionValue*/, SourceLocation /*IfLoc*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::Ifdef(SourceLocation Loc, const Token &,
- const MacroDefinition &) {
+void ExpandModularHeadersPPCallbacks::Ifdef(SourceLocation Loc,
+ const Token & /*MacroNameTok*/,
+ const MacroDefinition & /*MD*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::Ifndef(SourceLocation Loc, const Token &,
- const MacroDefinition &) {
+void ExpandModularHeadersPPCallbacks::Ifndef(SourceLocation Loc,
+ const Token & /*MacroNameTok*/,
+ const MacroDefinition & /*MD*/) {
parseToLocation(Loc);
}
-void ExpandModularHeadersPPCallbacks::Else(SourceLocation Loc, SourceLocation) {
+void ExpandModularHeadersPPCallbacks::Else(SourceLocation Loc,
+ SourceLocation /*IfLoc*/) {
parseToLocation(Loc);
}
void ExpandModularHeadersPPCallbacks::Endif(SourceLocation Loc,
- SourceLocation) {
+ SourceLocation /*IfLoc*/) {
parseToLocation(Loc);
}
diff --git a/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp b/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
index a94d6c8d7c4e6..4125b6d27d723 100644
--- a/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
@@ -51,17 +51,19 @@ class KernelNameRestrictionPPCallbacks : public PPCallbacks {
} // namespace
-void KernelNameRestrictionCheck::registerPPCallbacks(const SourceManager &SM,
- Preprocessor *PP,
- Preprocessor *) {
+void KernelNameRestrictionCheck::registerPPCallbacks(
+ const SourceManager &SM, Preprocessor *PP,
+ Preprocessor * /*ModuleExpanderPP*/) {
PP->addPPCallbacks(
std::make_unique<KernelNameRestrictionPPCallbacks>(*this, SM));
}
void KernelNameRestrictionPPCallbacks::InclusionDirective(
- SourceLocation HashLoc, const Token &, StringRef FileName, bool,
- CharSourceRange, OptionalFileEntryRef, StringRef, StringRef, const Module *,
- bool, SrcMgr::CharacteristicKind) {
+ SourceLocation HashLoc, const Token & /*IncludeTok*/, StringRef FileName,
+ bool /*IsAngled*/, CharSourceRange /*FilenameRange*/,
+ OptionalFileEntryRef /*File*/, StringRef /*SearchPath*/,
+ StringRef /*RelativePath*/, const Module * /*SuggestedModule*/,
+ bool /*ModuleImported*/, SrcMgr::CharacteristicKind /*FileType*/) {
IncludeDirective ID = {HashLoc, FileName};
IncludeDirectives.push_back(std::move(ID));
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
index e03cac6c5fd83..c02eb1bed7cc2 100644
--- a/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
@@ -35,13 +35,14 @@ void AssignmentInIfConditionCheck::check(
: Check(Check) {}
// Dont traverse into any lambda expressions.
- bool TraverseLambdaExpr(LambdaExpr *, DataRecursionQueue * = nullptr) {
+ bool TraverseLambdaExpr(LambdaExpr * /*unused*/,
+ DataRecursionQueue * /*unused*/ = nullptr) {
return true;
}
// Dont traverse into any requires expressions.
- bool TraverseRequiresExpr(RequiresExpr *,
- DataRecursionQueue * = nullptr) {
+ bool TraverseRequiresExpr(RequiresExpr * /*unused*/,
+ DataRecursionQueue * /*unused*/ = nullptr) {
return true;
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
index a6cd68edda55e..54d3bf612b3f8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -50,24 +50,28 @@ static bool isFallthroughSwitchBranch(const SwitchBranch &Branch) {
struct SwitchCaseVisitor : RecursiveASTVisitor<SwitchCaseVisitor> {
using RecursiveASTVisitor<SwitchCaseVisitor>::DataRecursionQueue;
- bool TraverseLambdaExpr(LambdaExpr *, DataRecursionQueue * = nullptr) {
+ bool TraverseLambdaExpr(LambdaExpr * /*unused*/,
+ DataRecursionQueue * /*unused*/ = nullptr) {
return true; // Ignore lambdas
}
- bool TraverseDecl(Decl *) {
+ bool TraverseDecl(Decl * /*unused*/) {
return true; // No need to check declarations
}
- bool TraverseSwitchStmt(SwitchStmt *, DataRecursionQueue * = nullptr) {
+ bool TraverseSwitchStmt(SwitchStmt * /*unused*/,
+ DataRecursionQueue * /*unused*/ = nullptr) {
return true; // Ignore sub-switches
}
// NOLINTNEXTLINE(readability-identifier-naming) - FIXME
- bool TraverseSwitchCase(SwitchCase *, DataRecursionQueue * = nullptr) {
+ bool TraverseSwitchCase(SwitchCase * /*unused*/,
+ DataRecursionQueue * /*unused*/ = nullptr) {
return true; // Ignore cases
}
- bool TraverseDefaultStmt(DefaultStmt *, DataRecursionQueue * = nullptr) {
+ bool TraverseDefaultStmt(DefaultStmt * /*unused*/,
+ DataRecursionQueue * /*unused*/ = nullptr) {
return true; // Ignore defaults
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
index 5812c18a2ccca..88530f86fb54e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -173,8 +173,8 @@ getFailureInfoImpl(StringRef Name, bool IsInGlobalNamespace, bool IsMacro,
}
std::optional<RenamerClangTidyCheck::FailureInfo>
-ReservedIdentifierCheck::getDeclFailureInfo(const NamedDecl *Decl,
- const SourceManager &) const {
+ReservedIdentifierCheck::getDeclFailureInfo(
+ const NamedDecl *Decl, const SourceManager & /*SM*/) const {
assert(Decl && Decl->getIdentifier() && !Decl->getName().empty() &&
"Decl must be an explicit identifier with a name.");
// Implicit identifiers cannot fail.
@@ -187,8 +187,8 @@ ReservedIdentifierCheck::getDeclFailureInfo(const NamedDecl *Decl,
}
std::optional<RenamerClangTidyCheck::FailureInfo>
-ReservedIdentifierCheck::getMacroFailureInfo(const Token &MacroNameTok,
- const SourceManager &) const {
+ReservedIdentifierCheck::getMacroFailureInfo(
+ const Token &MacroNameTok, const SourceManager & /*SM*/) const {
return getFailureInfoImpl(MacroNameTok.getIdentifierInfo()->getName(), true,
/*IsMacro = */ true, getLangOpts(), Invert,
AllowedIdentifiers);
diff --git a/clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp b/clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
index c9b48e922ea57..34b98db3a14a3 100644
--- a/clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
@@ -48,7 +48,7 @@ class UpgradeGoogletestCasePPCallback : public PPCallbacks {
: Check(Check), PP(PP) {}
void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
- SourceRange Range, const MacroArgs *) override {
+ SourceRange Range, const MacroArgs * /*Args*/) override {
macroUsed(MacroNameTok, MD, Range.getBegin(), CheckAction::Rename);
}
@@ -119,9 +119,9 @@ class UpgradeGoogletestCasePPCallback : public PPCallbacks {
} // namespace
-void UpgradeGoogletestCaseCheck::registerPPCallbacks(const SourceManager &,
- Preprocessor *PP,
- Preprocessor *) {
+void UpgradeGoogletestCaseCheck::registerPPCallbacks(
+ const SourceManager & /*SM*/, Preprocessor *PP,
+ Preprocessor * /*ModuleExpanderPP*/) {
PP->addPPCallbacks(
std::make_unique<UpgradeGoogletestCasePPCallback>(this, PP));
}
diff --git a/clang-tools-extra/clang-tidy/llvm/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseRangesCheck.cpp
index 4afab488b7dcc..70a082af526a1 100644
--- a/clang-tools-extra/clang-tidy/llvm/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/UseRangesCheck.cpp
@@ -29,7 +29,7 @@ class StdToLLVMReplacer : public utils::UseRangesCheck::Replacer {
}
std::optional<std::string>
- getHeaderInclusion(const NamedDecl &) const override {
+ getHeaderInclusion(const NamedDecl & /*OriginalName*/) const override {
return "llvm/ADT/STLExtras.h";
}
diff --git a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
index 1f6ceda9f5b9e..ca2dd8cd314d4 100644
--- a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
@@ -73,10 +73,13 @@ class CyclicDependencyCallbacks : public PPCallbacks {
Files.push_back({NewFile, FileName, std::exchange(NextToEnter, {})});
}
- void InclusionDirective(SourceLocation, const Token &, StringRef FilePath,
- bool, CharSourceRange Range,
- OptionalFileEntryRef File, StringRef, StringRef,
- const Module *, bool,
+ void InclusionDirective(SourceLocation /*HashLoc*/,
+ const Token & /*IncludeTok*/, StringRef FilePath,
+ bool /*IsAngled*/, CharSourceRange Range,
+ OptionalFileEntryRef File, StringRef /*SearchPath*/,
+ StringRef /*RelativePath*/,
+ const Module * /*SuggestedModule*/,
+ bool /*ModuleImported*/,
SrcMgr::CharacteristicKind FileType) override {
if (FileType != clang::SrcMgr::C_User)
return;
diff --git...
[truncated]
|
struct NOptionMap { | ||
NOptionMap(IO &) {} | ||
NOptionMap(IO &, const ClangTidyOptions::OptionMap &OptionMap) { | ||
NOptionMap(IO & /*unused*/) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these truly named unused
in the declaration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case in particular, that parameter is never named; unused
is what the checker falls back to when it cannot find a declaration with a name. In general, we can change the default unused
to something more meaningful (manually)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need it? IMO it is fine to keep parameter name empty when we don't need it.
@carlosgalvezp @5chmidti @vbvictor
I personally remove names when they are not needed, but we can go either way. |
Closes #156159