Tracking issue for `start`, `end` and `new` methods of RangeInclusive

[kennytm]

Tracking issue for the start, end and new methods of RangeInclusive.

(Originally the tracking issue for rust-lang/rfcs#1980 the start and end fields, but we decided to make the representation private in #49022 (comment))

Steps:

• Implementation Introduce RangeInclusive::{new, start, end} methods and make the fields private. #49724
• Stabilization PR Stabilise inclusive_range_methods #50758

Unresolved questions:

• Should we want to expose the fields as public (stabilize as is), or simply provide getter methods and don't show the implementation detail (redesign from scratch)?

This issue is separated from #28237 since there were vocal oppositions from the community (see #47813) due to performance loss if we commit to the two-field design.

If you want start and end, please use these for now:

• x.start() ↦ x.next().unwrap()
• x.end() ↦ x.next_back().unwrap()

[scottmcm]

Two important points here:

• There's no performance loss if you use internal iteration (fold & friends), just like other types (such as Chain) that can be faster if you use internal iteration instead of a for loop.
• There is core additional complexity to iterating RangeInclusive compared to a Range, since the loop body fundamentally must do something different in the last iteration in the interesting case

As I've said before, the optimal choice here would be to only have the range types be IntoIterator (not Iterator), but that change requires a time machine, so I think being like Range is best.

[SimonSapin]

The libs team discussed this and the consensus was to make all fields private, provide accessor methods, and add a boolean field to implement iteration in a way that hopefully optimizes better.

impl<Idx> RangeInclusive<Idx> {
    pub fn start(&self) -> &Idx {…}
    pub fn end(&self) -> &Idx {…}
}

If someone has a use case for moving bounds out, they should request something like pub fn into_inner(self) -> (Idx, Idx) to be added. The methods of the RangeBounds trait (formerly RangeArgument) are to be renamed start_bound and end_bound to avoid name collision.

@rfcbot fcp merge

[rfcbot]

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

• @Kimundi
• @SimonSapin
• @alexcrichton
• @aturon
• @dtolnay
• @sfackler
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[scottmcm]

What exact this this FCPing? It feels odd that the last two times the class was modified it was via RFC, but this time it's just a "hopefully", not even a PR.

[SimonSapin]

This is not making again deep changes to the data structure or syntax, only picking one of two already-proposed alternatives. And the more conservative one, since methods can keep working even if the internal structure changes.

[kennytm]

Simply making the fields private will break lowering of ..= 😒

error[E0451]: field `start` of struct `std::ops::RangeInclusive` is private
   --> librustc/ty/layout.rs:646:34
    |
646 |             self.valid_range == (0..=1)
    |                                  ^ field `start` is private

error[E0451]: field `end` of struct `std::ops::RangeInclusive` is private
   --> librustc/ty/layout.rs:646:38
    |
646 |             self.valid_range == (0..=1)
    |                                      ^ field `end` is private

Therefore x..=y will need to lower to something like RangeInclusive::new(x, y). Should the constructor be exposed or #[doc(hidden)]? Or we keep the fields public and perma-unstable them?

[SimonSapin]

I’d be fine with #[doc(hidden)] perma-unstable public fields, but we might want an constructor function anyway if the fields are "effectively private". Is lowering to a call difficult or inconvenient?

[kennytm]

It needs a special case in https://github.com/rust-lang/rust/blob/56714acc5eb0687ed9a7566fdebe5528657fc5b/src/librustc/hir/lowering.rs#L3082 for the ExprKind::Range(Some(..), Some(..), Closed) case. Not very difficult, just different.