https://github.com/unterumarmung created https://github.com/llvm/llvm-project/pull/196038
Preserve callable results with .fun, allow structured-binding-safe rewrites, and keep diagnostics while suppressing unsafe fix-its when ranges result objects do not match the original result shape. Assisted by Codex. >From 9da5f2751385b9b9d2363a31c84435ef8d5399c9 Mon Sep 17 00:00:00 2001 From: Daniil Dudkin <[email protected]> Date: Wed, 6 May 2026 11:38:10 +0300 Subject: [PATCH] [clang-tidy] `use-ranges`: avoid unsafe result fix-its Preserve callable results with .fun, allow structured-binding-safe rewrites, and keep diagnostics while suppressing unsafe fix-its when ranges result objects do not match the original result shape. Assisted by Codex. --- .../clang-tidy/modernize/UseRangesCheck.cpp | 27 ++++++++++--- .../clang-tidy/utils/UseRangesCheck.cpp | 40 ++++++++++++++++++- .../clang-tidy/utils/UseRangesCheck.h | 2 + clang-tools-extra/docs/ReleaseNotes.rst | 7 ++++ .../checks/modernize/use-ranges.rst | 2 + .../modernize/Inputs/use-ranges/fake_std.h | 26 ++++++++++++ .../checkers/modernize/use-ranges.cpp | 37 +++++++++++++++++ 7 files changed, 134 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp index e25e444cc3819..3f9f5ce8b4dfe 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp @@ -22,7 +22,6 @@ static constexpr const char *SingleRangeNames[] = { "all_of", "any_of", "none_of", - "for_each", "find", "find_if", "find_if_not", @@ -35,7 +34,6 @@ static constexpr const char *SingleRangeNames[] = { "partition_point", "lower_bound", "upper_bound", - "equal_range", "binary_search", "push_heap", "pop_heap", @@ -47,7 +45,6 @@ static constexpr const char *SingleRangeNames[] = { "shift_left", "shift_right", "is_partitioned", - "partition_copy", "sort", "stable_sort", "is_sorted", @@ -56,7 +53,6 @@ static constexpr const char *SingleRangeNames[] = { "is_heap_until", "max_element", "min_element", - "minmax_element", "uninitialized_fill", "uninitialized_default_construct", "uninitialized_value_construct", @@ -72,8 +68,16 @@ static constexpr const char *SingleRangeOutResultNames[] = { "transform", "unique_copy", "uninitialized_copy", "uninitialized_move", }; +static constexpr const char *SingleRangeFunctionResultNames[] = {"for_each"}; + +static constexpr const char *SingleRangeStructuredBindingNames[] = { + "equal_range", "minmax_element"}; + +static constexpr const char *SingleRangeDiagnosticOnlyNames[] = { + "partition_copy"}; + static constexpr const char *TwoRangeNames[] = { - "equal", "mismatch", "includes", "lexicographical_compare", + "equal", "includes", "lexicographical_compare", "find_end", "search", "is_permutation", }; @@ -86,6 +90,8 @@ static constexpr const char *TwoRangeOutResultNames[] = { "set_union", }; +static constexpr const char *TwoRangeStructuredBindingNames[] = {"mismatch"}; + static constexpr const char *SinglePivotRangeNames[] = {"inplace_merge"}; static constexpr const char *SinglePivotRangeBeginResultNames[] = {"rotate"}; @@ -159,6 +165,12 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const { PolicyKind::AppendAccessorForUsedResult, ".begin()"}; const ResultPolicy OutResultPolicy = {PolicyKind::AppendAccessorForUsedResult, ".out"}; + const ResultPolicy FunctionResultPolicy = { + PolicyKind::AppendAccessorForUsedResult, ".fun"}; + const ResultPolicy StructuredBindingPolicy = { + PolicyKind::KeepFixItOnlyForStructuredBinding, {}}; + const ResultPolicy DiagnosticOnlyPolicy = { + PolicyKind::SuppressFixItForUsedResult, {}}; struct AlgorithmGroup { ArrayRef<Signature> Signatures; @@ -169,8 +181,13 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const { {SingleRangeFunc, SingleRangeNames, DefaultPolicy}, {SingleRangeFunc, SingleRangeBeginResultNames, BeginResultPolicy}, {SingleRangeFunc, SingleRangeOutResultNames, OutResultPolicy}, + {SingleRangeFunc, SingleRangeFunctionResultNames, FunctionResultPolicy}, + {SingleRangeFunc, SingleRangeStructuredBindingNames, + StructuredBindingPolicy}, + {SingleRangeFunc, SingleRangeDiagnosticOnlyNames, DiagnosticOnlyPolicy}, {TwoRangeFunc, TwoRangeNames, DefaultPolicy}, {TwoRangeFunc, TwoRangeOutResultNames, OutResultPolicy}, + {TwoRangeFunc, TwoRangeStructuredBindingNames, StructuredBindingPolicy}, {SinglePivotFunc, SinglePivotRangeNames, DefaultPolicy}, {SinglePivotFunc, SinglePivotRangeBeginResultNames, BeginResultPolicy}, {SinglePivotFunc, SinglePivotRangeOutResultNames, OutResultPolicy}, diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index fb46ab157ee47..e72cc94c6724e 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp @@ -36,6 +36,7 @@ using namespace clang::ast_matchers; static constexpr const char BoundCall[] = "CallExpr"; static constexpr const char FuncDecl[] = "FuncDecl"; static constexpr const char ArgName[] = "ArgName"; +static constexpr const char StructuredBindingResult[] = "StructuredBinding"; namespace clang::tidy::utils { @@ -55,6 +56,16 @@ AST_MATCHER(Expr, hasSideEffects) { } } // namespace +static auto hasStructuredBindingResult() { + return callExpr( + anyOf(hasParent(decompositionDecl()), + hasParent(exprWithCleanups(hasParent(decompositionDecl()))))); +} + +static auto hasStructuredBindingResult(StringRef ID) { + return hasStructuredBindingResult().bind(ID); +} + static auto makeExprMatcher(const ast_matchers::internal::Matcher<Expr> &ArgumentMatcher, ArrayRef<StringRef> MethodNames, @@ -156,7 +167,9 @@ void UseRangesCheck::registerMatchers(MatchFinder *Finder) { ast_matchers::internal::DynTypedMatcher::VO_AnyOf, ASTNodeKind::getFromNodeKind<CallExpr>(), std::move(TotalMatchers)) - .convertTo<CallExpr>()), + .convertTo<CallExpr>(), + anyOf(hasStructuredBindingResult(StructuredBindingResult), + unless(hasStructuredBindingResult()))), this); } } @@ -211,6 +224,23 @@ static bool isResultUsed(const CallExpr &Call, return isResultUsed(DynTypedNode::create(Call), Result); } +static bool shouldEmitFixIts(UseRangesCheck::Replacer::ResultUsePolicy Policy, + bool ResultUsed, bool IsStructuredBinding) { + using Kind = UseRangesCheck::Replacer::ResultUsePolicy::Kind; + if (!ResultUsed) + return true; + switch (Policy.PolicyKind) { + case Kind::Preserve: + case Kind::AppendAccessorForUsedResult: + return true; + case Kind::KeepFixItOnlyForStructuredBinding: + return IsStructuredBinding; + case Kind::SuppressFixItForUsedResult: + return false; + } + llvm_unreachable("Unhandled result use policy"); +} + static void insertAccessor(DiagnosticBuilder &Diag, const CallExpr &Call, StringRef Accessor, const ASTContext &Ctx) { const SourceLocation End = Lexer::getLocForEndOfToken( @@ -254,10 +284,16 @@ void UseRangesCheck::check(const MatchFinder::MatchResult &Result) { return; } + const bool IsStructuredBinding = + Result.Nodes.getNodeAs<CallExpr>(StructuredBindingResult) != nullptr; const bool ResultUsed = isResultUsed(*Call, Result); - auto ResultPolicy = Replacer->getResultUsePolicy(*Function, false); + auto ResultPolicy = + Replacer->getResultUsePolicy(*Function, IsStructuredBinding); auto Diag = createDiag(*Call); + if (!shouldEmitFixIts(ResultPolicy, ResultUsed, IsStructuredBinding)) + return; + if (auto ReplaceName = Replacer->getReplaceName(*Function)) Diag << FixItHint::CreateReplacement(Call->getCallee()->getSourceRange(), *ReplaceName); diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h index 6105ce1a1f351..54645cb5f795a 100644 --- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h +++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.h @@ -47,6 +47,8 @@ class UseRangesCheck : public ClangTidyCheck { enum class Kind { Preserve, AppendAccessorForUsedResult, + KeepFixItOnlyForStructuredBinding, + SuppressFixItForUsedResult, }; Kind PolicyKind = Kind::Preserve; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2d1eed13dd3ad..5bf3d7cce7a76 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -445,6 +445,13 @@ Changes in existing checks - Preserved used output iterator results when replacing output algorithms such as ``std::copy``. + - Preserved used callable results when replacing ``std::for_each`` and + structured binding results when replacing algorithms such as + ``std::equal_range``. + + - Kept diagnostics but suppressed unsafe fix-its when no safe + result-preserving rewrite is available. + - 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 6c2c15b0445d2..566ff0fa6fe84 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 @@ -14,6 +14,7 @@ Example auto Iter1 = std::find(Items.begin(), Items.end(), 0); auto NewEnd = std::unique(Items.begin(), Items.end()); auto Out = std::copy(Items.begin(), Items.end(), Output); + auto [First, Last] = std::equal_range(Items.begin(), Items.end(), 0); auto AreSame = std::equal(Items1.cbegin(), Items1.cend(), std::begin(Items2), std::end(Items2)); @@ -25,6 +26,7 @@ Transforms to: auto Iter1 = std::ranges::find(Items, 0); auto NewEnd = std::ranges::unique(Items).begin(); auto Out = std::ranges::copy(Items, Output).out; + auto [First, Last] = std::ranges::equal_range(Items, 0); 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 0ed44d99c35c0..6145defc3a6bb 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 @@ -5,6 +5,11 @@ namespace std { +template <class T1, class T2> struct pair { + T1 first; + T2 second; +}; + template <typename Container> constexpr auto begin(const Container &Cont) { return Cont.begin(); } @@ -108,6 +113,11 @@ 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 InputIt, class UnaryFunc> +UnaryFunc for_each(InputIt first, InputIt last, UnaryFunc f) { + return f; +} + template <class ForwardIt> ForwardIt unique(ForwardIt first, ForwardIt last); @@ -134,6 +144,22 @@ template <class BidirIt, class UnaryPred> BidirIt stable_partition(BidirIt first, BidirIt last, UnaryPred pred) { return first; } +template <class InputIt, class OutputIt1, class OutputIt2, class UnaryPred> +pair<OutputIt1, OutputIt2> partition_copy(InputIt first, InputIt last, + OutputIt1 d_first_true, + OutputIt2 d_first_false, + UnaryPred pred) { + return {d_first_true, d_first_false}; +} + +template <class ForwardIt, class T> +pair<ForwardIt, ForwardIt> equal_range(ForwardIt first, ForwardIt last, + const T &value); +template <class ForwardIt> +pair<ForwardIt, ForwardIt> minmax_element(ForwardIt first, ForwardIt last); +template <class InputIt1, class InputIt2> +pair<InputIt1, InputIt2> mismatch(InputIt1 first1, InputIt1 last1, + InputIt2 first2, InputIt2 last2); template <class ForwardIt> ForwardIt rotate(ForwardIt first, ForwardIt middle, ForwardIt last); 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 29d847e8a0653..6f8e27872e2cc 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 @@ -180,6 +180,43 @@ void Positives() { // CHECK-FIXES: auto RotateCopyOutput = // CHECK-FIXES-NEXT: std::ranges::rotate_copy(I, I.begin() + 2, J.begin()).out; + auto ForEachResult = + std::for_each(I.begin(), I.end(), [](int N) { return N == 0; }); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a ranges version of this algorithm + // CHECK-FIXES: auto ForEachResult = + // CHECK-FIXES-NEXT: std::ranges::for_each(I, [](int N) { return N == 0; }).fun; + + std::for_each(I.begin(), I.end(), [](int N) { return N == 0; }); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm + // CHECK-FIXES: std::ranges::for_each(I, [](int N) { return N == 0; }); + + auto [EqualRangeBegin, EqualRangeEnd] = + std::equal_range(I.begin(), I.end(), 0); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a ranges version of this algorithm + // CHECK-FIXES: auto [EqualRangeBegin, EqualRangeEnd] = + // CHECK-FIXES-NEXT: std::ranges::equal_range(I, 0); + + auto [MinElement, MaxElement] = std::minmax_element(I.begin(), I.end()); + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use a ranges version of this algorithm + // CHECK-FIXES: auto [MinElement, MaxElement] = std::ranges::minmax_element(I); + + auto [MismatchBegin1, MismatchBegin2] = + std::mismatch(I.begin(), I.end(), J.begin(), J.end()); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a ranges version of this algorithm + // CHECK-FIXES: auto [MismatchBegin1, MismatchBegin2] = + // CHECK-FIXES-NEXT: std::ranges::mismatch(I, J); + + auto EqualRangeResult = std::equal_range(I.begin(), I.end(), 0); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use a ranges version of this algorithm + + auto PartitionCopyResult = std::partition_copy( + I.begin(), I.end(), J.begin(), J.begin(), [](int N) { return N == 0; }); + // CHECK-MESSAGES: :[[@LINE-2]]:30: warning: use a ranges version of this algorithm + + auto [PartitionCopyTrue, PartitionCopyFalse] = std::partition_copy( + I.begin(), I.end(), J.begin(), J.begin(), [](int N) { return N == 0; }); + // CHECK-MESSAGES: :[[@LINE-2]]:50: warning: use a ranges version of this algorithm + 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); _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
