https://github.com/dmpolukhin updated
https://github.com/llvm/llvm-project/pull/147953
>From 0f6d49539df0269daed67af2c7c054f3501fcc0b Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.poluk...@gmail.com>
Date: Thu, 10 Jul 2025 05:26:11 -0700
Subject: [PATCH 1/3] [clang-tidy][readability-named-parameter] Add an option
to print names without comment
Add InsertPlainNamesInForwardDecls option to readability-named-parameter check
to insert parameter names without comments for forward declarations only.
When enabled, forward declarations get plain parameter names (e.g., `int param`)
while function definitions continue to use commented names (e.g., `int
/*param*/`).
Named parameters in forward decls don't cause compiler warnings and some
developers
prefer to have names without comments but in sync between declarations
and the definition.
Default behavior remains unchanged (InsertPlainNamesInForwardDecls=false).
Example with InsertPlainNamesInForwardDecls=true:
```cpp
// Forward declaration - gets plain name
void func(int param);
void func(int param) { ... = param; }
```
---
.../readability/NamedParameterCheck.cpp | 34 +++++++++++++++---
.../readability/NamedParameterCheck.h | 7 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 7 +++-
.../checks/readability/named-parameter.rst | 9 +++++
.../checkers/readability/named-parameter.cpp | 35 +++++++++++++++++++
5 files changed, 85 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
b/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
index ea6597dbdd617..b2bc3d0a4d1ec 100644
--- a/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
@@ -15,6 +15,17 @@ using namespace clang::ast_matchers;
namespace clang::tidy::readability {
+NamedParameterCheck::NamedParameterCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ InsertPlainNamesInForwardDecls(
+ Options.get("InsertPlainNamesInForwardDecls", false)) {}
+
+void NamedParameterCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "InsertPlainNamesInForwardDecls",
+ InsertPlainNamesInForwardDecls);
+}
+
void NamedParameterCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
Finder->addMatcher(functionDecl().bind("decl"), this);
}
@@ -84,7 +95,8 @@ void NamedParameterCheck::check(const
MatchFinder::MatchResult &Result) {
for (auto P : UnnamedParams) {
// Fallback to an unused marker.
- StringRef NewName = "unused";
+ constexpr StringRef FallbackName = "unused";
+ StringRef NewName = FallbackName;
// If the method is overridden, try to copy the name from the base method
// into the overrider.
@@ -105,12 +117,26 @@ void NamedParameterCheck::check(const
MatchFinder::MatchResult &Result) {
NewName = Name;
}
- // Now insert the comment. Note that getLocation() points to the place
+ // Now insert the fix. Note that getLocation() points to the place
// where the name would be, this allows us to also get complex cases like
// function pointers right.
const ParmVarDecl *Parm = P.first->getParamDecl(P.second);
- D << FixItHint::CreateInsertion(Parm->getLocation(),
- " /*" + NewName.str() + "*/");
+
+ // The fix depends on the InsertPlainNamesInForwardDecls option,
+ // whether this is a forward declaration and whether the parameter has
+ // a real name.
+ bool IsForwardDeclaration = (!Definition || Function != Definition);
+ if (InsertPlainNamesInForwardDecls && IsForwardDeclaration &&
+ NewName != FallbackName) {
+ // For forward declarations with InsertPlainNamesInForwardDecls
enabled,
+ // insert the parameter name without comments
+ D << FixItHint::CreateInsertion(Parm->getLocation(),
+ " " + NewName.str());
+ } else {
+ // Default behavior: insert the parameter name as a comment
+ D << FixItHint::CreateInsertion(Parm->getLocation(),
+ " /*" + NewName.str() + "*/");
+ }
}
}
}
diff --git a/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.h
b/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.h
index 812d90ef7319c..f14a74d75eb49 100644
--- a/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.h
@@ -26,13 +26,16 @@ namespace clang::tidy::readability {
/// Corresponding cpplint.py check name: 'readability/function'.
class NamedParameterCheck : public ClangTidyCheck {
public:
- NamedParameterCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ NamedParameterCheck(StringRef Name, ClangTidyContext *Context);
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
std::optional<TraversalKind> getCheckTraversalKind() const override {
return TK_IgnoreUnlessSpelledInSource;
}
+
+private:
+ const bool InsertPlainNamesInForwardDecls;
};
} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index ad869265a2db5..95e6ee1b51334 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -215,7 +215,7 @@ Changes in existing checks
- Improved :doc:`cppcoreguidelines-missing-std-forward
<clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by adding a
flag to specify the function used for forwarding instead of ``std::forward``.
-
+
- Improved :doc:`cppcoreguidelines-pro-bounds-pointer-arithmetic
<clang-tidy/checks/cppcoreguidelines/pro-bounds-pointer-arithmetic>` check by
fixing false positives when calling indexing operators that do not perform
@@ -342,6 +342,11 @@ Changes in existing checks
false negatives where math expressions are the operand of assignment
operators
or comparison operators.
+- Improved :doc:`readability-named-parameter
+ <clang-tidy/checks/readability/named-parameter>` check by adding the option
+ `InsertPlainNamesInForwardDecls` to insert parameter names without comments
+ for forward declarations only.
+
- Improved :doc:`readability-qualified-auto
<clang-tidy/checks/readability/qualified-auto>` check by adding the option
`AllowedTypes`, that excludes specified types from adding qualifiers.
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/readability/named-parameter.rst
b/clang-tools-extra/docs/clang-tidy/checks/readability/named-parameter.rst
index 73677a48605f4..283e96173addb 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/named-parameter.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/named-parameter.rst
@@ -23,3 +23,12 @@ If a parameter is not utilized, its name can be commented
out in a function defi
}
Corresponding cpplint.py check name: `readability/function`.
+
+Options
+-------
+
+.. option:: InsertPlainNamesInForwardDecls
+
+ When `true`, the check will insert parameter names without comments for
+ forward declarations only. When `false` (default), parameter names are
+ always inserted as comments (e.g., ``/*param*/``).
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/named-parameter.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/named-parameter.cpp
index 50433d5d12ea9..461ad99b57fbe 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/named-parameter.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/named-parameter.cpp
@@ -1,29 +1,46 @@
// RUN: %check_clang_tidy %s readability-named-parameter %t
+// RUN: %check_clang_tidy -check-suffix=PLAIN-NAMES %s
readability-named-parameter %t -- -config="{CheckOptions: [{key:
readability-named-parameter.InsertPlainNamesInForwardDecls, value: true}]}"
void Method(char *) { /* */ }
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in
a function
// CHECK-FIXES: void Method(char * /*unused*/) { /* */ }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:19: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void Method(char * /*unused*/) { /* */ }
void Method2(char *) {}
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in
a function
// CHECK-FIXES: void Method2(char * /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:20: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void Method2(char * /*unused*/) {}
void Method3(char *, void *) {}
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in
a function
// CHECK-FIXES: void Method3(char * /*unused*/, void * /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:20: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void Method3(char * /*unused*/, void * /*unused*/)
{}
void Method4(char *, int /*unused*/) {}
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in
a function
// CHECK-FIXES: void Method4(char * /*unused*/, int /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:20: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void Method4(char * /*unused*/, int /*unused*/) {}
void operator delete[](void *) throw() {}
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: all parameters should be named in
a function
// CHECK-FIXES: void operator delete[](void * /*unused*/) throw() {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:30: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void operator delete[](void * /*unused*/) throw()
{}
int Method5(int) { return 0; }
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: all parameters should be named in
a function
// CHECK-FIXES: int Method5(int /*unused*/) { return 0; }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:16: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: int Method5(int /*unused*/) { return 0; }
void Method6(void (*)(void *)) {}
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: all parameters should be named in
a function
// CHECK-FIXES: void Method6(void (* /*unused*/)(void *)) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:21: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void Method6(void (* /*unused*/)(void *)) {}
template <typename T> void Method7(T) {}
// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: all parameters should be named in
a function
// CHECK-FIXES: template <typename T> void Method7(T /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:37: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: template <typename T> void Method7(T /*unused*/) {}
// Don't warn in macros.
#define M void MethodM(int) {}
@@ -55,6 +72,8 @@ struct Y {
void foo(T) {}
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: all parameters should be named in
a function
// CHECK-FIXES: void foo(T /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:13: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void foo(T /*unused*/) {}
};
Y<int> y;
@@ -69,19 +88,27 @@ struct Derived : public Base {
void foo(int);
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: all parameters should be named in
a function
// CHECK-FIXES: void foo(int /*argname*/);
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:15: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void foo(int argname);
};
void FDef(int);
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: all parameters should be named in
a function
// CHECK-FIXES: void FDef(int /*n*/);
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:14: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void FDef(int n);
void FDef(int n) {}
void FDef2(int, int);
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: all parameters should be named in
a function
// CHECK-FIXES: void FDef2(int /*n*/, int /*unused*/);
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:15: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void FDef2(int n, int /*unused*/);
void FDef2(int n, int) {}
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: all parameters should be named in
a function
// CHECK-FIXES: void FDef2(int n, int /*unused*/) {}
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:22: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: void FDef2(int n, int /*unused*/) {}
void FNoDef(int);
@@ -91,18 +118,26 @@ Z the_z;
Z &operator++(Z&) { return the_z; }
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in
a function
// CHECK-FIXES: Z &operator++(Z& /*unused*/) { return the_z; }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: Z &operator++(Z& /*unused*/) { return the_z; }
Z &operator++(Z&, int) { return the_z; }
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in
a function
// CHECK-FIXES: Z &operator++(Z& /*unused*/, int) { return the_z; }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: Z &operator++(Z& /*unused*/, int) { return the_z; }
Z &operator--(Z&) { return the_z; }
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in
a function
// CHECK-FIXES: Z &operator--(Z& /*unused*/) { return the_z; }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: Z &operator--(Z& /*unused*/) { return the_z; }
Z &operator--(Z&, int) { return the_z; }
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: all parameters should be named in
a function
// CHECK-FIXES: Z &operator--(Z& /*unused*/, int) { return the_z; }
+// CHECK-MESSAGES-PLAIN-NAMES: :[[@LINE-3]]:17: warning: all parameters should
be named in a function
+// CHECK-FIXES-PLAIN-NAMES: Z &operator--(Z& /*unused*/, int) { return the_z; }
namespace testing {
namespace internal {
>From 4518c1e3fe768c84911527fb1e019e7547e2c6ed Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.poluk...@gmail.com>
Date: Thu, 10 Jul 2025 08:37:38 -0700
Subject: [PATCH 2/3] Comments resolved.
---
.../clang-tidy/readability/NamedParameterCheck.cpp | 2 +-
.../docs/clang-tidy/checks/readability/named-parameter.rst | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
b/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
index b2bc3d0a4d1ec..80a09ddbb3a09 100644
--- a/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
@@ -125,7 +125,7 @@ void NamedParameterCheck::check(const
MatchFinder::MatchResult &Result) {
// The fix depends on the InsertPlainNamesInForwardDecls option,
// whether this is a forward declaration and whether the parameter has
// a real name.
- bool IsForwardDeclaration = (!Definition || Function != Definition);
+ const bool IsForwardDeclaration = (!Definition || Function !=
Definition);
if (InsertPlainNamesInForwardDecls && IsForwardDeclaration &&
NewName != FallbackName) {
// For forward declarations with InsertPlainNamesInForwardDecls
enabled,
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/readability/named-parameter.rst
b/clang-tools-extra/docs/clang-tidy/checks/readability/named-parameter.rst
index 283e96173addb..48b7e84d38ec8 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/named-parameter.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/named-parameter.rst
@@ -29,6 +29,6 @@ Options
.. option:: InsertPlainNamesInForwardDecls
- When `true`, the check will insert parameter names without comments for
- forward declarations only. When `false` (default), parameter names are
- always inserted as comments (e.g., ``/*param*/``).
+ If set to `true`, the check will insert parameter names without comments for
+ forward declarations only. Otherwise, the check will insert parameter names
+ as comments (e.g., ``/*param*/``). Default is `false`.
>From 98c4c9c18713d8cca9c2db72904fd10791f73467 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmitry.poluk...@gmail.com>
Date: Thu, 10 Jul 2025 09:26:02 -0700
Subject: [PATCH 3/3] More comments resolved.
---
.../clang-tidy/readability/NamedParameterCheck.cpp | 5 ++---
.../test/clang-tidy/checkers/readability/named-parameter.cpp | 3 ++-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
b/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
index 80a09ddbb3a09..6bb8c394f75cc 100644
--- a/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/NamedParameterCheck.cpp
@@ -95,7 +95,7 @@ void NamedParameterCheck::check(const
MatchFinder::MatchResult &Result) {
for (auto P : UnnamedParams) {
// Fallback to an unused marker.
- constexpr StringRef FallbackName = "unused";
+ static constexpr StringRef FallbackName = "unused";
StringRef NewName = FallbackName;
// If the method is overridden, try to copy the name from the base method
@@ -129,11 +129,10 @@ void NamedParameterCheck::check(const
MatchFinder::MatchResult &Result) {
if (InsertPlainNamesInForwardDecls && IsForwardDeclaration &&
NewName != FallbackName) {
// For forward declarations with InsertPlainNamesInForwardDecls
enabled,
- // insert the parameter name without comments
+ // insert the parameter name without comments.
D << FixItHint::CreateInsertion(Parm->getLocation(),
" " + NewName.str());
} else {
- // Default behavior: insert the parameter name as a comment
D << FixItHint::CreateInsertion(Parm->getLocation(),
" /*" + NewName.str() + "*/");
}
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/named-parameter.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/named-parameter.cpp
index 461ad99b57fbe..8ae0d7055867b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/named-parameter.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/named-parameter.cpp
@@ -1,5 +1,6 @@
// RUN: %check_clang_tidy %s readability-named-parameter %t
-// RUN: %check_clang_tidy -check-suffix=PLAIN-NAMES %s
readability-named-parameter %t -- -config="{CheckOptions: [{key:
readability-named-parameter.InsertPlainNamesInForwardDecls, value: true}]}"
+// RUN: %check_clang_tidy -check-suffix=PLAIN-NAMES %s
readability-named-parameter %t -- \
+// RUN: -config="{CheckOptions: [{key:
readability-named-parameter.InsertPlainNamesInForwardDecls, value: true}]}"
void Method(char *) { /* */ }
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in
a function
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
