RangeBound::into_bounds

[pitaj]

Proposal

Problem statement

The RangeBounds trait is typically used to make APIs generic over the various Range types. However, RangeBounds only allows the user to access the bounds by reference, not by value. This means that getting an owned value requires cloning, which can be expensive (BigInt for example), if it's even possible.

It is also not very ergonomic to get each bound separately and then clone it, which commonly results in the following verbose pattern:

(range.start_bound().cloned(), range.end_bound().cloned())

Motivating examples or use cases

• oxc

+ let (start, end) = range.into_bounds();
+ CommentsRange::new(comments, start, end)
- CommentsRange::new(comments, range.start_bound().cloned(), range.end_bound().cloned())

• gstreamer-rs

+ let (start, end) = range.into_bounds();
  Dump {
      data: self,
+     start,
+     end,
-     start: range.start_bound().cloned(),
-     end: range.end_bound().cloned(),
  }

• readyset

- self.add_op(Operation::AddRange((
-     range.start_bound().cloned(),
-     range.end_bound().cloned(),
- )))
+ self.add_op(Operation::AddRange(range.into_bounds()))

• artichoke

- let start = bounds.start_bound().cloned();
- let end = bounds.end_bound().cloned();
+ let (start, end) = bounds.into_bounds();

• nakamoto

- let from = range.start_bound().cloned();
- let to = range.end_bound().cloned();
+ let (from, to) = range.into_bounds();

• fontations

- let bounds = (range.start_bound().cloned(), range.end_bound().cloned());
- self.bytes.get(bounds).map(|bytes| FontData { bytes })
+ self.bytes.get(range.into_bounds()).map(|bytes| FontData { bytes })

Endless more in this Github code search

Solution sketch

RangeBounds is an unsealed trait, so this must be added as a provided method. The only way to add the provided method without T: Clone is for it to have a limiting bound.

trait IntoBounds<T: Sized> {
    fn into_bounds(self) -> (Bound<T>, Bound<T>);
}
impl<T> IntoBounds<T> for Range<T>
impl<T> IntoBounds<T> for RangeFrom<T>
impl<T> IntoBounds<T> for RangeFull
impl<T> IntoBounds<T> for RangeInclusive<T>
impl<T> IntoBounds<T> for RangeTo<T>
impl<T> IntoBounds<T> for RangeToInclusive<T>

trait RangeBounds<T> {
    // ... existing API

    fn into_bounds(self) -> (Bound<T>, Bound<T>)
    where
        Self: Sized + IntoBounds<T>,
        T: Sized,
    {
        IntoBounds::into_bounds(self)
    }
}

Alternatives

1. The above brings up an important question: if we're adding the new trait, why have the method on RangeBounds at all?

trait IntoBounds<T: Sized> {
    fn into_bounds(self) -> (Bound<T>, Bound<T>);
}

RangeBounds is already commonly imported for these purposes, so is significantly more discoverable. People expect something like this to be on RangeBounds.

2. Use Into instead of a new trait

impl<T> From<Range<T>> for (Bound<T>, Bound<T>)
impl<T> From<RangeFrom<T>> for (Bound<T>, Bound<T>)
impl<T> From<RangeFull> for (Bound<T>, Bound<T>)
impl<T> From<RangeInclusive<T>> for (Bound<T>, Bound<T>)
impl<T> From<RangeTo<T>> for (Bound<T>, Bound<T>)
impl<T> From<RangeToInclusive<T>> for (Bound<T>, Bound<T>)

trait RangeBounds<T> {
    // ... existing API

    fn into_bounds(self) -> (Bound<T>, Bound<T>)
    where
        Self: Into<(Bound<T>, Bound<T>)>,
        T: Sized,
    {
        self.into()
    }
}

This would work. But adding conversion impls like this could conflict with the new range type initiative, since currently .into() can only ever convert between new and old range types.

Links and related work

• Discussion in TwoSidedRange ACP comments

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

• We think this problem seems worth solving, and the standard library might be the right place to solve it.
• We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

• We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
• We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

[joshtriplett]

We talked about this in today's @rust-lang/libs-api meeting.

We were generally in favor of alternative 1, adding IntoBounds.

Also, IntoBounds should depend on RangeBound, to simplify writing trait bounds.