[clang-tools-extra] [clang-tidy][readability-identifier-length] refactoring and cleanup (PR #194610)
Alex Dutka via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 28 06:04:57 PDT 2026
https://github.com/dutkalex updated https://github.com/llvm/llvm-project/pull/194610
>From c34315a5d5595dbc758122f4bd407848852f7b4e Mon Sep 17 00:00:00 2001
From: Alex Dutka <97711898+dutkalex at users.noreply.github.com>
Date: Tue, 28 Apr 2026 14:05:52 +0200
Subject: [PATCH 1/6] extract helper function
---
.../readability/IdentifierLengthCheck.cpp | 122 +++++++-----------
1 file changed, 46 insertions(+), 76 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
index 8dd238ac8e595..0c1379f49098a 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
@@ -137,100 +137,70 @@ static bool isShortLived(const ValueDecl *Var, const SourceManager *SrcMgr,
return false;
}
-void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *StandaloneVar =
- Result.Nodes.getNodeAs<ValueDecl>("standaloneVar");
- if (StandaloneVar) {
- if (!StandaloneVar->getIdentifier())
- return;
+static bool shouldWarn(const ValueDecl *Var, unsigned MinNameLength,
+ llvm::Regex const &IgnoredNames,
+ const MatchFinder::MatchResult &Result,
+ unsigned LineCountThreshold) {
+ if (!Var->getIdentifier())
+ return false;
+
+ const StringRef VarName = Var->getName();
- const StringRef VarName = StandaloneVar->getName();
+ if (VarName.size() >= MinNameLength || IgnoredNames.match(VarName))
+ return false;
- if (VarName.size() >= MinimumVariableNameLength ||
- IgnoredVariableNames.match(VarName))
- return;
+ if (isShortLived(Var, Result.SourceManager, Result.Context,
+ LineCountThreshold))
+ return false;
- if (isShortLived(StandaloneVar, Result.SourceManager, Result.Context,
- LineCountThreshold))
- return;
+ return true;
+}
- diag(StandaloneVar->getLocation(), ErrorMessage)
- << 0 << StandaloneVar << MinimumVariableNameLength;
+void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *StandaloneVar =
+ Result.Nodes.getNodeAs<ValueDecl>("standaloneVar");
+ if (StandaloneVar) {
+ if (shouldWarn(StandaloneVar, MinimumVariableNameLength,
+ IgnoredVariableNames, Result, LineCountThreshold))
+ diag(StandaloneVar->getLocation(), ErrorMessage)
+ << 0 << StandaloneVar << MinimumVariableNameLength;
+ return;
}
const auto *BindingVar = Result.Nodes.getNodeAs<ValueDecl>("bindingVar");
if (BindingVar) {
- if (!BindingVar->getIdentifier())
- return;
-
- const StringRef VarName = BindingVar->getName();
-
- if (VarName.size() >= MinimumBindingNameLength ||
- IgnoredBindingNames.match(VarName))
- return;
-
- if (isShortLived(BindingVar, Result.SourceManager, Result.Context,
- LineCountThreshold))
- return;
-
- diag(BindingVar->getLocation(), ErrorMessage)
- << 1 << BindingVar << MinimumBindingNameLength;
+ if (shouldWarn(BindingVar, MinimumBindingNameLength, IgnoredBindingNames,
+ Result, LineCountThreshold))
+ diag(BindingVar->getLocation(), ErrorMessage)
+ << 1 << BindingVar << MinimumBindingNameLength;
+ return;
}
- auto *ExceptionVarName = Result.Nodes.getNodeAs<ValueDecl>("exceptionVar");
- if (ExceptionVarName) {
- if (!ExceptionVarName->getIdentifier())
- return;
-
- const StringRef VarName = ExceptionVarName->getName();
- if (VarName.size() >= MinimumExceptionNameLength ||
- IgnoredExceptionVariableNames.match(VarName))
- return;
-
- if (isShortLived(ExceptionVarName, Result.SourceManager, Result.Context,
- LineCountThreshold))
- return;
-
- diag(ExceptionVarName->getLocation(), ErrorMessage)
- << 2 << ExceptionVarName << MinimumExceptionNameLength;
+ auto *ExceptionVar = Result.Nodes.getNodeAs<ValueDecl>("exceptionVar");
+ if (ExceptionVar) {
+ if (shouldWarn(ExceptionVar, MinimumExceptionNameLength,
+ IgnoredExceptionVariableNames, Result, LineCountThreshold))
+ diag(ExceptionVar->getLocation(), ErrorMessage)
+ << 2 << ExceptionVar << MinimumExceptionNameLength;
+ return;
}
const auto *LoopVar = Result.Nodes.getNodeAs<ValueDecl>("loopVar");
if (LoopVar) {
- if (!LoopVar->getIdentifier())
- return;
-
- const StringRef VarName = LoopVar->getName();
-
- if (VarName.size() >= MinimumLoopCounterNameLength ||
- IgnoredLoopCounterNames.match(VarName))
- return;
-
- if (isShortLived(LoopVar, Result.SourceManager, Result.Context,
- LineCountThreshold))
- return;
-
- diag(LoopVar->getLocation(), ErrorMessage)
- << 3 << LoopVar << MinimumLoopCounterNameLength;
+ if (shouldWarn(LoopVar, MinimumLoopCounterNameLength,
+ IgnoredLoopCounterNames, Result, LineCountThreshold))
+ diag(LoopVar->getLocation(), ErrorMessage)
+ << 3 << LoopVar << MinimumLoopCounterNameLength;
+ return;
}
const auto *ParamVar = Result.Nodes.getNodeAs<ValueDecl>("paramVar");
if (ParamVar) {
- if (!ParamVar->getIdentifier())
- return;
-
- const StringRef VarName = ParamVar->getName();
-
- if (VarName.size() >= MinimumParameterNameLength ||
- IgnoredParameterNames.match(VarName))
- return;
-
- if (isShortLived(ParamVar, Result.SourceManager, Result.Context,
- LineCountThreshold))
- return;
-
- diag(ParamVar->getLocation(), ErrorMessage)
- << 4 << ParamVar << MinimumParameterNameLength;
+ if (shouldWarn(ParamVar, MinimumParameterNameLength, IgnoredParameterNames,
+ Result, LineCountThreshold))
+ diag(ParamVar->getLocation(), ErrorMessage)
+ << 4 << ParamVar << MinimumParameterNameLength;
+ return;
}
}
>From 15503c57e4d865d702fa349dffc17da67c0f423d Mon Sep 17 00:00:00 2001
From: Alex Dutka <97711898+dutkalex at users.noreply.github.com>
Date: Tue, 28 Apr 2026 14:20:16 +0200
Subject: [PATCH 2/6] rewrite helper function as a lambda
---
.../readability/IdentifierLengthCheck.cpp | 47 +++++++++----------
1 file changed, 21 insertions(+), 26 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
index 0c1379f49098a..28fbb24d8f29c 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
@@ -137,31 +137,28 @@ static bool isShortLived(const ValueDecl *Var, const SourceManager *SrcMgr,
return false;
}
-static bool shouldWarn(const ValueDecl *Var, unsigned MinNameLength,
- llvm::Regex const &IgnoredNames,
- const MatchFinder::MatchResult &Result,
- unsigned LineCountThreshold) {
- if (!Var->getIdentifier())
- return false;
-
- const StringRef VarName = Var->getName();
+void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
+ auto ShouldWarn = [&](const ValueDecl *Var, unsigned MinNameLength,
+ llvm::Regex const &IgnoredNames) -> bool {
+ if (!Var->getIdentifier())
+ return false;
- if (VarName.size() >= MinNameLength || IgnoredNames.match(VarName))
- return false;
+ const StringRef VarName = Var->getName();
+ if (VarName.size() >= MinNameLength || IgnoredNames.match(VarName))
+ return false;
- if (isShortLived(Var, Result.SourceManager, Result.Context,
- LineCountThreshold))
- return false;
+ if (isShortLived(Var, Result.SourceManager, Result.Context,
+ LineCountThreshold))
+ return false;
- return true;
-}
+ return true;
+ };
-void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
const auto *StandaloneVar =
Result.Nodes.getNodeAs<ValueDecl>("standaloneVar");
if (StandaloneVar) {
- if (shouldWarn(StandaloneVar, MinimumVariableNameLength,
- IgnoredVariableNames, Result, LineCountThreshold))
+ if (ShouldWarn(StandaloneVar, MinimumVariableNameLength,
+ IgnoredVariableNames))
diag(StandaloneVar->getLocation(), ErrorMessage)
<< 0 << StandaloneVar << MinimumVariableNameLength;
return;
@@ -169,8 +166,7 @@ void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
const auto *BindingVar = Result.Nodes.getNodeAs<ValueDecl>("bindingVar");
if (BindingVar) {
- if (shouldWarn(BindingVar, MinimumBindingNameLength, IgnoredBindingNames,
- Result, LineCountThreshold))
+ if (ShouldWarn(BindingVar, MinimumBindingNameLength, IgnoredBindingNames))
diag(BindingVar->getLocation(), ErrorMessage)
<< 1 << BindingVar << MinimumBindingNameLength;
return;
@@ -178,8 +174,8 @@ void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
auto *ExceptionVar = Result.Nodes.getNodeAs<ValueDecl>("exceptionVar");
if (ExceptionVar) {
- if (shouldWarn(ExceptionVar, MinimumExceptionNameLength,
- IgnoredExceptionVariableNames, Result, LineCountThreshold))
+ if (ShouldWarn(ExceptionVar, MinimumExceptionNameLength,
+ IgnoredExceptionVariableNames))
diag(ExceptionVar->getLocation(), ErrorMessage)
<< 2 << ExceptionVar << MinimumExceptionNameLength;
return;
@@ -187,8 +183,8 @@ void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
const auto *LoopVar = Result.Nodes.getNodeAs<ValueDecl>("loopVar");
if (LoopVar) {
- if (shouldWarn(LoopVar, MinimumLoopCounterNameLength,
- IgnoredLoopCounterNames, Result, LineCountThreshold))
+ if (ShouldWarn(LoopVar, MinimumLoopCounterNameLength,
+ IgnoredLoopCounterNames))
diag(LoopVar->getLocation(), ErrorMessage)
<< 3 << LoopVar << MinimumLoopCounterNameLength;
return;
@@ -196,8 +192,7 @@ void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
const auto *ParamVar = Result.Nodes.getNodeAs<ValueDecl>("paramVar");
if (ParamVar) {
- if (shouldWarn(ParamVar, MinimumParameterNameLength, IgnoredParameterNames,
- Result, LineCountThreshold))
+ if (ShouldWarn(ParamVar, MinimumParameterNameLength, IgnoredParameterNames))
diag(ParamVar->getLocation(), ErrorMessage)
<< 4 << ParamVar << MinimumParameterNameLength;
return;
>From 9cffec39c675db49ef9b55b99d4666bece8deb16 Mon Sep 17 00:00:00 2001
From: Alex Dutka <97711898+dutkalex at users.noreply.github.com>
Date: Tue, 28 Apr 2026 14:32:00 +0200
Subject: [PATCH 3/6] reduce var scopes
---
.../readability/IdentifierLengthCheck.cpp | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
index 28fbb24d8f29c..8031cbeb00a51 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
@@ -154,9 +154,8 @@ void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
return true;
};
- const auto *StandaloneVar =
- Result.Nodes.getNodeAs<ValueDecl>("standaloneVar");
- if (StandaloneVar) {
+ if (const auto *StandaloneVar =
+ Result.Nodes.getNodeAs<ValueDecl>("standaloneVar")) {
if (ShouldWarn(StandaloneVar, MinimumVariableNameLength,
IgnoredVariableNames))
diag(StandaloneVar->getLocation(), ErrorMessage)
@@ -164,16 +163,16 @@ void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
return;
}
- const auto *BindingVar = Result.Nodes.getNodeAs<ValueDecl>("bindingVar");
- if (BindingVar) {
+ if (const auto *BindingVar =
+ Result.Nodes.getNodeAs<ValueDecl>("bindingVar")) {
if (ShouldWarn(BindingVar, MinimumBindingNameLength, IgnoredBindingNames))
diag(BindingVar->getLocation(), ErrorMessage)
<< 1 << BindingVar << MinimumBindingNameLength;
return;
}
- auto *ExceptionVar = Result.Nodes.getNodeAs<ValueDecl>("exceptionVar");
- if (ExceptionVar) {
+ if (const auto *ExceptionVar =
+ Result.Nodes.getNodeAs<ValueDecl>("exceptionVar")) {
if (ShouldWarn(ExceptionVar, MinimumExceptionNameLength,
IgnoredExceptionVariableNames))
diag(ExceptionVar->getLocation(), ErrorMessage)
@@ -181,8 +180,7 @@ void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
return;
}
- const auto *LoopVar = Result.Nodes.getNodeAs<ValueDecl>("loopVar");
- if (LoopVar) {
+ if (const auto *LoopVar = Result.Nodes.getNodeAs<ValueDecl>("loopVar")) {
if (ShouldWarn(LoopVar, MinimumLoopCounterNameLength,
IgnoredLoopCounterNames))
diag(LoopVar->getLocation(), ErrorMessage)
@@ -190,8 +188,7 @@ void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
return;
}
- const auto *ParamVar = Result.Nodes.getNodeAs<ValueDecl>("paramVar");
- if (ParamVar) {
+ if (const auto *ParamVar = Result.Nodes.getNodeAs<ValueDecl>("paramVar")) {
if (ShouldWarn(ParamVar, MinimumParameterNameLength, IgnoredParameterNames))
diag(ParamVar->getLocation(), ErrorMessage)
<< 4 << ParamVar << MinimumParameterNameLength;
>From 608b9d0a23ff4cc9c72085c37183c65ec0e0caaf Mon Sep 17 00:00:00 2001
From: Alex Dutka <97711898+dutkalex at users.noreply.github.com>
Date: Tue, 28 Apr 2026 14:45:29 +0200
Subject: [PATCH 4/6] replace std::string -> llvm::StringRef
---
.../clang-tidy/readability/IdentifierLengthCheck.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
index 8fd762971b01b..4c9aa1e355493 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
@@ -32,19 +32,19 @@ class IdentifierLengthCheck : public ClangTidyCheck {
const unsigned MinimumExceptionNameLength;
const unsigned MinimumParameterNameLength;
- std::string IgnoredVariableNamesInput;
+ llvm::StringRef IgnoredVariableNamesInput;
llvm::Regex IgnoredVariableNames;
- std::string IgnoredBindingNamesInput;
+ llvm::StringRef IgnoredBindingNamesInput;
llvm::Regex IgnoredBindingNames;
- std::string IgnoredLoopCounterNamesInput;
+ llvm::StringRef IgnoredLoopCounterNamesInput;
llvm::Regex IgnoredLoopCounterNames;
- std::string IgnoredExceptionVariableNamesInput;
+ llvm::StringRef IgnoredExceptionVariableNamesInput;
llvm::Regex IgnoredExceptionVariableNames;
- std::string IgnoredParameterNamesInput;
+ llvm::StringRef IgnoredParameterNamesInput;
llvm::Regex IgnoredParameterNames;
const unsigned LineCountThreshold;
>From 01fbacacc3bdb5a8a6e6ad36d618a6ed6b3258c3 Mon Sep 17 00:00:00 2001
From: Alex Dutka <97711898+dutkalex at users.noreply.github.com>
Date: Tue, 28 Apr 2026 14:57:22 +0200
Subject: [PATCH 5/6] doc cleanup
---
.../checks/readability/identifier-length.rst | 20 +++----------------
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-length.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-length.rst
index 2e19897bcab66..f08dbb89404c6 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-length.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/identifier-length.rst
@@ -4,9 +4,9 @@ readability-identifier-length
=============================
This check finds variables and function parameters whose length are too short.
-The desired name length is configurable.
-
-Special cases are supported for loop counters and for exception variable names.
+The desired name length is configurable. Special short names which should be
+ignored can be specified. Local variables with short names can also be ignored
+if they are short-lived. This check offers no automated fix.
Options
-------
@@ -28,14 +28,10 @@ The following options are described below:
`MinimumVariableNameLength` (default is `3`). Setting it to `0` or `1`
disables the check entirely.
-
.. code-block:: c++
int i = 42; // warns that 'i' is too short
- This check does not have any fix suggestions in the general case since
- variable names have semantic value.
-
.. option:: IgnoredVariableNames
Specifies a regular expression for variable names that are
@@ -51,9 +47,6 @@ The following options are described below:
auto [a] = get_result(); // warns that 'a' is too short
- This check does not have any fix suggestions in the general case since
- variable names have semantic value.
-
.. option:: IgnoredBindingNames
Specifies a regular expression for variable names introduced by structured
@@ -66,7 +59,6 @@ The following options are described below:
`MinimumParameterNameLength` (default is `3`). Setting it to `0` or `1`
disables the check entirely.
-
.. code-block:: c++
int doubler(int x) // warns that x is too short
@@ -74,9 +66,6 @@ The following options are described below:
return 2 * x;
}
- This check does not have any fix suggestions in the general case since
- variable names have semantic value.
-
.. option:: IgnoredParameterNames
Specifies a regular expression for parameters that are to be ignored.
@@ -88,7 +77,6 @@ The following options are described below:
`MinimumLoopCounterNameLength` characters (default is `2`). Setting it to
`0` or `1` disables the check entirely.
-
.. code-block:: c++
// This warns that 'q' is too short.
@@ -103,7 +91,6 @@ The following options are described below:
reasons and the last one since it is frequently used as a "don't care"
value, specifically in tools such as Google Benchmark.
-
.. code-block:: c++
// This does not warn by default, for historical reasons.
@@ -117,7 +104,6 @@ The following options are described below:
`MinimumExceptionNameLength` (default is `2`). Setting it to `0` or `1`
disables the check entirely.
-
.. code-block:: c++
try {
>From 78ef60e974d21d5fa6514d27f19074fb364485cc Mon Sep 17 00:00:00 2001
From: Alex Dutka <97711898+dutkalex at users.noreply.github.com>
Date: Tue, 28 Apr 2026 15:04:35 +0200
Subject: [PATCH 6/6] format
---
.../clang-tidy/readability/IdentifierLengthCheck.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
index 8031cbeb00a51..833f40eba63c1 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
@@ -139,7 +139,7 @@ static bool isShortLived(const ValueDecl *Var, const SourceManager *SrcMgr,
void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
auto ShouldWarn = [&](const ValueDecl *Var, unsigned MinNameLength,
- llvm::Regex const &IgnoredNames) -> bool {
+ const llvm::Regex &IgnoredNames) -> bool {
if (!Var->getIdentifier())
return false;
More information about the cfe-commits
mailing list