Implement SortedStarts and SortedDisjoint for std::iter::* types by mineichen · Pull Request #27 · CarlKCarlK/range-set-blaze

mineichen · 2026-03-12T11:41:16Z

Implement SortedStarts and SortedDisjoint for std::iter::* types, which cannot break the invariants of SortedStarts or SortedDisjoint... This enables users of the library to use these operators, as demonstrated in the test.

If you think this is nice, I'll implement this for sorted_disjoint_map too before the merge
To my surprise, StepBy doesn't implement FusedIterator (I opened a discussion here) if it could be added)
--Would you be willing to drop the FusedIterator requirement of SortedStarts (breaking), if std::ops::Fused itself implements SortedStarts, if it's inner Iterator does?

…ch cannot break the invariants of SortedStarts or SortedDisjoint

CarlKCarlK · 2026-03-12T15:16:57Z

Would you be willing to drop the FusedIterator requirement of SortedStarts (breaking), if std::ops::Fused itself implements SortedStarts, if it's inner Iterator does?

Yes, Please add a TODO comment in the code that references https://internals.rust-lang.org/t/implement-fusediterator-for-core-stepby/24074
so that sometime we could put it back. (Or open an issue, I guess :-) )

Is the PR ready to review?

mineichen · 2026-03-12T16:17:50Z

If you want this in your lib, I'd suggest the following workflow:

I'll add Implementations for SortedStartsMap and SortedDisjointMap now with a Implementation for Fused
(StepBy has to be added later) and merge this (non-breaking). If you hit thumbs up on this msg, I'll start right away).

To Remove the FusedIterator, it would be good, if someone with deep knowledge could do it in a separate PR, because all .next() calls have to be reviewed. Do you have time to do this ?

Add a Implementation for StepBy, if FusedIterator is no longer required...

It is not ready for review yet, just to show you code to have a discussion about.

…StartsMap, if the inner iter implements it

Copilot

Pull request overview

This PR extends the crate’s marker traits (SortedStarts/SortedDisjoint and map equivalents) to cover common core::iter adapter types, enabling users to compose standard iterator adapters while retaining access to the crate’s optimized set/map operations.

Changes:

Implement SortedStarts/SortedDisjoint for several core::iter adapters (Filter, TakeWhile, SkipWhile, Fuse, Skip, Take, Peekable) plus Empty/Once.
Implement SortedStartsMap/SortedDisjointMap for the corresponding core::iter adapter types.
Add compile-time coverage tests demonstrating union() over chained standard iterator adapters.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
`src/sorted_disjoint.rs`	Adds marker-trait impls for `core::iter` adapters over `RangeInclusive<T>` and adds a small test.
`src/sorted_disjoint_map.rs`	Adds marker-trait impls for `core::iter` adapters over `(RangeInclusive<T>, VR)` and adds a small test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-12T19:38:21Z

+impl<T: Integer, I, P> SortedDisjoint<T> for core::iter::SkipWhile<I, P>
+where
+    I: SortedStarts<T>,
+    P: FnMut(&I::Item) -> bool,
+{


Same issue as TakeWhile: this SortedDisjoint impl only requires I: SortedStarts<T>. skip_while preserves disjointness only if the input is already disjoint, so the bound should be I: SortedDisjoint<T> to avoid falsely marking overlapping iterators as disjoint.

Copilot · 2026-03-12T19:38:21Z

+impl<T, VR, I, P> SortedDisjointMap<T, VR> for core::iter::TakeWhile<I, P>
+where
+    T: Integer,
+    VR: ValueRef,
+    I: SortedStartsMap<T, VR>,


SortedDisjointMap promises the underlying (range, value) pairs are disjoint. This impl currently only requires I: SortedStartsMap<T, VR>, which would allow overlapping ranges to be treated as SortedDisjointMap after take_while. Tighten the bound to I: SortedDisjointMap<T, VR> to preserve the trait contract.

Copilot · 2026-03-12T19:38:21Z

+impl<T, VR, I, P> SortedDisjointMap<T, VR> for core::iter::SkipWhile<I, P>
+where
+    T: Integer,
+    VR: ValueRef,
+    I: SortedStartsMap<T, VR>,


This SortedDisjointMap impl for SkipWhile only requires I: SortedStartsMap<T, VR>, but skip_while preserves disjointness only when the input is already disjoint. Require I: SortedDisjointMap<T, VR> here to avoid violating SortedDisjointMap’s invariants.

Copilot · 2026-03-12T19:38:22Z

+// Check if core::iter::StepBy implements FusedIterator if it's inner is. Seems like it could be upstreamed
+// https://internals.rust-lang.org/t/implement-fusediterator-for-core-stepby/24074
+// impl<T: Integer, I> SortedStarts<T> for core::iter::Fuse<core::iter::StepBy<I>> where
+//     I: SortedStarts<T>
+// {
+// }
+// impl<T: Integer, I> SortedDisjoint<T> for core::iter::Fuse<core::iter::StepBy<I>> where
+//     I: SortedDisjoint<T>
+// {
+// }
+


The commented-out StepBy impls and the inline link to an upstream internals thread are dead code that will likely rot over time. Consider removing this block from the library source, or converting it into a concise TODO that points to a tracked repository issue (so it’s discoverable and has an action item).

Suggested change

// Check if core::iter::StepBy implements FusedIterator if it's inner is. Seems like it could be upstreamed

// https://internals.rust-lang.org/t/implement-fusediterator-for-core-stepby/24074

// impl<T: Integer, I> SortedStarts<T> for core::iter::Fuse<core::iter::StepBy<I>> where

// I: SortedStarts<T>

// {

// }

// impl<T: Integer, I> SortedDisjoint<T> for core::iter::Fuse<core::iter::StepBy<I>> where

// I: SortedDisjoint<T>

// {

// }

Copilot · 2026-03-12T19:38:22Z

+
+impl<T: Integer, I, P> SortedDisjoint<T> for core::iter::TakeWhile<I, P>
+where
+    I: SortedStarts<T>,


SortedDisjoint is a stronger contract than SortedStarts (it promises disjointness). This impl currently only requires I: SortedStarts<T>, which would allow a non-disjoint iterator to be treated as SortedDisjoint after take_while, violating the trait’s guarantees. Change the bound to require I: SortedDisjoint<T> (and keep the predicate bound as-is).

Suggested change

I: SortedStarts<T>,

I: SortedDisjoint<T>,

CarlKCarlK · 2026-03-12T19:49:56Z

Please look at Copilot’s review (it did a good job, I think).
SortedDisjoint for TakeWhile/SkipWhile should require I: SortedDisjoint, and map versions should require I: SortedDisjointMap (not SortedStarts*), otherwise overlapping inputs can be mislabeled disjoint.

For StepBy, let’s leave it out for now. Please add a TODO plus a tracking issue for revisiting StepBy/FusedIterator later.

…> into a SortedDisjoint using FlatMap and Flatten

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    Self: FusedIterator + Iterator<Item = (RangeInclusive<T>, VR)>,
+    I: SortedStartsMap<T, VR>,
+    TMap: FnMut(I) -> IInner,
+{
+}
+
+impl<T, VR, I> SortedStartsMap<T, VR> for core::iter::Flatten<core::option::IntoIter<I>>
+where
+    T: Integer,
+    VR: ValueRef,
+    Self: FusedIterator + Iterator<Item = (RangeInclusive<T>, VR)>,
+    I: SortedStartsMap<T, VR>,
+{
+}
+
+impl<T, VR, I> SortedDisjointMap<T, VR> for core::iter::Flatten<core::option::IntoIter<I>>
+where
+    T: Integer,
+    VR: ValueRef,
+    Self: FusedIterator + Iterator<Item = (RangeInclusive<T>, VR)>,
+    I: SortedDisjointMap<T, VR>,
+{


+impl<T, I, IInner, TMap> SortedStarts<T>
+    for core::iter::FlatMap<core::option::IntoIter<I>, IInner, TMap>
+where
+    T: Integer,
+    IInner: SortedStarts<T>,
+    Self: FusedIterator + Iterator<Item = RangeInclusive<T>>,
+    I: SortedStarts<T>,
+    TMap: FnMut(I) -> IInner,
+{


+    T: Integer,
+    IInner: SortedDisjoint<T>,
+    Self: FusedIterator + Iterator<Item = RangeInclusive<T>>,
+    I: SortedStarts<T>,


+impl<T, I> SortedStarts<T> for core::iter::Flatten<core::option::IntoIter<I>>
+where
+    T: Integer,
+    Self: FusedIterator + Iterator<Item = RangeInclusive<T>>,
+    I: SortedStarts<T>,
+{
+}
+
+impl<T, I> SortedDisjoint<T> for core::iter::Flatten<core::option::IntoIter<I>>
+where
+    T: Integer,
+    Self: FusedIterator + Iterator<Item = RangeInclusive<T>>,
+    I: SortedDisjoint<T>,
+{
+}


+// Check if core::iter::StepBy implements FusedIterator if it's inner is. Seems like it could be upstreamed
+// https://internals.rust-lang.org/t/implement-fusediterator-for-core-stepby/24074
+// impl<T: Integer, I> SortedStarts<T> for core::iter::Fuse<core::iter::StepBy<I>> where
+//     I: SortedStarts<T>
+// {
+// }
+// impl<T: Integer, I> SortedDisjoint<T> for core::iter::Fuse<core::iter::StepBy<I>> where
+//     I: SortedDisjoint<T>
+// {
+// }


+{
+}
+
+// Check if core::iter::StepBy implements FusedIterator if it's inner is. Seems like it could be upstreamed


+    I: SortedStartsMap<T, VR>,
+    TMap: FnMut(I) -> IInner,
+{
+}
+


+    VR: ValueRef,
+    IInner: SortedDisjointMap<T, VR>,
+    Self: FusedIterator + Iterator<Item = (RangeInclusive<T>, VR)>,
+    I: SortedStartsMap<T, VR>,


CarlKCarlK · 2026-03-15T15:22:17Z

Please look at Copilot’s latest suggestions. They seem right to me.

mineichen · 2026-03-15T16:45:17Z

Sorry for the sloppy changes... I think i got things right now... Copilot suggested to Remove the code in one block, and in the other not... I kept it to allow implementing it in the future... Seems like StepBy could soon be added... :-)

…ify iterator support

CarlKCarlK · 2026-03-15T21:06:03Z

Look good. I've added documentation. I'll include this in the next release.

mineichen force-pushed the std_iterators branch from 9944ed1 to 7f91633 Compare March 12, 2026 12:43

Implement SortedStarts and SortedDisjoint for std::iter::* types, whi…

d6bd680

…ch cannot break the invariants of SortedStarts or SortedDisjoint

mineichen force-pushed the std_iterators branch from 7f91633 to d6bd680 Compare March 12, 2026 13:12

Allow some core::iterators to be used as SortedDisjointMap and Sorted…

f5ca327

…StartsMap, if the inner iter implements it

CarlKCarlK requested a review from Copilot March 12, 2026 19:34

Copilot started reviewing on behalf of CarlKCarlK March 12, 2026 19:35 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

Fix trait bounds and add a combinator to turn a Option<SortedDisjoint…

426a158

…> into a SortedDisjoint using FlatMap and Flatten

mineichen force-pushed the std_iterators branch from baea53b to 426a158 Compare March 14, 2026 10:46

CarlKCarlK requested a review from Copilot March 15, 2026 15:06

Copilot started reviewing on behalf of CarlKCarlK March 15, 2026 15:06 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

Apply suggested changes

b335b89

Update documentation for SortedDisjoint and SortedDisjointMap to clar…

7c014e7

…ify iterator support

CarlKCarlK self-assigned this Mar 15, 2026

CarlKCarlK added the planned label Mar 15, 2026

Conversation

mineichen commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CarlKCarlK commented Mar 12, 2026

Uh oh!

mineichen commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

CarlKCarlK commented Mar 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

CarlKCarlK commented Mar 15, 2026

Uh oh!

mineichen commented Mar 15, 2026

Uh oh!

CarlKCarlK commented Mar 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mineichen commented Mar 12, 2026 •

edited

Loading