https://github.com/chomosuke updated
https://github.com/llvm/llvm-project/pull/118568
>From b43a2602025bdacea06ced5171904fb5d765de9f Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Tue, 3 Dec 2024 07:10:33 +0000
Subject: [PATCH 1/4] fixed removeFunctionArgs don't remove comma
---
.../clang-tidy/utils/UseRangesCheck.cpp | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index aba4d17ccd035e..88cba70b931d5d 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -28,6 +28,7 @@
#include "llvm/ADT/Twine.h"
#include "llvm/Support/raw_ostream.h"
#include <cassert>
+#include <iostream>
#include <optional>
#include <string>
@@ -173,21 +174,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag,
const CallExpr &Call,
for (unsigned Index : Sorted) {
const Expr *Arg = Call.getArg(Index);
if (Commas[Index]) {
- if (Index >= Commas.size()) {
- Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
- } else {
+ if (Index + 1 < Call.getNumArgs()) {
// Remove the next comma
Commas[Index + 1] = true;
+ const Expr *NextArg = Call.getArg(Index + 1);
Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- {Arg->getBeginLoc(),
- Lexer::getLocForEndOfToken(
- Arg->getEndLoc(), 0, Ctx.getSourceManager(),
Ctx.getLangOpts())
- .getLocWithOffset(1)}));
+ {Arg->getBeginLoc(),
NextArg->getBeginLoc().getLocWithOffset(-1)}));
+ } else {
+ Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
}
} else {
- Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc()));
+ // At this point we know Index > 0 because `Commas[0] = true` earlier
Commas[Index] = true;
+ const Expr *PrevArg = Call.getArg(Index - 1);
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc()));
}
}
}
>From 644c8491e0fba203e89595827781d0c2c0609081 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Mon, 23 Dec 2024 05:17:08 +1100
Subject: [PATCH 2/4] find , and remove only ,
---
.../clang-tidy/utils/UseRangesCheck.cpp | 49 +++++++++++++++----
1 file changed, 40 insertions(+), 9 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index 88cba70b931d5d..8b8e44a9898fdd 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -28,7 +28,6 @@
#include "llvm/ADT/Twine.h"
#include "llvm/Support/raw_ostream.h"
#include <cassert>
-#include <iostream>
#include <optional>
#include <string>
@@ -165,6 +164,33 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder)
{
static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call,
ArrayRef<unsigned> Indexes,
const ASTContext &Ctx) {
+ auto GetCommaLoc =
+ [&](SourceLocation BeginLoc,
+ SourceLocation EndLoc) -> std::optional<CharSourceRange> {
+ auto Invalid = false;
+ auto SourceText = Lexer::getSourceText(
+ CharSourceRange::getCharRange({BeginLoc, EndLoc}),
+ Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid);
+ assert(!Invalid);
+
+ size_t I = 0;
+ while (I < SourceText.size() && SourceText[I] != ',') {
+ I++;
+ }
+
+ if (I < SourceText.size()) {
+ // also remove space after ,
+ size_t J = I + 1;
+ while (J < SourceText.size() && SourceText[J] == ' ') {
+ J++;
+ }
+
+ return std::make_optional(CharSourceRange::getCharRange(
+ {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)}));
+ }
+ return std::nullopt;
+ };
+
llvm::SmallVector<unsigned> Sorted(Indexes);
llvm::sort(Sorted);
// Keep track of commas removed
@@ -176,20 +202,25 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag,
const CallExpr &Call,
if (Commas[Index]) {
if (Index + 1 < Call.getNumArgs()) {
// Remove the next comma
- Commas[Index + 1] = true;
const Expr *NextArg = Call.getArg(Index + 1);
- Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- {Arg->getBeginLoc(),
NextArg->getBeginLoc().getLocWithOffset(-1)}));
- } else {
- Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
+ auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1),
+ NextArg->getBeginLoc());
+ if (CommaLoc) {
+ Commas[Index + 1] = true;
+ Diag << FixItHint::CreateRemoval(*CommaLoc);
+ }
}
} else {
// At this point we know Index > 0 because `Commas[0] = true` earlier
- Commas[Index] = true;
const Expr *PrevArg = Call.getArg(Index - 1);
- Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- PrevArg->getEndLoc().getLocWithOffset(1), Arg->getEndLoc()));
+ auto CommaLoc = GetCommaLoc(PrevArg->getEndLoc().getLocWithOffset(1),
+ Arg->getBeginLoc());
+ if (CommaLoc) {
+ Commas[Index] = true;
+ Diag << FixItHint::CreateRemoval(*CommaLoc);
+ }
}
+ Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
}
}
>From 2e80b9bfc4357068d786c0c7e6f4a5de96ab50e3 Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Thu, 2 Jan 2025 20:18:23 +0800
Subject: [PATCH 3/4] no auto when type isn't obvious and added to release note
---
clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp | 10 +++++-----
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index 8b8e44a9898fdd..b126e940701fa7 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -168,7 +168,7 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag,
const CallExpr &Call,
[&](SourceLocation BeginLoc,
SourceLocation EndLoc) -> std::optional<CharSourceRange> {
auto Invalid = false;
- auto SourceText = Lexer::getSourceText(
+ StringRef SourceText = Lexer::getSourceText(
CharSourceRange::getCharRange({BeginLoc, EndLoc}),
Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid);
assert(!Invalid);
@@ -203,8 +203,8 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag,
const CallExpr &Call,
if (Index + 1 < Call.getNumArgs()) {
// Remove the next comma
const Expr *NextArg = Call.getArg(Index + 1);
- auto CommaLoc = GetCommaLoc(Arg->getEndLoc().getLocWithOffset(1),
- NextArg->getBeginLoc());
+ std::optional<CharSourceRange> CommaLoc = GetCommaLoc(
+ Arg->getEndLoc().getLocWithOffset(1), NextArg->getBeginLoc());
if (CommaLoc) {
Commas[Index + 1] = true;
Diag << FixItHint::CreateRemoval(*CommaLoc);
@@ -213,8 +213,8 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag,
const CallExpr &Call,
} else {
// At this point we know Index > 0 because `Commas[0] = true` earlier
const Expr *PrevArg = Call.getArg(Index - 1);
- auto CommaLoc = GetCommaLoc(PrevArg->getEndLoc().getLocWithOffset(1),
- Arg->getBeginLoc());
+ std::optional<CharSourceRange> CommaLoc = GetCommaLoc(
+ PrevArg->getEndLoc().getLocWithOffset(1), Arg->getBeginLoc());
if (CommaLoc) {
Commas[Index] = true;
Diag << FixItHint::CreateRemoval(*CommaLoc);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index fabd0cc78ac645..32ab7037ee9cef 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -355,6 +355,11 @@ Changes in existing checks
<clang-tidy/checks/readability/identifier-naming>` check to
validate ``namespace`` aliases.
+- Improved :doc:`modernize-use-ranges
+ <clang-tidy/checks/modernize/use-ranges>` and :doc:`boost-use-ranges
+ <clang-tidy/checks/boost/use-ranges>` check to more precisely remove
+ comma.
+
Removed checks
^^^^^^^^^^^^^^
>From 323fbe3ae21d3edb0d488de1f915e140af538d4c Mon Sep 17 00:00:00 2001
From: chomosuke <a13323...@gmail.com>
Date: Wed, 15 Jan 2025 16:19:07 +0800
Subject: [PATCH 4/4] Addressed comments
---
.../clang-tidy/utils/UseRangesCheck.cpp | 50 ++++++++-----------
clang-tools-extra/docs/ReleaseNotes.rst | 11 ++--
2 files changed, 27 insertions(+), 34 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index b126e940701fa7..eeb33708ab95b2 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "UseRangesCheck.h"
+#include "LexerUtils.h"
#include "Matchers.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
@@ -164,31 +165,22 @@ void UseRangesCheck::registerMatchers(MatchFinder
*Finder) {
static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call,
ArrayRef<unsigned> Indexes,
const ASTContext &Ctx) {
- auto GetCommaLoc =
- [&](SourceLocation BeginLoc,
- SourceLocation EndLoc) -> std::optional<CharSourceRange> {
- auto Invalid = false;
- StringRef SourceText = Lexer::getSourceText(
- CharSourceRange::getCharRange({BeginLoc, EndLoc}),
- Ctx.getSourceManager(), Ctx.getLangOpts(), &Invalid);
- assert(!Invalid);
+ auto GetCommaLoc = [&](SourceLocation BeginLoc,
+ SourceLocation EndLoc) -> CharSourceRange {
+ SourceLocation CommaLoc =
+ lexer::findNextAnyTokenKind(BeginLoc, Ctx.getSourceManager(),
+ Ctx.getLangOpts(), tok::comma, tok::comma);
- size_t I = 0;
- while (I < SourceText.size() && SourceText[I] != ',') {
- I++;
+ std::optional<Token> NextTok = lexer::findNextTokenIncludingComments(
+ CommaLoc, Ctx.getSourceManager(), Ctx.getLangOpts());
+
+ if (!NextTok) {
+ return {};
}
- if (I < SourceText.size()) {
- // also remove space after ,
- size_t J = I + 1;
- while (J < SourceText.size() && SourceText[J] == ' ') {
- J++;
- }
+ SourceLocation CommaEndLoc = NextTok->getLocation();
- return std::make_optional(CharSourceRange::getCharRange(
- {BeginLoc.getLocWithOffset(I), BeginLoc.getLocWithOffset(J)}));
- }
- return std::nullopt;
+ return CharSourceRange::getCharRange(CommaLoc, CommaEndLoc);
};
llvm::SmallVector<unsigned> Sorted(Indexes);
@@ -203,21 +195,21 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag,
const CallExpr &Call,
if (Index + 1 < Call.getNumArgs()) {
// Remove the next comma
const Expr *NextArg = Call.getArg(Index + 1);
- std::optional<CharSourceRange> CommaLoc = GetCommaLoc(
- Arg->getEndLoc().getLocWithOffset(1), NextArg->getBeginLoc());
- if (CommaLoc) {
+ CharSourceRange CommaLoc =
+ GetCommaLoc(Arg->getEndLoc(), NextArg->getBeginLoc());
+ if (CommaLoc.isValid()) {
Commas[Index + 1] = true;
- Diag << FixItHint::CreateRemoval(*CommaLoc);
+ Diag << FixItHint::CreateRemoval(CommaLoc);
}
}
} else {
// At this point we know Index > 0 because `Commas[0] = true` earlier
const Expr *PrevArg = Call.getArg(Index - 1);
- std::optional<CharSourceRange> CommaLoc = GetCommaLoc(
- PrevArg->getEndLoc().getLocWithOffset(1), Arg->getBeginLoc());
- if (CommaLoc) {
+ CharSourceRange CommaLoc =
+ GetCommaLoc(PrevArg->getEndLoc(), Arg->getBeginLoc());
+ if (CommaLoc.isValid()) {
Commas[Index] = true;
- Diag << FixItHint::CreateRemoval(*CommaLoc);
+ Diag << FixItHint::CreateRemoval(CommaLoc);
}
}
Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index f4388ffeba3070..b1103883e182b8 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -190,6 +190,9 @@ Changes in existing checks
<clang-tidy/checks/altera/id-dependent-backward-branch>` check by fixing
crashes from invalid code.
+- Improved :doc:`boost-use-ranges
+ <clang-tidy/checks/boost/use-ranges>` check to more precisely remove comma.
+
- Improved :doc:`bugprone-branch-clone
<clang-tidy/checks/bugprone/branch-clone>` check to improve detection of
branch clones by now detecting duplicate inner and outer if statements.
@@ -325,6 +328,9 @@ Changes in existing checks
<clang-tidy/checks/modernize/use-nullptr>` check to also recognize
``NULL``/``__null`` (but not ``0``) when used with a templated type.
+- Improved :doc:`modernize-use-ranges
+ <clang-tidy/checks/modernize/use-ranges>` check to more precisely remove
comma.
+
- Improved :doc:`modernize-use-starts-ends-with
<clang-tidy/checks/modernize/use-starts-ends-with>` check to handle two new
cases from ``rfind`` and ``compare`` to ``ends_with``, and one new case from
@@ -389,11 +395,6 @@ Changes in existing checks
<clang-tidy/checks/readability/use-std-min-max>` check to use correct
template
type in ``std::min`` and ``std::max`` when operand is integer literal.
-- Improved :doc:`modernize-use-ranges
- <clang-tidy/checks/modernize/use-ranges>` and :doc:`boost-use-ranges
- <clang-tidy/checks/boost/use-ranges>` check to more precisely remove
- comma.
-
Removed checks
^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
