Change the Range struct to support inclusive ranges

[Stebalien]

rendered

[liigo]

What's the range syntax of this exclusive bound?

[Stebalien]

Regardless of how the range syntax will eventually work, the underlying data structure will need to support both inclusive and exclusive ranges.

This RFC doesn't address syntax.

[Diggsey]

So many problems with this:

1. All code dealing with ranges gets 4x longer (you can't just reuse the same code because of overflow differences between inclusive/exclusive ranges)
2. Ranges become up to twice as large in memory as they are now
3. Inclusiveness of ranges is tracked at runtime rather than compile-time. This is pointless overhead as the programmer will always know at compile-time which one is desired.
4. Range syntax necessarily becomes more complicated

Regardless of how the range syntax will eventually work, the underlying data structure will need to support both inclusive and exclusive ranges.

That doesn't follow at all. Just use a different data structure when you absolutely need an inclusive range. However, this is uncommon as the inclusive lower-bound/exclusive upper-bound combination used by range is far more useful.

[liigo]

Is there any precedent in other language that has exclusive range for RangeFrom? Maybe it's too over-designed. It makes the language unnecessarily complicated, as @Diggsey said.

[Stebalien]

1. All code dealing with ranges gets 4x longer (you can't just reuse the same code because of overflow differences between inclusive/exclusive ranges)

From the RFC:

We might want some way to extract inclusive bounds from any integral range to make bounds checking easier.

2. Ranges become up to twice as large in memory as they are now

This is a valid concern but I don't see users storing a large number of ranges.

3. Inclusiveness of ranges is tracked at runtime rather than compile-time. This is pointless overhead as the programmer will always know at compile-time which one is desired.

Are you suggesting encoding inclusiveness in the types? This would require 9 range types and four different functions just to deal with normal ranges.

4. Range syntax necessarily becomes more complicated

Again, this RFC does not address syntax. Furthermore, the current range syntax simply can't express all ranges which is why adding support for inclusive ranges has been discussed extensively on the internals forum.

Regardless of how the range syntax will eventually work, the underlying data structure will need to support both inclusive and exclusive ranges.
That doesn't follow at all. Just use a different data structure when you absolutely need an inclusive range. However, this is uncommon as the inclusive lower-bound/exclusive upper-bound combination used by range is far more useful.

I wrote this RFC to address the stability hazard here. If we don't provide support for all combinations of inclusive/exclusive ranges in the Range data structure itself, libraries that need them will have to use their own ranges and will be unable to migrate to a standard range if/when rust adds syntax for new inclusive/exclusive range variants.

[Stebalien]

@liigo

Is there any precedent in other language that has exclusive range for RangeFrom? Maybe it's too over-designed. It makes the language unnecessarily complicated, as @Diggsey said.

Both the rand crate and the BTreeMap need an exclusive RangeFrom to be fully expressive. While one can always substitute a half open integral range for an inclusive integral range, this isn't the case for non integral ranges. For example, (0, 1] (0-1 but not zero) or ("", ..) (all strings but the empty string).

However, no, I don't know of any languages that have special syntax for these other range variants. Furthermore, I'm not proposing that rust introduce special syntax for them. I just don't want APIs to either (a) stick with custom range implementations because the standard one isn't expressive enough or (b) provide a method that accepts standard ranges and another that deals with the cases that the standard range syntax can't.

[Diggsey]

From the RFC:

We might want some way to extract inclusive bounds from any integral range to make bounds checking easier.

From my comment:

you can't just reuse the same code because of overflow differences between inclusive/exclusive ranges

It's not valid to convert between integral ranges like that because inclusive/exclusive ranges overflow at different places. In addition, inclusive ranges cannot store an empty range.

This is a valid concern but I don't see users storing a large number of ranges.

On what basis? Rustc actually does something like this, albeit with its own range type rather than the built in one: rust-lang/rust#15594
It seems quite likely to me that there will be users will be storing large numbers of ranges given how they form a core part of the language.

Are you suggesting encoding inclusiveness in the types? This would require 9 range types and four different functions just to deal with normal ranges.

You're the one proposing 9 different variations of range and 4x as much code to deal with them. That holds regardless of whether you make the distinction at compile-time or runtime.

Again, this RFC does not address syntax. Furthermore, the current range syntax simply can't express all ranges which is why adding support for inclusive ranges has been discussed extensively on the internals forum.

This RFC introduces a new problem of syntax which didn't previously exist. The fact that it doesn't make any attempt to address that is not a defense.

I wrote this RFC to address the stability hazard here. If we don't provide support for all combinations of inclusive/exclusive ranges in the Range data structure itself, libraries that need them will have to use their own ranges and will be unable to migrate to a standard range if/when rust adds syntax for new inclusive/exclusive range variants.

I don't see how that's a stability hazard? It's perfectly fine for rust to introduce new features later, even if libraries have already implemented their own versions. That's how it should be in fact - common features being pulled up into the standard library as their usage becomes universal.

[Stebalien]

It's not valid to convert between integral ranges like that because inclusive/exclusive ranges overflow at different places. In addition, inclusive ranges cannot store an empty range.

You could by returning Option<(Idx, Idx)> where None means that the range is empty. However, after trying to code this, I realized that any conversions would require taking the ordering into account which makes the conversion complicated:

trait InclusiveInterval<Idx> {
    fn interval(&self) -> Option<(Idx, Idx)>;
}

// You'd have to implement for all integers independently (macros...) instead of implementing for
// generic Int...
impl<I: Int> InclusiveInterval<I> for Range<I> {
    fn interval(&self) -> Option<(I, I)> {
        match self.start {
            Inclusive(s) => match self.end {
                Inclusive(e) => Some((s, e)),
                Exclusive(e) => match s.cmp(&e) {
                    Ordering::Equal => None,
                    Ordering::Less => Some((s, e-Int::one())),
                    Ordering::Greater => Some((s, e+Int::one())),
                },
            },
            Exclusive(s) => match self.end {
                Exclusive(e) => match s.cmp(&e) {
                    Ordering::Equal => None,
                    Ordering::Less => if s + Int::one() == e { None } else { Some((s + Int::one(), e - Int::one())) },
                    Ordering::Greater => if s - Int::one() == e { None } else { Some((s - Int::one(), e + Int::one())) }
                },
                Inclusive(e) => match s.cmp(&e) {
                    Ordering::Equal => None,
                    Ordering::Less => Some((s+Int::one(), e)),
                    Ordering::Greater => Some((s-Int::one(), e)),
                }
            }
        }
    }
}

impl<I: Int> InclusiveInterval<I> for RangeFrom<I> {
    fn interval(&self) -> Option<(I, I)> {
        match self.start {
            Inclusive(s) => Some((s, Int::max_value())),
            Exclusive(s) => s.checked_add(Int::one()).map(|s| (s, Int::max_value())),
        }
    }
}

impl<I: Int> InclusiveInterval<I> for RangeTo<I> {
    fn interval(&self) -> Option<(I, I)> {
        match self.end {
            Inclusive(e) => Some((Int::min_value(), e)),
            Exclusive(e) => e.checked_sub(Int::one()).map(|e| (Int::min_value(), e)),
        }
    }
}

impl<I: Int> InclusiveInterval<I> for RangeFull {
    fn interval(&self) -> Option<(I, I)> {
        Some((Int::min_value(), Int::max_value()))
    }
}

On what basis? Rustc actually does something like this, albeit with its own range type rather than the built in one: rust-lang/rust#15594
It seems quite likely to me that there will be users will be storing large numbers of ranges given how they form a core part of the language.

From what I've seen, Range is usually used in APIs and only APIs.

You're the one proposing 9 different variations of range and 4x as much code to deal with them. That holds regardless of whether you make the distinction at compile-time or runtime.

My goal was to reduce API complexity; I don't want API writers to have to implement 9 different range accepting functions. However, an alternative way to do this would be to have 9 different range structs and then define a Range trait that converts a range struct to a pair of bounds as follows:

pub enum Bound<Idx> {
    Inclusive(Idx),
    Exclusive(Idx),
    Unbounded,
}
pub trait Range<Idx> {
    fn into_bounds(self) -> (Bound<Idx>, Bound<Idx>);
    fn bounds(&self) -> (Bound<&Idx>, Bound<&Idx>);
}

pub struct ExclusiveRange<Idx> {
    pub start: Idx,
    pub end: Idx,
}

pub struct LeftExclusiveRange<Idx> {
    pub start: Idx,
    pub end: Idx,
}

pub struct RightExclusiveRange<Idx> {
    pub start: Idx,
    pub end: Idx,
}

pub struct InclusiveRange<Idx> {
    pub start: Idx,
    pub end: Idx,
}

pub struct ExclusiveRangeTo<Idx> {
    pub end: Idx,
}

pub struct InclusiveRangeTo<Idx> {
    pub end: Idx,
}

pub struct ExclusiveRangeFrom<Idx> {
    pub start: Idx,
}

pub struct InclusiveRangeFrom<Idx> {
    pub start: Idx,
}
pub struct RangeFull;

impl<Idx> Range<Idx> for ExclusiveRange<Idx> {
    fn into_bounds(self) -> (Bound<Idx>, Bound<Idx>) {
        (Bound::Exclusive(self.start), Bound::Exclusive(self.end))
    }
    fn bounds(&self) -> (Bound<&Idx>, Bound<&Idx>) {
        (Bound::Exclusive(&self.start), Bound::Exclusive(&self.end))
    }
}

impl<Idx> Range<Idx> for LeftExclusiveRange<Idx> {
    fn into_bounds(self) -> (Bound<Idx>, Bound<Idx>) {
        (Bound::Exclusive(self.start), Bound::Inclusive(self.end))
    }
    fn bounds(&self) -> (Bound<&Idx>, Bound<&Idx>) {
        (Bound::Exclusive(&self.start), Bound::Inclusive(&self.end))
    }
}

impl<Idx> Range<Idx> for RightExclusiveRange<Idx> {
    fn into_bounds(self) -> (Bound<Idx>, Bound<Idx>) {
        (Bound::Inclusive(self.start), Bound::Exclusive(self.end))
    }
    fn bounds(&self) -> (Bound<&Idx>, Bound<&Idx>) {
        (Bound::Inclusive(&self.start), Bound::Exclusive(&self.end))
    }
}

impl<Idx> Range<Idx> for InclusiveRange<Idx> {
    fn into_bounds(self) -> (Bound<Idx>, Bound<Idx>) {
        (Bound::Inclusive(self.start), Bound::Inclusive(self.end))
    }
    fn bounds(&self) -> (Bound<&Idx>, Bound<&Idx>) {
        (Bound::Inclusive(&self.start), Bound::Inclusive(&self.end))
    }
}

impl<Idx> Range<Idx> for ExclusiveRangeFrom<Idx> {
    fn into_bounds(self) -> (Bound<Idx>, Bound<Idx>) {
        (Bound::Exclusive(self.start), Bound::Unbounded)
    }
    fn bounds(&self) -> (Bound<&Idx>, Bound<&Idx>) {
        (Bound::Exclusive(&self.start), Bound::Unbounded)
    }
}

impl<Idx> Range<Idx> for InclusiveRangeFrom<Idx> {
    fn into_bounds(self) -> (Bound<Idx>, Bound<Idx>) {
        (Bound::Inclusive(self.start), Bound::Unbounded)
    }
    fn bounds(&self) -> (Bound<&Idx>, Bound<&Idx>) {
        (Bound::Inclusive(&self.start), Bound::Unbounded)
    }
}

impl<Idx> Range<Idx> for ExclusiveRangeTo<Idx> {
    fn into_bounds(self) -> (Bound<Idx>, Bound<Idx>) {
        (Bound::Unbounded, Bound::Exclusive(self.end))
    }
    fn bounds(&self) -> (Bound<&Idx>, Bound<&Idx>) {
        (Bound::Unbounded, Bound::Exclusive(&self.end))
    }
}

impl<Idx> Range<Idx> for InclusiveRangeTo<Idx> {
    fn into_bounds(self) -> (Bound<Idx>, Bound<Idx>) {
        (Bound::Unbounded, Bound::Inclusive(self.end))
    }
    fn bounds(&self) -> (Bound<&Idx>, Bound<&Idx>) {
        (Bound::Unbounded, Bound::Inclusive(&self.end))
    }
}

impl<Idx> Range<Idx> for RangeFull {
    fn into_bounds(self) -> (Bound<Idx>, Bound<Idx>) {
        (Bound::Unbounded, Bound::Unbounded)
    }
    fn bounds(&self) -> (Bound<&Idx>, Bound<&Idx>) {
        (Bound::Unbounded, Bound::Unbounded)
    }
}

This RFC introduces a new problem of syntax which didn't previously exist. The fact that it doesn't make any attempt to address that is not a defense.

Please read the relevant discussion http://internals.rust-lang.org/t/vs-for-inclusive-ranges/1539. Several of the core developers have expressed an interest in supporting inclusive ranges however, no one could agree on a syntax.

is not a defense

This isn't a trial.

I don't see how that's a stability hazard? It's perfectly fine for rust to introduce new features later, even if libraries have already implemented their own versions. That's how it should be in fact - common features being pulled up into the standard library as their usage becomes universal.

1. Other libraries will stabilize on their custom range structs.
2. Others will depend on rust's current Range struct not changing.
3. Rust's standard library will stabilize on ad-hoc custom ranges.

Syntax can always be introduced later. However, we can't rename the Range struct to something like RightExclusiveRange or change its fields after 1.0.

I opened this because I wasn't getting any objections on the internals forum. However, as this obviously needs to be discussed more, I'll move the discussion back there (http://internals.rust-lang.org/t/pre-rfc-change-the-range-struct-to-support-inclusive-ranges/1683).