Make range_step and range_inclusive adaptors of Range


#1

Code changes:

  • range_inclusive(a, b) => (a .. b).inclusive()
  • range_step(a, b, c) => (a .. b).step(c)
  • range_step_inclusive(a, b, c) => (a .. b).inclusive().step(c)

This unifies our “range” syntax as well as continues our trend of “composition over combinations”. I started poking at a proper implementation/RFC for this change but got caught up in details of the precise/traits/bounds these would require, and other fiddly details. So I’m making a quick issue on this topic just to get the idea out there for more people to poke at.

Some sketches of ideas:

// basic trait we need for `.step()`. Takes Amount as an input because 
// it's not clear if there's a "right" choice for Amount, and if it should 
// be unique. e.g. one could support StepBy i16 and u16 on a i16
// (i16 is more natural to StepBy, but insufficient to span its own range).
// Mandating only usize seems otherwise wrong.
trait StepBy<Amount> : Step {
   // equivalent to calling `step` `amount` times. Returns Err 
   // if some boundary condition like overflow occurs.
   fn step_by(&mut self, amount: &Amount) -> Result<(), ()>; 
   // equivalent to calling `step_back` `amount` times. Returns Err 
   // if some boundary condition like underflow occurs.
   fn step_back_by(&mut self, amount: &Amount) -> Result<(), ()>;
  
   // steps_between_by???
}
  • Should step().inclusive() and inclusive().step() work?
  • Can/should Inclusive be written as a wrapper that accepts either Range or RangeStep generically and does the right thing? Thus avoiding the need to create a RangeStepInclusive type?
  • Should .step(x) permit x to be negative?
  • Should that then “work” for start > stop? Or should such behaviour be relegated to "use rev"?
  • If we allow backwards ranges without rev then you need some way to check for x being “negative”.
  • If we don’t allow it, at what level do we enforce it? Unspecified behaviour? Still require a notion of negative just to check?
  • Should inclusive be encouraged for “other” range uses (e.g. overloaded array indexing) or “only” for iteration? (Note, I believe inclusive needs an extra boolean flag to remember if it has yielded stop yet).

[Pre-RFC] Change the Range struct to support inclusive ranges
#2

Thanks for running with these ideas, @Gankro!

Another question is: we could conceivably support ... for inclusive ranges, since we already use that notation with that meaning in pattern matching. (This came up in a recent thread that I can’t find right now). Should we do that instead of .inclusive?


#3

http://discuss.rust-lang.org/t/inconsistent-syntax-for-normal-range-and-match-range/1294

This would solve everything but the .step()

well there would be differences between .rev().step(x) and .step(x).rev() if x is not a divisor of the range length. so .step(-x) would probably be .rev().step(x), but might cause more confusion than it’s worth.


#4

What is the behavior of this?

let mut x = 0_i32..1_i32;
let _ = x.next();
x.inclusive();
println!("{:?}", x.next());

Should it be None or Some(1)?


#5

I assume inclusive/step would take their ranges by value so the output would be None (and the compiler would warn that the output of x.inclusive() is unused).


#6

I assume that inclusive() and step() would return new iterators, not modify an existing iterator in-place. So the answer should be that the x.inclusive(); statement would have no effect, and this program will print None.


#7

I like this a lot. The trend toward composition is very welcome! It requires re-learning a few things, but it becomes natural quite quickly.


#8

@stebalien @sivadeilra I got it slightly wrong. Let me try it again.

What is the behavior of this?

let mut x = 0_i32..1_i32;
let _ = x.next();
let mut y = x.inclusive();
println!("{:?}", y.next());

It is not clear whether it should print None or Some(1)


#9

@theme the fields of range are public, so the effects of calling next and next_back are well-observable: next moves start forward, next_back moves stop backward. So calling next will make the range 1…1 which should work through inclusive.


#10

How does .inclusive() work with (u8::MIN, u8::MAX)? I think you need more fields than start and end for an inclusive range.


#11

if you look at the old range_inclusive implementation, you can see that there was an additional boolean: http://doc.rust-lang.org/src/core/iter.rs.html#2555-2587


#12

Yes, as I mention in the original post, I believe this boolean is necessary to distinguish these two states in inclusive:

(x, x) // x not yielded
(x, x) // x yielded

However this is not a problem in the transition from exclusive to inclusive (we can safely assume the first state, always).


#13

A step operator would be nice also. Here, % could be considered similar to modulus, though I’m not proposing this. It looks really weird even if it could work. range_step(a, b, c) => a..b%c


#14

Currently, the fields of range are not public.

Additionally, I am not sure how to implement .inclusive() exactly correctly without a performance hit. I believe that even a slight performance hit on range (say, 1 additional instruction per iteration) would have a measurable effect due to the fact that some programs spend most of their time looping in ranges.

Examples of hard-to-handle code:

  • This should output None:
let mut x = 0_u8..1_u8;
let _ = x.next();
let _ = x.next();
let mut y = x.inclusive();
println!("{:?}", y.next());
  • This should also output None:
let mut x = 0_u8..1_u8;
for _ in 0_i32..256_i32 {
    let _ = x.next();
}
let mut y = x.inclusive();
println!("{:?}", y.next());

#15

They are public: http://doc.rust-lang.org/std/ops/struct.Range.html

This is necessary for using Range in indexing to do subslicing.


#16

Wouldn’t it make more sense to prevent calling these after the range starts iterating? We can have two range types like Range<Initial> and Range<Started> and implement .inclusive() and .step() only for Range<Initial>. HTTP library Hyper does this to prevent changing HTTP headers after response has been sent.


#17

If you wanted to do that, you’d probably want Range and RangeIter. where Range::iter(self) returns a RangeIter (transform methods such as inclusive() and step() would be defined on Range). However, to make this ergonomic, we’d probably want IntoIterator first.


#18

It should be noted that @aturon is changing the way Range iteration works. I think Step is dying altogether, and you just need to impl Iterator for Range<MyType>.