https://github.com/unterumarmung created https://github.com/llvm/llvm-project/pull/196035
Preserve iterator uses when replacing std::unique with std::ranges::unique by appending .begin() in used-result contexts. Fix #127658 Assisted by Codex. >From ef6da5e383c3ed7c5a6a31140f551ef8dfc9ad50 Mon Sep 17 00:00:00 2001 From: Daniil Dudkin <[email protected]> Date: Wed, 6 May 2026 00:25:03 +0300 Subject: [PATCH] [clang-tidy] `use-ranges`: preserve used unique results Preserve iterator uses when replacing std::unique with std::ranges::unique by appending .begin() in used-result contexts. Fix #127658 Assisted by Codex. --- .../clang-tidy/modernize/UseRangesCheck.cpp | 39 ++++++++++++++---- .../clang-tidy/utils/UseRangesCheck.cpp | 41 +++++++++++++++++++ .../clang-tidy/utils/UseRangesCheck.h | 14 +++++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 +++ .../checks/modernize/use-ranges.rst | 2 + .../modernize/Inputs/use-ranges/fake_std.h | 3 ++ .../checkers/modernize/use-ranges.cpp | 25 +++++++++++ 7 files changed, 121 insertions(+), 9 deletions(-) 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); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
