`RangeBound::into_bounds` · Issue #538 · rust-lang/libs-team (original) (raw)

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

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, Bound); } impl IntoBounds for Range impl IntoBounds for RangeFrom impl IntoBounds for RangeFull impl IntoBounds for RangeInclusive impl IntoBounds for RangeTo impl IntoBounds for RangeToInclusive

trait RangeBounds { // ... existing API

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

}

Alternatives

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, Bound); }

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

Use Into instead of a new trait

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

trait RangeBounds { // ... 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.

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.

RangeBound::into_bounds · Issue #538 · rust-lang/libs-team (original) (raw)