Make range_step and range_inclusive adaptors of Range

[Gankro]

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).

[aturon]

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?

[oli-obk]

This came up in a recent thread that I can’t find right now

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

This would solve everything but the .step()

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?

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.

[theme]

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)?

[stebalien]

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).

[sivadeilra]

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.

[sivadeilra]

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.

[theme]

@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)

[Gankro]

@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.

[tbu]

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

[oli-obk]

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

[Gankro]

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).

[mdinger]

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

[theme]

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());

[Gankro]

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

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

[phaux]

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.

[stebalien]

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.

[Gankro]

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>.

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.