prefer ranges algorithms over iterator-pair std calls (library + tests) - #244
Merged
Merged
Conversation
Convert the full-range std::iota(begin, end, v) calls to ranges::iota(range, v). This favors the ranges algorithms already used throughout the library and removes an all(...) macro expansion on main. Partial-range calls (e.g. iterator sub-ranges) and std::partial_sum are left as-is: partial_sum has no ranges equivalent, and partial ranges read more clearly with explicit iterators.
Convert the remaining full-range std algorithm calls to their ranges:: equivalents: - virtual_tree: unique(all(subset)) -> begin(ranges::unique(subset)) - edge_cd: partition(all(g[u]), ...) -> begin(ranges::partition(g[u], ...)) ranges::unique / ranges::partition return a subrange, so begin(...) yields the iterator the erase / offset logic needs. Partial-range calls (iterator sub-ranges) and std::partial_sum are still left as-is: partial_sum has no ranges equivalent, and partial ranges read more clearly with explicit iterators.
Apply the same "prefer ranges" convention to the test suite: - iota(all(x), v) -> ranges::iota(x, v) - unique(all(x)) -> begin(ranges::unique(x)) (erase / == end idiom) - remove_if(all(x), pred) -> begin(ranges::remove_if(x, pred)) (erase idiom) - next_permutation(all(x)) -> ranges::next_permutation(x).found accumulate(all(x), ...) is left as-is: there is no ranges::accumulate in C++23 (ranges::fold_left would change the idiom), matching how partial_sum is left in the library.
…ranges-algorithms # Conflicts: # .verify-helper/timestamps.remote.json
cameroncuster
added a commit
that referenced
this pull request
Jul 19, 2026
cppcheck flags mixing iterators from two different expressions when the erase-remove idiom is written as erase(begin(ranges::unique(x)), end(x)): the first iterator comes from the returned subrange while the second comes from the container. Store the returned subrange in a local and erase [begin, end) of that single subrange, which is semantically identical (the subrange spans the trailing removed elements) and keeps both iterators from one expression. Applies to ranges::unique and ranges::remove_if call sites introduced in the ranges-algorithm migration (#244): - library/trees/extra_members/virtual_tree.hpp - tests .../bit_ordered_set.test.cpp - tests .../kth_smallest_pst.test.cpp - tests .../mode_query.test.cpp - tests .../sa_sort_pairs.test.cpp - tests .../single_matching_bs.test.cpp
cameroncuster
added a commit
that referenced
this pull request
Jul 19, 2026
Keep the terse erase-remove one-liners from the ranges migration (#244) and silence cppcheck's mismatchingContainerExpression false positive instead. The warning fires because a range algorithm returns a subrange, so the first iterator's source expression (ranges::unique(x)) differs syntactically from the second (end(x)) even though end(subrange) is end(x) by construction. Reverts the earlier local-variable rewrite and adds line-anchored suppressions for the six affected call sites.
cameroncuster
added a commit
that referenced
this pull request
Jul 19, 2026
* fix pre-existing dev clang-format violations Reformat 5 files that violated the dev clang-format config (ColumnLimit 59) so the grep_clangformat_cppcheck CI job passes. Line-wrapping only; no behavior change. - library/dsu/kruskal_tree.hpp - library/strings/longest_common_subsequence/lcs_dp.hpp - library/strings/manacher/manacher.hpp - tests/.../data_structures/bit_ordered_set.test.cpp - tests/.../strings/sa_sort_pairs.test.cpp * [auto-verifier] verify commit 35b1fb2 * sync stale cppcheck suppression line numbers The line-anchored knownConditionTrueFalse suppressions had drifted out of sync with their source lines, causing both unmatchedSuppression and now-unsuppressed errors in the cppcheck CI step: - kruskal_tree.hpp: 13 -> 15 (shifted by the clang-format reflow in the previous commit) - suffix_array.hpp: 62 -> 63 (pre-existing drift) - suffix_array_short.hpp: 34 -> 35 (pre-existing drift) * fix cppcheck mismatchingContainerExpression on ranges erase-remove cppcheck flags mixing iterators from two different expressions when the erase-remove idiom is written as erase(begin(ranges::unique(x)), end(x)): the first iterator comes from the returned subrange while the second comes from the container. Store the returned subrange in a local and erase [begin, end) of that single subrange, which is semantically identical (the subrange spans the trailing removed elements) and keeps both iterators from one expression. Applies to ranges::unique and ranges::remove_if call sites introduced in the ranges-algorithm migration (#244): - library/trees/extra_members/virtual_tree.hpp - tests .../bit_ordered_set.test.cpp - tests .../kth_smallest_pst.test.cpp - tests .../mode_query.test.cpp - tests .../sa_sort_pairs.test.cpp - tests .../single_matching_bs.test.cpp * [auto-verifier] verify commit 0152415 * suppress cppcheck mismatchingContainerExpression instead of rewriting Keep the terse erase-remove one-liners from the ranges migration (#244) and silence cppcheck's mismatchingContainerExpression false positive instead. The warning fires because a range algorithm returns a subrange, so the first iterator's source expression (ranges::unique(x)) differs syntactically from the second (end(x)) even though end(subrange) is end(x) by construction. Reverts the earlier local-variable rewrite and adds line-anchored suppressions for the six affected call sites. --------- Co-authored-by: GitHub <noreply@github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Prefer the
ranges::algorithms over their iterator-pairstd::counterparts for full-range calls, across both the library and the
test suite. The rule: use
ranges::even whenall()would work, unlessiterators specifically make more sense.
Conversions applied:
iota(all(x), v)→ranges::iota(x, v)unique(all(x))→begin(ranges::unique(x))(erase /== endidioms)partition(all(x), pred)→begin(ranges::partition(x, pred))remove_if(all(x), pred)→begin(ranges::remove_if(x, pred))(erase)next_permutation(all(x))→ranges::next_permutation(x).foundranges::unique/ranges::partition/ranges::remove_ifreturn asubrange, so
begin(...)yields the iterator the erase / offset logicneeds;
ranges::next_permutationreturns a struct whose.founddrivesthe
do/while.Why
The consistency signal is the goal, not raw source length. #221
("standardize on c++20: ranges/views where shorter") used a
shorter-in-this-repo heuristic and, on that basis, reverted the
subrange-returning algos (
unique/remove_if/partition) back toiterator form. This PR intentionally revisits that for one reason:
all(x)macro expands onmain. When KACTL macros areexpanded for the published notebook,
all(x)becomesbegin(x), end(x), so the iterator formunique(begin(x), end(x))isactually longer than
begin(ranges::unique(x)). The"shorter" heuristic that motivated the standardize on c++20: ranges/views and <bit> used consistently #221 revert inverts once the
macro is gone — so the
ranges::form is the one that stays clean inthe shipped code.
ranges::algorithms extensively (e.g.ranges::sorteverywhere);these conversions close the remaining full-range gaps rather than
leaving a mix of idioms.
Scope / what is intentionally NOT changed
std::partial_sumandstd::accumulate— noranges::numeric-reduction equivalents in C++23 (
ranges::fold_leftwould belonger and change the idiom:
accumulate(all(v), 0)→fold_left(v, 0, plus{})), so these stay as-is.lower_bound(begin(sa) + lo, ...),find_if(rank + begin(mat), ...),partition(el, er, ...),remove_if(idx + begin(upd_st), ...)) — readmore clearly with explicit iterators.
find/count— notstd::algorithms.std::min/std::max.Files
library/ (12 sites, 12 files): line_tree, mst,
offline_incremental_scc, complement_graph_ccs, calc_sieve,
calc_linear_sieve, lcs_dp, longest_palindrome_query, suffix_array (2),
suffix_array_short (iota); virtual_tree (unique); edge_cd (partition).
tests/ (10 files): bit_ordered_set, kth_smallest_pst, mode_query,
sa_sort_pairs, seg_tree_find (unique); mono_st_asserts,
offline_incremental_scc, sa_sort_pairs (iota); permutation_tree_small,
rmq_small_n (iota + next_permutation); single_matching_bs (remove_if).
Testing
Syntax-checked all affected library-checker/aizu tests locally with
clang++ -std=c++2b -fsyntax-only— all pass.Behaviorally ran (exit 0, all asserts pass):
edge_cd_small_trees— exhaustive over all trees on 2–7 nodes(
ranges::partition).lca_all_methods_aizu— includescompress_tree_asserts, comparingcompress_treeagainst KACTL's reference over random subsets plus theempty-subset edge case (
ranges::unique).permutation_tree_smallandrmq_small_n— exhaustive over allpermutations of size 1–8 (
ranges::next_permutation(...).found,ranges::iota).