[clang-tools-extra] [clang-tidy][clangd] modernize-use-ranges: use token-based comma removal (PR #234687)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 23 21:55:35 PDT 2026
https://github.com/Evilpasture updated https://github.com/llvm/llvm-project/pull/234687
>From 79af3d5ac938df6ee0590d93608d6a7b0183d68f Mon Sep 17 00:00:00 2001
From: Evilpasture <mandatorythebest at gmail.com>
Date: Sat, 24 Oct 2026 11:55:03 +0700
Subject: [PATCH] [clang-tidy] modernize-use-ranges: use token-based comma
removal
---
.../clang-tidy/utils/UseRangesCheck.cpp | 152 ++++++++++++++++--
clang-tools-extra/docs/ReleaseNotes.md | 14 ++
.../modernize/use-ranges-comma-fixit.cpp | 66 ++++++++
.../checkers/modernize/use-ranges.cpp | 34 ++++
4 files changed, 249 insertions(+), 17 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-comma-fixit.cpp
diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index 67db20e15934f0..f895722a364803 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -18,7 +18,9 @@
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Token.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallBitVector.h"
@@ -172,11 +174,116 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) {
}
}
-static void removeFunctionArgs(const DiagnosticBuilder &Diag,
- const CallExpr &Call, ArrayRef<unsigned> Indexes,
- const ASTContext &Ctx) {
+static CharSourceRange findCommaBetween(const Expr &PrevArg,
+ const Expr &NextArg,
+ const ASTContext &Ctx,
+ bool NextArgRemoved) {
+ const SourceManager &SM = Ctx.getSourceManager();
+ const LangOptions &LangOpts = Ctx.getLangOpts();
+ const CharSourceRange PrevRange = Lexer::makeFileCharRange(
+ CharSourceRange::getTokenRange(PrevArg.getSourceRange()), SM, LangOpts);
+ const CharSourceRange NextRange = Lexer::makeFileCharRange(
+ CharSourceRange::getTokenRange(NextArg.getSourceRange()), SM, LangOpts);
+ if (PrevRange.isInvalid() || NextRange.isInvalid())
+ return {};
+
+ const CharSourceRange FileRange =
+ CharSourceRange::getCharRange(PrevRange.getEnd(), NextRange.getBegin());
+ const auto [BeginFID, BeginOffset] =
+ SM.getDecomposedLoc(FileRange.getBegin());
+ const auto [EndFID, EndOffset] = SM.getDecomposedLoc(FileRange.getEnd());
+ if (BeginFID != EndFID || BeginOffset > EndOffset)
+ return {};
+
+ bool Invalid = false;
+ const StringRef Buffer = SM.getBufferData(BeginFID, &Invalid);
+ if (Invalid)
+ return {};
+
+ Lexer TheLexer(SM.getLocForStartOfFile(BeginFID), LangOpts, Buffer.begin(),
+ Buffer.data() + BeginOffset, Buffer.end());
+ TheLexer.SetCommentRetentionState(true);
+
+ unsigned ParenDepth = 0;
+ bool HasTokensBeforeComma = false;
+ SourceLocation LastTokenEnd = FileRange.getBegin();
+ std::optional<Token> CommaTok;
+ std::optional<Token> AfterCommaTok;
+
+ while (true) {
+ Token Tok;
+ TheLexer.LexFromRawLexer(Tok);
+
+ if (Tok.is(tok::eof) || Tok.getLocation() == FileRange.getEnd() ||
+ SM.isBeforeInTranslationUnit(FileRange.getEnd(), Tok.getLocation()))
+ break;
+
+ if (CommaTok && !AfterCommaTok)
+ AfterCommaTok = Tok;
+
+ // Bail out on preprocessor directives (e.g. #if / #ifdef) in the gap.
+ if (Tok.is(tok::hash))
+ return {};
+
+ if (Tok.is(tok::l_paren)) {
+ ++ParenDepth;
+ continue;
+ }
+ if (Tok.is(tok::r_paren)) {
+ if (ParenDepth == 0)
+ return {};
+ --ParenDepth;
+ if (ParenDepth == 0 && !CommaTok) {
+ HasTokensBeforeComma = true;
+ LastTokenEnd = Tok.getEndLoc();
+ }
+ continue;
+ }
+
+ if (ParenDepth == 0 && Tok.is(tok::comma)) {
+ if (CommaTok)
+ return {};
+ CommaTok = Tok;
+ continue;
+ }
+
+ if (ParenDepth == 0 && !CommaTok && Tok.isNot(tok::comment)) {
+ HasTokensBeforeComma = true;
+ LastTokenEnd = Tok.getEndLoc();
+ }
+ }
+
+ if (!CommaTok || ParenDepth != 0)
+ return {};
+
+ if (!AfterCommaTok && (NextArgRemoved || !HasTokensBeforeComma))
+ return CharSourceRange::getCharRange(CommaTok->getLocation(),
+ FileRange.getEnd());
+
+ // When a comment or empty macro stands adjacent to the comma, remove only
+ // the comma token itself so separating whitespace is preserved. Bail out if
+ // removing `,` with no surrounding whitespace or comment would paste two
+ // tokens together (e.g. `I.begin(),EMPTY(1, 2) I.end()`).
+ const bool TightBefore = (NextArgRemoved || HasTokensBeforeComma) &&
+ CommaTok->getLocation() == LastTokenEnd;
+ const bool TightAfter =
+ AfterCommaTok
+ ? (AfterCommaTok->isNot(tok::comment) &&
+ CommaTok->getEndLoc() == AfterCommaTok->getLocation())
+ : (!NextArgRemoved && CommaTok->getEndLoc() == FileRange.getEnd());
+ if (TightBefore && TightAfter)
+ return {};
+
+ return CharSourceRange::getCharRange(CommaTok->getLocation(),
+ CommaTok->getEndLoc());
+}
+
+static std::optional<SmallVector<FixItHint>>
+getArgRemovalFixes(const CallExpr &Call, ArrayRef<unsigned> Indexes,
+ const ASTContext &Ctx) {
SmallVector<unsigned> Sorted(Indexes);
llvm::sort(Sorted);
+ SmallVector<FixItHint> Fixes;
// Keep track of commas removed
llvm::SmallBitVector Commas(Call.getNumArgs());
// The first comma is actually the '(' which we can't remove
@@ -184,23 +291,27 @@ static void removeFunctionArgs(const DiagnosticBuilder &Diag,
for (const unsigned Index : Sorted) {
const Expr *Arg = Call.getArg(Index);
if (Commas[Index]) {
- if (Index >= Commas.size()) {
- Diag << FixItHint::CreateRemoval(Arg->getSourceRange());
- } else {
- // Remove the next comma
+ if (Index + 1 < Call.getNumArgs()) {
+ // Remove the next comma (including trailing trivia when safe).
+ const CharSourceRange CommaRange = findCommaBetween(
+ *Arg, *Call.getArg(Index + 1), Ctx, /*NextArgRemoved=*/false);
+ if (CommaRange.isInvalid())
+ return std::nullopt;
Commas[Index + 1] = true;
- Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- {Arg->getBeginLoc(),
- Lexer::getLocForEndOfToken(
- Arg->getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts())
- .getLocWithOffset(1)}));
+ Fixes.push_back(FixItHint::CreateRemoval(CommaRange));
}
} else {
- Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- Arg->getBeginLoc().getLocWithOffset(-1), Arg->getEndLoc()));
+ assert(Index > 0);
+ const CharSourceRange CommaRange = findCommaBetween(
+ *Call.getArg(Index - 1), *Arg, Ctx, /*NextArgRemoved=*/true);
+ if (CommaRange.isInvalid())
+ return std::nullopt;
Commas[Index] = true;
+ Fixes.push_back(FixItHint::CreateRemoval(CommaRange));
}
+ Fixes.push_back(FixItHint::CreateRemoval(Arg->getSourceRange()));
}
+ return Fixes;
}
static bool isResultUsed(const DynTypedNode &Node,
@@ -293,13 +404,20 @@ void UseRangesCheck::check(const MatchFinder::MatchResult &Result) {
if (!shouldEmitFixIts(ResultPolicy, ResultUsed, IsStructuredBinding))
return;
+ SmallVector<unsigned, 3> ToRemove;
+ for (const auto &[First, Second, Replace] : Sig)
+ ToRemove.push_back(Replace == Indexes::Second ? First : Second);
+ const std::optional<SmallVector<FixItHint>> RemovalFixes =
+ getArgRemovalFixes(*Call, ToRemove, *Result.Context);
+ if (!RemovalFixes)
+ return;
+
if (auto ReplaceName = Replacer->getReplaceName(*Function))
Diag << FixItHint::CreateReplacement(Call->getCallee()->getSourceRange(),
*ReplaceName);
if (auto Include = Replacer->getHeaderInclusion(*Function))
Diag << Inserter.createIncludeInsertion(
Result.SourceManager->getFileID(Call->getBeginLoc()), *Include);
- SmallVector<unsigned, 3> ToRemove;
for (const auto &[First, Second, Replace] : Sig) {
auto ArgNode = ArgName + std::to_string(First);
if (const auto *ArgExpr = Result.Nodes.getNodeAs<Expr>(ArgNode)) {
@@ -334,9 +452,9 @@ void UseRangesCheck::check(const MatchFinder::MatchResult &Result) {
->getSourceRange(),
ReplaceText);
}
- ToRemove.push_back(Replace == Indexes::Second ? First : Second);
}
- removeFunctionArgs(Diag, *Call, ToRemove, *Result.Context);
+ for (const FixItHint &Fix : *RemovalFixes)
+ Diag << Fix;
using ResultPolicyKind = Replacer::ResultUsePolicy::Kind;
if (ResultUsed && ResultPolicy.PolicyKind ==
ResultPolicyKind::AppendAccessorForUsedResult) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md
index 57ed93042aff2c..8b070196af5e93 100644
--- a/clang-tools-extra/docs/ReleaseNotes.md
+++ b/clang-tools-extra/docs/ReleaseNotes.md
@@ -184,6 +184,11 @@ infrastructure are described first, followed by tool-specific sections.
#### Changes in existing checks
+- Improved {doc}`boost-use-ranges
+ <clang-tidy/checks/boost/use-ranges>` check by using token-based comma
+ removal for argument fix-its and suppressing partial fix-its when a comma
+ comes from a macro expansion.
+
- Improved {doc}`bugprone-assignment-in-if-condition
<clang-tidy/checks/bugprone/assignment-in-if-condition>` check by ignoring
assignments inside GNU statement expressions unless they form the value of
@@ -249,6 +254,11 @@ infrastructure are described first, followed by tool-specific sections.
diagnostics are still emitted at the class name for implicitly declared
destructors.
+- Improved {doc}`llvm-use-ranges
+ <clang-tidy/checks/llvm/use-ranges>` check by using token-based comma
+ removal for argument fix-its and suppressing partial fix-its when a comma
+ comes from a macro expansion.
+
- Improved {doc}`misc-const-correctness
<clang-tidy/checks/misc/const-correctness>` check:
@@ -316,6 +326,10 @@ infrastructure are described first, followed by tool-specific sections.
- Kept diagnostics but suppressed unsafe fix-its when no safe
result-preserving rewrite is available.
+ - Fixed comma removal in argument fix-its using token-based lexing instead of
+ fixed character offsets, and suppressed partial fix-its when a comma comes
+ from a macro expansion.
+
- Improved {doc}`performance-inefficient-algorithm
<clang-tidy/checks/performance/inefficient-algorithm>` check to no longer
produce a fix with the container or the searched-for value missing, such as
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-comma-fixit.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-comma-fixit.cpp
new file mode 100644
index 00000000000000..ca93f36a98b7bb
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-comma-fixit.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy -std=c++20 -export-fixes=%t.yaml %s modernize-use-ranges %t -- -- -I %S/Inputs/
+// RUN: FileCheck -input-file=%t.yaml %s
+
+#include "use-ranges/fake_std.h"
+
+void commaFixIts(std::vector<int> &I, std::vector<int> &J) {
+ // Verify raw FixIt replacements cover the full "I.begin(), I.end()" span (18
+ // bytes = 11 + 7) rather than skipping the comma token.
+ std::reverse(I.begin(), I.end());
+ // CHECK-LABEL: - DiagnosticName: modernize-use-ranges
+ // CHECK: ReplacementText: 'std::ranges::reverse'
+ // CHECK: Offset: [[#R1:]]
+ // CHECK-NEXT: Length: 11
+ // CHECK-NEXT: ReplacementText: I
+ // CHECK: Offset: [[#R1 + 11]]
+ // CHECK-NEXT: Length: 7
+ // CHECK-NEXT: ReplacementText: ''
+
+ std::reverse(I.begin() , I.end());
+ // CHECK-LABEL: - DiagnosticName: modernize-use-ranges
+ // CHECK: ReplacementText: 'std::ranges::reverse'
+ // CHECK: Offset: [[#R2:]]
+ // CHECK-NEXT: Length: 9
+ // CHECK-NEXT: ReplacementText: I
+ // CHECK: Offset: [[#R2 + 10]]
+ // CHECK-NEXT: Length: 9
+ // CHECK-NEXT: ReplacementText: ''
+
+ std::find(I.begin(), I.end(), 0);
+ // CHECK-LABEL: - DiagnosticName: modernize-use-ranges
+ // CHECK: ReplacementText: 'std::ranges::find'
+ // CHECK: Offset: [[#R3:]]
+ // CHECK-NEXT: Length: 11
+ // CHECK-NEXT: ReplacementText: I
+ // CHECK: Offset: [[#R3 + 11]]
+ // CHECK-NEXT: Length: 7
+ // CHECK-NEXT: ReplacementText: ''
+
+ std::equal(I.begin(), I.end(), J.begin(), J.end());
+ // CHECK-LABEL: - DiagnosticName: modernize-use-ranges
+ // CHECK: ReplacementText: 'std::ranges::equal'
+ // CHECK: Offset: [[#R4A:]]
+ // CHECK-NEXT: Length: 11
+ // CHECK-NEXT: ReplacementText: I
+ // CHECK: Offset: [[#R4A + 11]]
+ // CHECK-NEXT: Length: 7
+ // CHECK-NEXT: ReplacementText: ''
+ // CHECK: Offset: [[#R4A + 20]]
+ // CHECK-NEXT: Length: 11
+ // CHECK-NEXT: ReplacementText: J
+ // CHECK: Offset: [[#R4A + 31]]
+ // CHECK-NEXT: Length: 7
+ // CHECK-NEXT: ReplacementText: ''
+
+#define EMPTY(a, b)
+ std::reverse(I.begin() EMPTY(1, 2), I.end());
+ // CHECK-LABEL: - DiagnosticName: modernize-use-ranges
+ // CHECK: ReplacementText: 'std::ranges::reverse'
+ // CHECK: Offset: [[#R5:]]
+ // CHECK-NEXT: Length: 9
+ // CHECK-NEXT: ReplacementText: I
+ // CHECK: Offset: [[#R5 + 21]]
+ // CHECK-NEXT: Length: 9
+ // CHECK-NEXT: ReplacementText: ''
+#undef EMPTY
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp
index 958b47c1c1ee12..6cc84e308dc9b7 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp
@@ -275,6 +275,40 @@ void Positives() {
auto LogicalEndFromNamespaceAlias = my_std::unique(I.begin(), I.end());
// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: use a ranges version of this algorithm
// CHECK-FIXES: auto LogicalEndFromNamespaceAlias = std::ranges::unique(I).begin();
+
+ std::reverse(I.begin() /*begin*/, I.end());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: std::ranges::reverse(I /*begin*/);
+
+ std::reverse(I.begin(),/*end*/I.end());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: std::ranges::reverse(I/*end*/);
+
+#define COMMA ,
+ std::reverse(I.begin() COMMA I.end());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: std::reverse(I.begin() COMMA I.end());
+#undef COMMA
+
+#define EMPTY(a, b)
+ std::reverse(I.begin() EMPTY(1, 2), I.end());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: std::ranges::reverse(I EMPTY(1, 2));
+
+ std::reverse(I.begin() EMPTY((1, 2), 3), I.end());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: std::ranges::reverse(I EMPTY((1, 2), 3));
+
+ std::reverse(I.begin(), EMPTY(1, 2) I.end());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: std::ranges::reverse(I EMPTY(1, 2) );
+#undef EMPTY
+
+#define SEP(a, b) ,
+ std::reverse(I.begin() SEP(1, 2) I.end());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: std::reverse(I.begin() SEP(1, 2) I.end());
+#undef SEP
}
void Reverse(){
More information about the cfe-commits
mailing list