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/3] 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/3] 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/3] 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 ^^^^^^^^^^^^^^ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits