[clang-tools-extra] [clang-tidy][readability-identifier-length] refactoring and cleanup (PR #194610)
Alex Dutka via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 29 00:56:52 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/7] 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/7] 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/7] 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/7] 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/7] 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/7] 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;
>From 1498524d93b250d0cbfe7a43ec29865d4067c26e Mon Sep 17 00:00:00 2001
From: Alex Dutka <adutka at cerfacs.fr>
Date: Wed, 29 Apr 2026 09:56:39 +0200
Subject: [PATCH 7/7] cleanup documentation
Co-authored-by: Victor Chernyakin <chernyakin.victor.j at outlook.com>
---
.../docs/clang-tidy/checks/readability/identifier-length.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 f08dbb89404c6..52910ef032f26 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
@@ -6,7 +6,7 @@ readability-identifier-length
This check finds variables and function parameters whose length are too short.
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.
+if they are short-lived.
Options
-------
More information about the cfe-commits
mailing list