llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Alex Dutka (dutkalex)
<details>
<summary>Changes</summary>
This PR implements the feature described in #<!-- -->185318
A new parameter named `LineCountThreshold` to the
`readability-identifier-length` check, which controls how much lines of code
must separate the the last use of a variable from its declaration for the check
to warn. For backwards-compatibility, the default value for this parameter is
set to 0.
Increasing the threshold to 1 allows for short names in one-liners (for
example: `std::transform(..., [](auto i){ return i*i; });`), and in the general
case with `LineCountThreshold = N` a variable is allowed to have a shorter name
than otherwise required if it is never used again after `N` lines (including
its declaration line).
This feature is implemented using a secondary `MatchFinder` for each variable
with a short name. For performance reasons, the new piece of code is
short-circuited if `LineCountThreshold` is set to 0 (the default value).
---
Full diff: https://github.com/llvm/llvm-project/pull/185319.diff
3 Files Affected:
- (modified) clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
(+48-1)
- (modified) clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
(+2)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-length.cpp
(+21-12)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
index a6204de16224d..4755588b70d18 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
@@ -21,6 +21,7 @@ const char DefaultIgnoredLoopCounterNames[] = "^[ijk_]$";
const char DefaultIgnoredVariableNames[] = "";
const char DefaultIgnoredExceptionVariableNames[] = "^[e]$";
const char DefaultIgnoredParameterNames[] = "^[n]$";
+const unsigned DefaultLineCountThreshold = 0;
const char ErrorMessage[] =
"%select{variable|exception variable|loop variable|"
@@ -49,7 +50,8 @@ IdentifierLengthCheck::IdentifierLengthCheck(StringRef Name,
IgnoredExceptionVariableNames(IgnoredExceptionVariableNamesInput),
IgnoredParameterNamesInput(
Options.get("IgnoredParameterNames", DefaultIgnoredParameterNames)),
- IgnoredParameterNames(IgnoredParameterNamesInput) {}
+ IgnoredParameterNames(IgnoredParameterNamesInput),
+ LineCountThreshold(Options.get("LineCountThreshold",
DefaultLineCountThreshold)) {}
void IdentifierLengthCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "MinimumVariableNameLength", MinimumVariableNameLength);
@@ -62,6 +64,7 @@ void
IdentifierLengthCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IgnoredExceptionVariableNames",
IgnoredExceptionVariableNamesInput);
Options.store(Opts, "IgnoredParameterNames", IgnoredParameterNamesInput);
+ Options.store(Opts, "LineCountThreshold", LineCountThreshold);
}
void IdentifierLengthCheck::registerMatchers(MatchFinder *Finder) {
@@ -85,6 +88,38 @@ void IdentifierLengthCheck::registerMatchers(MatchFinder
*Finder) {
this);
}
+static unsigned countLinesToLastUse(const VarDecl* Var, const SourceManager*
SrcMgr, ASTContext* Ctx){
+ const unsigned DeclLine = SrcMgr->getSpellingLineNumber(Var->getLocation());
+
+ class VarUseCallback : public MatchFinder::MatchCallback {
+ private:
+ unsigned* LastUseLineNumber;
+
+ public:
+ explicit VarUseCallback(unsigned* Output): LastUseLineNumber{Output} {}
+
+ void run(const MatchFinder::MatchResult &Result) override {
+ const DeclRefExpr *Use = Result.Nodes.getNodeAs<DeclRefExpr>("varUse");
+ if (Use && LastUseLineNumber) {
+ auto Loc = Use->getLocation();
+ unsigned UseLine = Result.SourceManager->getSpellingLineNumber(Loc);
+ *LastUseLineNumber = std::max(*LastUseLineNumber, UseLine);
+ }
+ }
+ };
+
+ unsigned LastUseLine = DeclLine;
+ VarUseCallback Callback{&LastUseLine};
+
+ auto Matcher = declRefExpr(to(varDecl(equalsNode(Var)))).bind("varUse");
+
+ MatchFinder Finder;
+ Finder.addMatcher(Matcher, &Callback);
+ Finder.matchAST(*Ctx);
+
+ return LastUseLine - DeclLine + 1;
+}
+
void IdentifierLengthCheck::check(const MatchFinder::MatchResult &Result) {
const auto *StandaloneVar = Result.Nodes.getNodeAs<VarDecl>("standaloneVar");
if (StandaloneVar) {
@@ -97,6 +132,9 @@ void IdentifierLengthCheck::check(const
MatchFinder::MatchResult &Result) {
IgnoredVariableNames.match(VarName))
return;
+ if (LineCountThreshold > 0 && countLinesToLastUse(StandaloneVar,
Result.SourceManager, Result.Context) <= LineCountThreshold)
+ return;
+
diag(StandaloneVar->getLocation(), ErrorMessage)
<< 0 << StandaloneVar << MinimumVariableNameLength;
}
@@ -111,6 +149,9 @@ void IdentifierLengthCheck::check(const
MatchFinder::MatchResult &Result) {
IgnoredExceptionVariableNames.match(VarName))
return;
+ if (LineCountThreshold > 0 && countLinesToLastUse(ExceptionVarName,
Result.SourceManager, Result.Context) <= LineCountThreshold)
+ return;
+
diag(ExceptionVarName->getLocation(), ErrorMessage)
<< 1 << ExceptionVarName << MinimumExceptionNameLength;
}
@@ -126,6 +167,9 @@ void IdentifierLengthCheck::check(const
MatchFinder::MatchResult &Result) {
IgnoredLoopCounterNames.match(VarName))
return;
+ if (LineCountThreshold > 0 && countLinesToLastUse(LoopVar,
Result.SourceManager, Result.Context) <= LineCountThreshold)
+ return;
+
diag(LoopVar->getLocation(), ErrorMessage)
<< 2 << LoopVar << MinimumLoopCounterNameLength;
}
@@ -141,6 +185,9 @@ void IdentifierLengthCheck::check(const
MatchFinder::MatchResult &Result) {
IgnoredParameterNames.match(VarName))
return;
+ if (LineCountThreshold > 0 && countLinesToLastUse(ParamVar,
Result.SourceManager, Result.Context) <= LineCountThreshold)
+ return;
+
diag(ParamVar->getLocation(), ErrorMessage)
<< 3 << ParamVar << MinimumParameterNameLength;
}
diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
index 3adaf50bc57a1..2a59788260f8c 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
@@ -42,6 +42,8 @@ class IdentifierLengthCheck : public ClangTidyCheck {
std::string IgnoredParameterNamesInput;
llvm::Regex IgnoredParameterNames;
+
+ const unsigned LineCountThreshold;
};
} // namespace clang::tidy::readability
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-length.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-length.cpp
index 33b0ac7ce7aa3..1972862d05f3e 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-length.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-length.cpp
@@ -1,6 +1,6 @@
// RUN: %check_clang_tidy %s readability-identifier-length %t \
// RUN: -config='{CheckOptions: \
-// RUN: {readability-identifier-length.IgnoredVariableNames: "^[xy]$"}}' \
+// RUN: {readability-identifier-length.IgnoredVariableNames: "^[xy]$",
readability-identifier-length.LineCountThreshold: 1}}' \
// RUN: -- -fexceptions
struct myexcept {
@@ -11,7 +11,8 @@ struct simpleexcept {
int other;
};
-void doIt();
+template<typename... Ts>
+void doIt(Ts...);
void tooShortVariableNames(int z)
// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: parameter name 'z' is too short,
expected at least 3 characters [readability-identifier-length]
@@ -25,39 +26,47 @@ void tooShortVariableNames(int z)
for (int m = 0; m < 5; ++m)
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: loop variable name 'm' is too
short, expected at least 2 characters [readability-identifier-length]
{
- doIt();
+ doIt(i, jj, m);
}
try {
- doIt();
+ doIt(z);
} catch (const myexcept &x)
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: exception variable name 'x' is
too short, expected at least 2 characters [readability-identifier-length]
{
- doIt();
+ doIt(x);
}
}
-void longEnoughVariableNames(int n) // argument 'n' ignored by default
configuration
+void longEnoughVariableNames(int n, int m) // argument 'n' ignored by default
configuration, 'm' is only used on this line
{
int var = 5;
for (int i = 0; i < 42; ++i) // 'i' is default allowed, for historical
reasons
{
- doIt();
+ doIt(var, i);
+ }
+
+ for (int a = 0; a < 42; ++a) // 'a' is only used on this line
+ {
+ doIt();
}
for (int kk = 0; kk < 42; ++kk) {
- doIt();
+ doIt(kk);
}
try {
- doIt();
+ doIt(n);
} catch (const simpleexcept &e) // ignored by default configuration
{
- doIt();
+ doIt(e);
} catch (const myexcept &ex) {
- doIt();
- }
+ doIt(ex);
+ } catch (const int &d) { doIt(d); } // 'd' is only used on this line
int x = 5; // ignored by configuration
+ ++x;
+
+ int b = 0; // 'b' is only used on this line
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/185319
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
