-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[clang-tidy][NFC] Refactor bugprone-argument-comment [1/3]
#172521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: mitchell (zeyi2) ChangesThis is a necessary step to land #171757 Part of #170921 Full diff: https://github.com/llvm/llvm-project/pull/172521.diff 1 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
index ed30d01e645d1..0cb4f2f234dc0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -142,31 +142,44 @@ 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(),
- /*AllowReplacements=*/true, UpperBound);
+ 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)
+ for (const auto &Candidate : Candidates) {
+ const IdentifierInfo *II = Candidate->getIdentifier();
+ if (II->getName() == TargetName)
continue;
- const IdentifierInfo *II = Params[I]->getIdentifier();
+
if (!II)
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 +332,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());
}
@@ -335,8 +348,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")
|
|
✅ With the latest revision this PR passed the C/C++ code linter. |
| StringRef ArgName, unsigned ArgIndex) { | ||
| const std::string ArgNameLowerStr = ArgName.lower(); | ||
| const StringRef ArgNameLower = ArgNameLowerStr; | ||
| static llvm::SmallString<64> getLowercasedString(StringRef Name) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was added mainly to avoid the use of std::string
This is a necessary step to land #171757
Part of #170921