[clang-tools-extra] [clang-tidy] `use-ranges`: preserve used unique results (PR #196035)
via cfe-commits
cfe-commits at lists.llvm.org
Wed May 6 03:02:45 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Daniil Dudkin (unterumarmung)
<details>
<summary>Changes</summary>
Preserve iterator uses when replacing std::unique with std::ranges::unique by appending .begin() in used-result contexts.
Fix #<!-- -->127658
Assisted by Codex.
---
Full diff: https://github.com/llvm/llvm-project/pull/196035.diff
7 Files Affected:
- (modified) clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp (+30-9)
- (modified) clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp (+41)
- (modified) clang-tools-extra/clang-tidy/utils/UseRangesCheck.h (+14)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
- (modified) clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst (+2)
- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h (+3)
- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp (+25)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp
index 28f77b4bc25ba..27fc52d95921e 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp
@@ -41,7 +41,6 @@ static constexpr const char *SingleRangeNames[] = {
"remove_if",
"remove_copy",
"remove_copy_if",
- "unique",
"unique_copy",
"sample",
"partition_point",
@@ -80,6 +79,8 @@ static constexpr const char *SingleRangeNames[] = {
"destroy",
};
+static constexpr const char *SingleRangeBeginResultNames[] = {"unique"};
+
static constexpr const char *TwoRangeNames[] = {
"equal",
"mismatch",
@@ -102,8 +103,11 @@ static constexpr const char *SinglePivotRangeNames[] = {"rotate", "rotate_copy",
namespace {
class StdReplacer : public utils::UseRangesCheck::Replacer {
public:
- explicit StdReplacer(SmallVector<UseRangesCheck::Signature> Signatures)
- : Signatures(std::move(Signatures)) {}
+ using ResultUsePolicy = utils::UseRangesCheck::Replacer::ResultUsePolicy;
+
+ explicit StdReplacer(SmallVector<UseRangesCheck::Signature> Signatures,
+ ResultUsePolicy ResultPolicy = {})
+ : Signatures(std::move(Signatures)), ResultPolicy(ResultPolicy) {}
std::optional<std::string>
getReplaceName(const NamedDecl &OriginalName) const override {
return ("std::ranges::" + OriginalName.getName()).str();
@@ -112,9 +116,13 @@ class StdReplacer : public utils::UseRangesCheck::Replacer {
getReplacementSignatures() const override {
return Signatures;
}
+ ResultUsePolicy getResultUsePolicy(const NamedDecl &, bool) const override {
+ return ResultPolicy;
+ }
private:
SmallVector<UseRangesCheck::Signature> Signatures;
+ ResultUsePolicy ResultPolicy;
};
class StdAlgorithmReplacer : public StdReplacer {
@@ -152,14 +160,27 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
static const Signature SinglePivotFunc[] = {SinglePivotRange};
- static constexpr std::pair<ArrayRef<Signature>, ArrayRef<const char *>>
- AlgorithmNames[] = {{SingleRangeFunc, SingleRangeNames},
- {TwoRangeFunc, TwoRangeNames},
- {SinglePivotFunc, SinglePivotRangeNames}};
+ using ResultPolicy = StdReplacer::ResultUsePolicy;
+ using PolicyKind = ResultPolicy::Kind;
+ const ResultPolicy DefaultPolicy;
+ const ResultPolicy BeginResultPolicy = {
+ PolicyKind::AppendAccessorForUsedResult, ".begin()"};
+
+ struct AlgorithmGroup {
+ ArrayRef<Signature> Signatures;
+ ArrayRef<const char *> Names;
+ ResultPolicy Policy;
+ };
+ const AlgorithmGroup AlgorithmNames[] = {
+ {SingleRangeFunc, SingleRangeNames, DefaultPolicy},
+ {SingleRangeFunc, SingleRangeBeginResultNames, BeginResultPolicy},
+ {TwoRangeFunc, TwoRangeNames, DefaultPolicy},
+ {SinglePivotFunc, SinglePivotRangeNames, DefaultPolicy},
+ };
SmallString<64> Buff;
- for (const auto &[Signatures, Values] : AlgorithmNames) {
+ for (const auto &[Signatures, Values, Policy] : AlgorithmNames) {
auto Replacer = llvm::makeIntrusiveRefCnt<StdAlgorithmReplacer>(
- SmallVector<UseRangesCheck::Signature>{Signatures});
+ SmallVector<UseRangesCheck::Signature>{Signatures}, Policy);
for (const auto &Name : Values) {
Buff.assign({"::std::", Name});
Result.try_emplace(Buff, Replacer);
diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index 1a1b0e1dba629..fb46ab157ee47 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/ASTMatchers/ASTMatchersInternal.h"
@@ -191,6 +192,33 @@ static void removeFunctionArgs(DiagnosticBuilder &Diag, const CallExpr &Call,
}
}
+static bool isResultUsed(const DynTypedNode &Node,
+ const ast_matchers::MatchFinder::MatchResult &Result) {
+ const DynTypedNodeList Parents = Result.Context->getParents(Node);
+ if (Parents.size() != 1)
+ return true;
+ if (Parents[0].get<CompoundStmt>())
+ return false;
+ if (const auto *Cleanups = Parents[0].get<ExprWithCleanups>())
+ return isResultUsed(DynTypedNode::create(*Cleanups), Result);
+ if (const auto *Temporary = Parents[0].get<CXXBindTemporaryExpr>())
+ return isResultUsed(DynTypedNode::create(*Temporary), Result);
+ return true;
+}
+
+static bool isResultUsed(const CallExpr &Call,
+ const ast_matchers::MatchFinder::MatchResult &Result) {
+ return isResultUsed(DynTypedNode::create(Call), Result);
+}
+
+static void insertAccessor(DiagnosticBuilder &Diag, const CallExpr &Call,
+ StringRef Accessor, const ASTContext &Ctx) {
+ const SourceLocation End = Lexer::getLocForEndOfToken(
+ Call.getEndLoc(), 0, Ctx.getSourceManager(), Ctx.getLangOpts());
+ if (End.isValid())
+ Diag << FixItHint::CreateInsertion(End, Accessor);
+}
+
void UseRangesCheck::check(const MatchFinder::MatchResult &Result) {
const Replacer *Replacer = nullptr;
const FunctionDecl *Function = nullptr;
@@ -226,6 +254,9 @@ void UseRangesCheck::check(const MatchFinder::MatchResult &Result) {
return;
}
+ const bool ResultUsed = isResultUsed(*Call, Result);
+ auto ResultPolicy = Replacer->getResultUsePolicy(*Function, false);
+
auto Diag = createDiag(*Call);
if (auto ReplaceName = Replacer->getReplaceName(*Function))
Diag << FixItHint::CreateReplacement(Call->getCallee()->getSourceRange(),
@@ -271,6 +302,11 @@ void UseRangesCheck::check(const MatchFinder::MatchResult &Result) {
ToRemove.push_back(Replace == Indexes::Second ? First : Second);
}
removeFunctionArgs(Diag, *Call, ToRemove, *Result.Context);
+ using ResultPolicyKind = Replacer::ResultUsePolicy::Kind;
+ if (ResultUsed && ResultPolicy.PolicyKind ==
+ ResultPolicyKind::AppendAccessorForUsedResult) {
+ insertAccessor(Diag, *Call, ResultPolicy.Accessor, *Result.Context);
+ }
return;
}
llvm_unreachable("No valid signature found");
@@ -301,6 +337,11 @@ UseRangesCheck::Replacer::getHeaderInclusion(const NamedDecl &) const {
return std::nullopt;
}
+UseRangesCheck::Replacer::ResultUsePolicy
+UseRangesCheck::Replacer::getResultUsePolicy(const NamedDecl &, bool) const {
+ return {};
+}
+
DiagnosticBuilder UseRangesCheck::createDiag(const CallExpr &Call) {
return diag(Call.getBeginLoc(), "use a ranges version of this algorithm");
}
diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h
index b85a157ba2873..6105ce1a1f351 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h
@@ -43,6 +43,16 @@ class UseRangesCheck : public ClangTidyCheck {
class Replacer : public llvm::RefCountedBase<Replacer> {
public:
+ struct ResultUsePolicy {
+ enum class Kind {
+ Preserve,
+ AppendAccessorForUsedResult,
+ };
+
+ Kind PolicyKind = Kind::Preserve;
+ StringRef Accessor;
+ };
+
/// Gets the name to replace a function with, return std::nullopt for a
/// replacement where we just call a different overload.
virtual std::optional<std::string>
@@ -56,6 +66,10 @@ class UseRangesCheck : public ClangTidyCheck {
/// Gets an array of all the possible overloads for a function with indexes
/// where begin and end arguments are.
virtual ArrayRef<Signature> getReplacementSignatures() const = 0;
+
+ /// Gets how to handle fix-its when the original algorithm result is used.
+ virtual ResultUsePolicy getResultUsePolicy(const NamedDecl &OriginalName,
+ bool IsStructuredBinding) const;
virtual ~Replacer() = default;
};
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d2db3dc2b3e78..62aada7cf3a82 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -432,6 +432,12 @@ Changes in existing checks
private deleted functions, if they do not have a public overload or are a
special member function.
+- Improved :doc:`modernize-use-ranges
+ <clang-tidy/checks/modernize/use-ranges>` check:
+
+ - Preserved used iterator results when replacing ``std::unique`` calls with
+ ``std::ranges::unique``.
+
- Improved :doc:`modernize-use-std-format
<clang-tidy/checks/modernize/use-std-format>` check:
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst
index eb90debb3804b..8d1a12490ceb3 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst
@@ -12,6 +12,7 @@ Example
.. code-block:: c++
auto Iter1 = std::find(Items.begin(), Items.end(), 0);
+ auto NewEnd = std::unique(Items.begin(), Items.end());
auto AreSame = std::equal(Items1.cbegin(), Items1.cend(),
std::begin(Items2), std::end(Items2));
@@ -21,6 +22,7 @@ Transforms to:
.. code-block:: c++
auto Iter1 = std::ranges::find(Items, 0);
+ auto NewEnd = std::ranges::unique(Items).begin();
auto AreSame = std::ranges::equal(Items1, Items2);
Supported algorithms
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h
index f417dd32d8946..648a399f9fcff 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h
@@ -89,6 +89,9 @@ bool equal(InputIt1 first1, InputIt1 last1, InputIt2 first2, InputIt2 last2,
template <class ForwardIt, class T>
void iota(ForwardIt first, ForwardIt last, T value);
+template <class ForwardIt>
+ForwardIt unique(ForwardIt first, ForwardIt last);
+
template <class ForwardIt>
ForwardIt rotate(ForwardIt first, ForwardIt middle, ForwardIt last);
} // namespace _V1
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 80b054b74b49a..be293e19a65a6 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
@@ -43,6 +43,18 @@ void Positives() {
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
// CHECK-FIXES: std::ranges::reverse(I);
+ auto LogicalEnd = std::unique(I.begin(), I.end());
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: auto LogicalEnd = std::ranges::unique(I).begin();
+
+ bool AlreadyUnique = std::unique(I.begin(), I.end()) == I.end();
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: bool AlreadyUnique = std::ranges::unique(I).begin() == I.end();
+
+ std::unique(I.begin(), I.end());
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: std::ranges::unique(I);
+
std::includes(I.begin(), I.end(), I.begin(), I.end());
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
// CHECK-FIXES: std::ranges::includes(I, I);
@@ -79,9 +91,18 @@ void Positives() {
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
// CHECK-FIXES: std::ranges::find(I, 5);
+ using std::unique;
+ auto LogicalEndFromUsing = unique(I.begin(), I.end());
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: auto LogicalEndFromUsing = std::ranges::unique(I).begin();
+
my_std::find(I.begin(), I.end(), 6);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
// CHECK-FIXES: std::ranges::find(I, 6);
+
+ 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();
}
void Reverse(){
@@ -99,6 +120,10 @@ void Reverse(){
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
// CHECK-FIXES: std::ranges::find(std::views::reverse(I), 0);
+ auto ReverseLogicalEnd = std::unique(I.rbegin(), I.rend());
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use a ranges version of this algorithm
+ // CHECK-FIXES: auto ReverseLogicalEnd = std::ranges::unique(std::views::reverse(I)).begin();
+
std::equal(std::rbegin(I), std::rend(I), J.begin(), J.end());
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
// CHECK-FIXES: std::ranges::equal(std::views::reverse(I), J);
``````````
</details>
https://github.com/llvm/llvm-project/pull/196035
More information about the cfe-commits
mailing list