[clang-tools-extra] [clang-tidy][NFC] Refactor `bugprone-argument-comment` [1/3] (PR #172521)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 16 09:29:26 PST 2025
https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/172521
>From f1c7bfc55dc2012fa40d4603713fb073901287b2 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Wed, 10 Dec 2025 22:34:47 +0800
Subject: [PATCH 1/5] [clang-tidy] Refactor isLikelyTypo to be a template
function
---
.../bugprone/ArgumentCommentCheck.cpp | 46 ++++++++++++-------
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
index ed30d01e645d1..0c1e73c378784 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -142,31 +142,45 @@ getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) {
return Comments;
}
-static bool isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
- StringRef ArgName, unsigned ArgIndex) {
- const std::string ArgNameLowerStr = ArgName.lower();
- const StringRef ArgNameLower = ArgNameLowerStr;
+static llvm::SmallString<64> getLowercasedString(StringRef Name) {
+ llvm::SmallString<64> Result;
+ Result.reserve(Name.size());
+ for (char C : Name)
+ Result.push_back(llvm::toLower(C));
+ return Result;
+}
+
+template <typename NamedDeclRange>
+static bool isLikelyTypo(const NamedDeclRange &Candidates, StringRef ArgName,
+ StringRef TargetName) {
+ const llvm::SmallString<64> ArgNameLower = getLowercasedString(ArgName);
+ const StringRef ArgNameLowerRef = StringRef(ArgNameLower);
// The threshold is arbitrary.
const unsigned UpperBound = ((ArgName.size() + 2) / 3) + 1;
- const unsigned ThisED = ArgNameLower.edit_distance(
- Params[ArgIndex]->getIdentifier()->getName().lower(),
+ const llvm::SmallString<64> TargetNameLower = getLowercasedString(TargetName);
+ const unsigned ThisED = ArgNameLowerRef.edit_distance(
+ StringRef(TargetNameLower),
/*AllowReplacements=*/true, UpperBound);
if (ThisED >= UpperBound)
return false;
- for (unsigned I = 0, E = Params.size(); I != E; ++I) {
- if (I == ArgIndex)
- continue;
- const IdentifierInfo *II = Params[I]->getIdentifier();
+ for (const auto &Candidate : Candidates) {
+ const IdentifierInfo *II = Candidate->getIdentifier();
if (!II)
continue;
+ // Skip the target itself.
+ if (II->getName() == TargetName)
+ continue;
+
const unsigned Threshold = 2;
- // Other parameters must be an edit distance at least Threshold more away
- // from this parameter. This gives us greater confidence that this is a
- // typo of this parameter and not one with a similar name.
- const unsigned OtherED = ArgNameLower.edit_distance(
- II->getName().lower(),
+ // Other candidates must be an edit distance at least Threshold more away
+ // from this candidate. This gives us greater confidence that this is a
+ // typo of this candidate and not one with a similar name.
+ const llvm::SmallString<64> CandidateLower =
+ getLowercasedString(II->getName());
+ const unsigned OtherED = ArgNameLowerRef.edit_distance(
+ StringRef(CandidateLower),
/*AllowReplacements=*/true, ThisED + Threshold);
if (OtherED < ThisED + Threshold)
return false;
@@ -319,7 +333,7 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
diag(Comment.first, "argument name '%0' in comment does not "
"match parameter name %1")
<< Matches[2] << II;
- if (isLikelyTypo(Callee->parameters(), Matches[2], I)) {
+ if (isLikelyTypo(Callee->parameters(), Matches[2], II->getName())) {
Diag << FixItHint::CreateReplacement(
Comment.first, (Matches[1] + II->getName() + Matches[3]).str());
}
>From 96323fde3f8c9f9204816db9c6db93a268937bb4 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Wed, 17 Dec 2025 00:49:31 +0800
Subject: [PATCH 2/5] [clang-tidy] Optimize string handling in
ArgumentCommentCheck
---
.../clang-tidy/bugprone/ArgumentCommentCheck.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
index 0c1e73c378784..e7eaebf2046da 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -349,8 +349,8 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
// If the argument comments are missing for literals add them.
if (Comments.empty() && shouldAddComment(Args[I])) {
- const std::string ArgComment =
- (llvm::Twine("/*") + II->getName() + "=*/").str();
+ llvm::SmallString<32> ArgComment;
+ (llvm::Twine("/*") + II->getName() + "=*/").toStringRef(ArgComment);
const DiagnosticBuilder Diag =
diag(Args[I]->getBeginLoc(),
"argument comment missing for literal argument %0")
>From e64e5153938ed174fab058352e808b5531625ed3 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Wed, 17 Dec 2025 00:50:58 +0800
Subject: [PATCH 3/5] Fix formatting
---
.../clang-tidy/bugprone/ArgumentCommentCheck.cpp | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
index e7eaebf2046da..0cb4f2f234dc0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -158,19 +158,18 @@ static bool isLikelyTypo(const NamedDeclRange &Candidates, StringRef ArgName,
// The threshold is arbitrary.
const unsigned UpperBound = ((ArgName.size() + 2) / 3) + 1;
const llvm::SmallString<64> TargetNameLower = getLowercasedString(TargetName);
- const unsigned ThisED = ArgNameLowerRef.edit_distance(
- StringRef(TargetNameLower),
- /*AllowReplacements=*/true, UpperBound);
+ const unsigned ThisED =
+ ArgNameLowerRef.edit_distance(StringRef(TargetNameLower),
+ /*AllowReplacements=*/true, UpperBound);
if (ThisED >= UpperBound)
return false;
for (const auto &Candidate : Candidates) {
const IdentifierInfo *II = Candidate->getIdentifier();
- if (!II)
+ if (II->getName() == TargetName)
continue;
- // Skip the target itself.
- if (II->getName() == TargetName)
+ if (!II)
continue;
const unsigned Threshold = 2;
>From 7b101b20d684fd41ed51282244fcb04b6dd94fd7 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Wed, 17 Dec 2025 01:20:49 +0800
Subject: [PATCH 4/5] Fix clang-tidy warnings, though it may be ugly..
---
.../bugprone/ArgumentCommentCheck.cpp | 45 +++++++++----------
1 file changed, 22 insertions(+), 23 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
index 0cb4f2f234dc0..033818a30be25 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -145,7 +145,7 @@ getCommentsBeforeLoc(ASTContext *Ctx, SourceLocation Loc) {
static llvm::SmallString<64> getLowercasedString(StringRef Name) {
llvm::SmallString<64> Result;
Result.reserve(Name.size());
- for (char C : Name)
+ for (const char C : Name)
Result.push_back(llvm::toLower(C));
return Result;
}
@@ -164,28 +164,27 @@ static bool isLikelyTypo(const NamedDeclRange &Candidates, StringRef ArgName,
if (ThisED >= UpperBound)
return false;
- for (const auto &Candidate : Candidates) {
- const IdentifierInfo *II = Candidate->getIdentifier();
- if (II->getName() == TargetName)
- continue;
-
- if (!II)
- continue;
-
- const unsigned Threshold = 2;
- // Other candidates must be an edit distance at least Threshold more away
- // from this candidate. This gives us greater confidence that this is a
- // typo of this candidate and not one with a similar name.
- const llvm::SmallString<64> CandidateLower =
- getLowercasedString(II->getName());
- const unsigned OtherED = ArgNameLowerRef.edit_distance(
- StringRef(CandidateLower),
- /*AllowReplacements=*/true, ThisED + Threshold);
- if (OtherED < ThisED + Threshold)
- return false;
- }
-
- return true;
+ return std::all_of(Candidates.begin(), Candidates.end(),
+ [&](const auto &Candidate) {
+ const IdentifierInfo *II = Candidate->getIdentifier();
+ if (II->getName() == TargetName)
+ return true;
+
+ if (!II)
+ return true;
+
+ const unsigned Threshold = 2;
+ // Other candidates must be an edit distance at least
+ // Threshold more away from this candidate. This gives us
+ // greater confidence that this is a typo of this
+ // candidate and not one with a similar name.
+ const llvm::SmallString<64> CandidateLower =
+ getLowercasedString(II->getName());
+ const unsigned OtherED = ArgNameLowerRef.edit_distance(
+ StringRef(CandidateLower),
+ /*AllowReplacements=*/true, ThisED + Threshold);
+ return OtherED >= ThisED + Threshold;
+ });
}
static bool sameName(StringRef InComment, StringRef InDecl, bool StrictMode) {
>From ae5b6e844f2ed38bca5dd0432a9032fe89cacbf5 Mon Sep 17 00:00:00 2001
From: mtx <mitchell.xu2 at gmail.com>
Date: Wed, 17 Dec 2025 01:28:35 +0800
Subject: [PATCH 5/5] use `llvm::all_of`
---
.../bugprone/ArgumentCommentCheck.cpp | 41 +++++++++----------
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
index 033818a30be25..2140ba5796006 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -164,27 +164,26 @@ static bool isLikelyTypo(const NamedDeclRange &Candidates, StringRef ArgName,
if (ThisED >= UpperBound)
return false;
- return std::all_of(Candidates.begin(), Candidates.end(),
- [&](const auto &Candidate) {
- const IdentifierInfo *II = Candidate->getIdentifier();
- if (II->getName() == TargetName)
- return true;
-
- if (!II)
- return true;
-
- const unsigned Threshold = 2;
- // Other candidates must be an edit distance at least
- // Threshold more away from this candidate. This gives us
- // greater confidence that this is a typo of this
- // candidate and not one with a similar name.
- const llvm::SmallString<64> CandidateLower =
- getLowercasedString(II->getName());
- const unsigned OtherED = ArgNameLowerRef.edit_distance(
- StringRef(CandidateLower),
- /*AllowReplacements=*/true, ThisED + Threshold);
- return OtherED >= ThisED + Threshold;
- });
+ return llvm::all_of(Candidates, [&](const auto &Candidate) {
+ const IdentifierInfo *II = Candidate->getIdentifier();
+ if (II->getName() == TargetName)
+ return true;
+
+ if (!II)
+ return true;
+
+ const unsigned Threshold = 2;
+ // Other candidates must be an edit distance at least
+ // Threshold more away from this candidate. This gives us
+ // greater confidence that this is a typo of this
+ // candidate and not one with a similar name.
+ const llvm::SmallString<64> CandidateLower =
+ getLowercasedString(II->getName());
+ const unsigned OtherED = ArgNameLowerRef.edit_distance(
+ StringRef(CandidateLower),
+ /*AllowReplacements=*/true, ThisED + Threshold);
+ return OtherED >= ThisED + Threshold;
+ });
}
static bool sameName(StringRef InComment, StringRef InDecl, bool StrictMode) {
More information about the cfe-commits
mailing list