Pre-RFC: Fixing Range by 2027

I believe that there is some guidance that cargo fix (or a similar tool) is able to point out what needs looked at. Pointing out every literal and Range constructor is not feasible IMO.

5 Likes

FWIW, I believe that in that context "span" generally refers to, or is associated with, a concrete section of some data, while the current Range is much closer to a mathematical interval without intrinsic association. Although I guess you could always say that it's a span of numbers on the number line (or a generalization thereof).

I'd probably vouch for Interval, especially if it came with some useful mathematical operations like the lattice ops, map, and others.

9 Likes

Having public fields and a bunch of unrelated methods and trait impls on an iterator type is not conventional. And we may want to add an exhausted field to the RangeFrom iterator.

So I think it's best not to use the existing types as the iterators for the new types.

This will actually be a pretty small change IMO. Most use cases will just work without changes. For more complicated ones, we can emit a specialized error.

There's also the fact that RangeFull already doesn't implement Iterator or IntoIterator, so it would be best to just reuse that type. If we rename the rest, we'd have to copy this one as well, instead of just reexporting it.

1 Like

Another name option is Bound. But I'm still planning on keeping the name Range, since having the name of the syntax match the type name is important, and changing the syntax name would not be wise at this point.

The piece to me that most says it shouldn't is analogy with indexing.

x[10..=0] (And especially things like off-by-one things like x[1..0]) not returning something is important, and it can't return the slice in the opposite order (since it needs to be the same type as x[0..1]).

So if x[1..0] doesn't return anything, I think 1..0 should also be empty.

I really like the approach of using newtypes to opt-in to reverse, where appropriate. Here's a sketch of doing that even for slices: rev_slice - Rust

(And there are perf costs to checking ranges for working in both directions that I think would be unacceptable in Rust -- I'd much rather people make their own CanGoBothWaysRange than FasterRange, like how String is the basic thing so other things can be built atop it cleanly.)

9 Likes

It seems rev may not even be the most common use case here. Specifically, .collect and .for_each are somewhat widely in use based on a rough search grep.app | code search.

It would be interesting to have an extension trait that provides Iterator methods as a shortcut for .into_iter().method().

trait IntoIteratorExt: IntoIterator {
    fn map(self, f: FnMut(...)) -> Map<Self::IntoIter> { self.into_iter().map() }
    fn count(self) -> usize { self.into_iter().count() }
    fn collect<B: FromIterator<...>>(self) -> B { self.into_iter().collect() }
    // ...
}

I don't think this is a good fit for std, but it does make it nicer to create "iterator builders" that don't really do much until they turn into an iterator - which is what Range will become with this change.

I wonder if some special-case suggestions for those could be better. The search results find a bunch of (0..n).collect::<Vec<_>>(), and with new editions, I think Vec::from_iter(0..n) is a nicer way to write that anyway. And that takes IntoIterator, so would work fine with a future e2024::Range: !Iterator too.

Similarly, when I see (0..10).map(|_| … I feel like Range is only being used because it's short, not because it's a particularly good expression of the intent, so I wonder if we could give something better for that. There are other phrasings today, like repeat_with(|| …).take(10), but I guess they have longer names than ...

8 Likes

I've also noticed that a lot of these are in tests, rather than application code. Tests are important but may not have the same standards for quality and such.

Oh, to clarify, I don't think that they're at all wrong for having written what they did -- especially if it was before 2021 -- just that I think that a smart migration lint that had a couple of these cases could make the migration feel less messy than the obvious one that fixed things like this by adding .into_iter()s.

I see in lang team notes on the project that they may be in favor of @Diggsey's comment from here:

Presumably, with the necessary compiler support, we could backwards-compatibly add IntoIterator implementations for the Range types which return RangeIter wrappers that are !Copy, and then add a compiler warning whenever the Iterator implementation of Range is actually used.

Once a warning exists, implementing Copy for Range would not be such a footgun.

This could likely be done as soon as an RFC is accepted (or prototyped sooner), minimizing the change at ed2024 to only a impl !Iterator + Copy swap.

The concern listed here should probably be addressed in the RFC: Impl Copy for Range · Issue #2848 · rust-lang/rfcs · GitHub. I think the example is only expressing that usage across different-edition crates will be inconsistent, which seems tolerable. The exact semantics will be interesting.

Bad idea. Consider 1..=n. It would have n elements except that for n == 0 it would have 2 elements.

8 Likes

One way to make decreasing ranges work would be adding step_size to the new Range, similarly to how range works in Python. The current lo..hi syntax would imply the step size of 1 and a decreasing range would have to be constructed explicitly with a constructor or possibly with a future syntactic extension like lo..step..hi.

Though the step size being part of the range excludes those PartialOrd impls that are not Step so it probably is not a good idea.

Also, Range covering a contiguous span just feels more natural to me. Similarly, decreasing range is just an increasing range traversed backwards so your proposal of representing it by newtype makes sense to me. Or if the standalone existence existence of those different alternative modes of traversal is not needed, we can just continue using the StepBy and Rev iterators. Though, I admit that whether a decreasing and an increasing range are seen as just a different traversal of the same thing or separate objects is largely domain-specific.

7 Likes

Both decreasing and strided ranges would have to be their own types, since slice indexes can only reasonably be increasing, contiguous ranges. (A strided slice abstraction would be nifty but it, also, would have to be its own type.)

Presumably, with the necessary compiler support, we could backwards-compatibly add IntoIterator implementations for the Range types which return RangeIter wrappers that are !Copy, and then add a compiler warning whenever the Iterator implementation of Range is actually used.

I don't think this is possible without specialization because it would conflict with the blanket impl

impl<I> IntoIterator for I
where
    I: Iterator,

And it would even involve specializing the associated type which may never be possible.

I guess one other option would be to exclude Range from that blanket impl by adding an auto trait and a negative impl for Range. Not sure if adding another auto trait is really something we want to do just for this. Also it would have to be a special auto trait which doesn't apply to containing types (that may be avoided using a wrapper struct, but it's unclear if that will be supported by negative impls).

A LinSpace type would be great, I agree. I just don't think it should be called Range.

One huge advantage of making new not-an-iterator types that are Copy is that they become reasonable to put in data structures. Today, you should never store a RangeInclusive in a struct because it has the extra flag, and thus people store (T, T) instead.

So the new types, call them Interval and IntervalInclusive should both just be 2-field types with public fields. No extra overhead. Not for iterator optimizations, not for supporting an optional step, etc.

More features should be wrappers or different types.

EDIT to clarify because of the response below: I object only to it being the Range. If it wants to be LinRange, that'd be fine too. I just feel very strongly that the main type -- be that named Range or Interval or whatever -- is just the half-open two-field version, nothing more.

10 Likes

Also agree that it would be great to have something with a step - but I think that would be separate from the proposed changes here. Anyone interested enough could probably file an ACP (issue template at Issues · rust-lang/libs-team · GitHub, link it here if you do).

Linspace is matlab and numpy, but Julia does use LinRange :slight_smile: (not saying that's an ideal name).

One thing inspired by https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/.60.280.2E.2En.29.2Emap.28.7C_.7C.20.E2.80.A6.29.60.20alternatives/near/405553671.

We could lean into the "interval" idea more here and take advantage of it not being an iterator to provide a bunch of useful methods that return different things that don't fit so well on Iterator.

For example, RangeInclusive<f32> isn't something that should ever be stored in another type, really, because of the extra bool. But if we had an IntervalInclusive<f32>, then we could put the linspace-like methods on that, to have (0.0 ..= 10.0).fenceposts(3) to get the 0.0, 5.0, 10.0 iterator, or whatever.

4 Likes

We could also have something like .with_increment(2.0) return an iterator with a given step size.

1 Like

I think that if we were to add an Interval type, it should have a different PartialOrd implementation than Range has though, specifically Interval order - Wikipedia as discussed in Should `partial_cmp` on `Range` implement interval order? · Issue #54421 · rust-lang/rust · GitHub