ACP: TwoSidedRange trait
#365
Comments
pitaj
added
api-change-proposal
|
sorry gotta -1 this one if this is the only motivation:
I don't find this motivating enough. While the pub trait SampleRange<T> {
fn sample_single<R: RngCore + ?Sized>(self, rng: &mut R) -> T; // <- certainly out of scope for a std TwoSidedRange
fn is_empty(&self) -> bool;
}furthermore, since impl<B, T> SampleRange<T> for B
where
T: SampleUniform + PartialOrd,
B: TwoSidedRange<T>,
{
fn sample_single<R: RngCore +?Sized>(self, rng: &mut R) -> T {
let start = self.start_inclusive();
if let Some(end) = self.end_inclusive() {
T::Sampler::sample_single_inclusive(*start, *end, rng) // actually can't move out start and end
} else if let Some(end) = self.end_exclusive() {
T::Sampler::sample_single(*start, *end, rng) // ditto
} else {
panic!("the range has no inclusive nor exclusive end why oh why");
}
}
fn is_empty(&self) -> bool {
self.width() <= Some(0)
}
}The |
|
For completeness sake, I'm posting the original situation I found /// Returns a new `Vec` with `len` elements, where the elements are uniformly random in the `range`.
fn random_uniform<R: RangeBounds<i32>>(len: usize, range: R) -> Vec<i32>Given the existing trait, implementing this function felt so cumbersome I decided to go a different route in the past. I think abstracting over ranges is a reasonable thing to do. So something like Another situation where I found the |
|
I will add more examples when I get home (submitted this on my phone). If you read the comments, I called out explicitly whether impl<B, T> SampleRange<T> for B
where
T: SampleUniform + PartialOrd,
B: TwoEndedRange<T>,
{
fn sample_single<R: RngCore +?Sized>(self, rng: &mut R) -> T {
let start = self.start_inclusive();
// Exclusive end bound can always convert
// to inclusive, unless the range is empty
// But we explicitly don't support empty
let end = self.end_inclusive().expect("range was empty");
T::Sampler::sample_single_inclusive(start, end, rng)
}
fn is_empty(&self) -> bool {
RangeBounds::is_empty(self)
}
} |
|
The new sample code doesn't work because if trait TwoSidedRange<T> {
fn try_into_range(self) -> Result<Range<T>, Self>;
fn try_into_range_inclusive(self) -> Result<RangeInclusive<T>, Self>;
} |
No, you can just clone the bound. impl<T: Step> TwoSidedRange<T> for Range<T> {
// Just a copy for every `Step` type at the moment
fn start_inclusive(&self) { self.start.clone() }
...
} |
|
The original implementation of Edit: Also, (P.S. I think |
Yeah a But |
|
@kennytm unless I'm missing something, even if the proposed API extension doesn't help with rand's |
|
Indeed, the sketch solution doesn't resolve the A more general solution would be something like struct BoundIE<T> {
Included(T),
Excluded(T),
}
fn into_bounds(self) -> (T, BoundIE<T>);
// Or with pattern types
fn into_bounds(self) -> (T, Bound<T> is Bound::Included(_) | Bound::Excluded(_));Which wouldn't require |
|
@Voultapher That's why the first sentence of my comment is "gotta -1 this one (rand::SampleRange) if this is the only motivation" So far the other motivation is your DSL use case in #365 (comment). |
|
@kennytm I provided two motivating examples, |
|
@Voultapher why do you want to implement your own |
|
@kennytm while I could explain why that signature makes sense in my use-case, I don't see how that's relevant here. And looking at
And the list goes on. If most users of the API end up doing the same |
Have you actually looked into what your linked examples actually does? All of them showed counter-examples of |
|
Also, for the
|
|
@kennytm maybe we are talking about different things here. I'm observing an ergonomic hole in the stdlib, that forces users to re-implement very similar logic again and again. You seem focused on showing that the current API sketch has its issues. Can we agree that the API could be more ergonomic? If yes, then let's think about the various use-cases and figure out how they could be improved. |
|
I'd also like to add that the type size limit for |
|
I don't see In your examples (let's ignore Xline):
If there is a significant hole I'd say to expand |
|
Specifically for finite integer ranges there's actually an exact bound one can use today: |
|
|
|
I think many cases that would be helped by something like this probably just take an argument of Some examples:
The trait originally described would work for those cases, but something like this might be better. pub enum BoundIE<T> {
Included(T),
Excluded(T),
}
pub trait TwoSidedRange<T>: RangeBounds<T> {
// Should `into_bounds` just return `(T, BoundIE<T>)`
// since the start bound is always inclusive?
/// Get the bounds directly.
fn into_bounds(self) -> (BoundIE<T>, BoundIE<T>);
// `Step` implies `Clone` so technically `inclusive_bounds`
// could take `&self` instead.
/// Convert both bounds to inclusive,
/// returns `None` for an empty range.
fn inclusive_bounds(self) -> Option<(T, T)>
where
T: Step
{
match (self.start_bound(), self.end_bound()) {
(Bound::Included(start), Bound::Included(end)) => Some((start, end)),
(Bound::Included(start), Bound::Excluded(end)) => {
if start >= end {
// Empty
None
} else {
let end_inclusive = Step::backward(end, 1);
Some((start, end_inclusive))
}
}
_ => unreachable!(),
}
}
}
impl<T> TwoSidedRange<T> for Range<T> { ... }
impl<T> TwoSidedRange<T> for RangeInclusive<T> { ... } |
Generally,
|
Unfortunately, this may not be possible because of existing impls like this one: impl<T> RangeBounds<T> for Range<&T> { ... }(I was thinking about exactly that same thing earlier today) At the very least I think you would need |
People find I will also just note that the unstable |
|
Going back to my original use-case of |
This could be hacked by constraining on the trait RangeBounds<T: ?Sized> {
// Hack(?) to make `.into_bounds()` work without requiring `T: Clone`.
type Interior
where
Self: Sized; // can omit this `where` clause if we don't care about object-safety.
fn into_bounds(self) -> (Bound<T>, Bound<T>)
where
Self: Sized,
T: Sized,
Self::Interior: Into<T>;
}(Because
In that case we should be improving
OneSidedRange was introduced in rust-lang/rust#69780 which was introduced to support rust-lang/rust#62280 i.e. slice::take. Unlike As explained in the tracking issue of
I think |
Conventionally Imagine that type inference were improved so we could use any integer type to index into slices. We would still expect |
|
Another solution for the pub trait RangeIntoBounds<T> {
fn into_bounds(self) -> (Bound<T>, Bound<T>);
}
// impl for each range type where T: Sized
pub trait RangeBounds<T: ?Sized> {
fn into_bounds(self) -> (Bound<T>, Bound<T>)
where
Self: RangeIntoBounds,
{
RangeIntoBounds::into_bounds(self)
}
}Though it's hard to argue in that case that it should be on |
|
Actually I think the associated type solution you propose would be a breaking change - |
Yes indeed it is 🥲. And there are quite many third-party crates implementing Technically the |
|
Instead of impl<T: Sized> From<Range<T>> for (Bound<T>, Bound<T>) { ... }
// etc for all other range typesThen fn into_bounds(self) -> (Bound<T>, Bound<T>)
where
T: Sized,
Self: Sized + Into<(Bound<T>, Bound<T>)>,
{
self.into()
}You could just have the conversions, but that would be more convenient. |
|
So what I agree with kennytm that |
This strikes at a core topic I think. I had assumed that would go to the end of the number range. E.g. for i in 5u8.. {
println!("{i}");
}Until testing it, I had assumed this would print
Have to say, I don't share your expectations here. To me that code makes no sense, and I don't expect it should. |
|
The inconsistencies keep going, take this example: for i in ..5u8 {
println!("{i}");
}Here the compiler complains that it doesn't want to guess where you want to start. A sensible answer, but then why is the inverse, guessing where I want to stop with |
|
Having now encountered this schism in boundedness, I agree with @jdahlstrom that conceptualizing them as finite and infinite ranges makes more sense. |
First of all, it's not guessing where to stop I think that // Returns an "infinite" iterator
// a, a+1, a+2, ...
(a..).iter()
// Returns an "infinite" reversed iterator
// b-1, b-2, b-3, ...
(..b).rev()
// Returns an "infinite" reversed iterator
// b, b-1, b-2, ...
(..=b).rev()I intend for the new range types to resolve this inconsistency. |
that would break the pretty popular pattern |
Minor nitpick, in my eyes it performs a guess. It could have guessed go until infinity, go until the end of the number range, etc. I disagree that infinity is the only logical answer here and thus it's not a guess. Infinity can be a logical answer though. That said what use-cases are there where infinity that can't be represented and leads to implementation-defined behavior is useful? Another source of inconsistency is https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.take which is given a |
optimization i guess
err how are you going to consume a |
easy: using compiler optimizations: this runs a |
Of the top of my head I can't think of meaningful optimization that would change if the range is assumed to end at the numeric limits of the number. |
Certainly a cool optimization showcase, took me a bit to figure out the inner loop is an accumulate. But this code would be quite impractical for debug builds, it only sensibly works in release mode. Who would want to use such code? |
|
another way we can iterate a full |
In release mode you can reliably get rid of the end bounds check. Though I assume most compilers is able to recognize |
Proposal
Problem statement
RangeBoundsexists as a useful generic abstraction that lets users write functions that take any range type. However, is not uncommon that users want to only accept ranges with both start and end bounds (RangeandRangeInclusive). If they choose to useRangeBoundsfor this, they have to resort to runtime panics. Or they have to write their own trait only implemented for the given ranges.Motivating examples or use cases
The
randcrate has aSampleRangetrait implemented only forRangeandRangeInclusive.If
TwoSidedRangeexisted, they could use that instead.TODO: Other examples
Solution sketch
Alternatives
The design of width (airways returning usize) is not ideal, we could instead return the next larger unsigned integer or something instead, but that would require a special trait.
The functions could live on rangebounds instead for better visibility.
Could instead have two traits:
StartBoundRangeandEndBoundRangeand thenTwoSidedRangewould be expressed withStartBoundRange + EndBoundRangebut providingfn widthwould be tough.widthis similar toExactSizeIterator::lenbut implemented fallibly, whereas.len()can silently return incorrect results on platforms whereusizeis 16 bits.Links and related work
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):
Second, if there's a concrete solution:
The text was updated successfully, but these errors were encountered: