Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unfortunate type bound on Range #10414

Closed
emberian opened this issue Nov 11, 2013 · 7 comments
Closed

Unfortunate type bound on Range #10414

emberian opened this issue Nov 11, 2013 · 7 comments
Labels
A-traits Area: Trait system E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot.

Comments

@emberian
Copy link
Member

Range has a type bound on ToPrimitive so that it can have a size_hint. It does not need this to function, and the size_hint is just an optional nicety.

It'd be nice if there were some way to express this with the type system; this "optional" type bound makes the interface less generic (though it's not that big of a deal).

@nikomatsakis
Copy link
Contributor

On Mon, Nov 11, 2013 at 02:02:45AM -0800, Corey Richardson wrote:

Range has a type bound on ToPrimitive so that it can have a size_hint. It does not need this to function, and the size_hint is just an optional nicety.

It'd be nice if there were some way to express this with the type system; this "optional" type bound makes the interface less generic (though it's not that big of a deal).

The only way I can think to do this would be to add "negative" bounds,
so that you could have

struct Range<T> { from: T, to: T }
impl<T:ToPrimitive> Iterator<T> for Range<T> { ... }
impl<T:-ToPrimitive> Iterator<T> for Range<T> { ... }

(Or whatever trait this is) I've always been kind of against this
because nobody else does it but the truth is that thanks to coherence
we could do it, I think, and there are occasional uses cropping up
here and there (like this one).

@lilyball
Copy link
Contributor

There are other circumstances in which negative bounds would be nice. In this particular case, it's a shame that you'd need to have identical implementations of next() in both impls, only differing in size_hint(). It would be really nice if there was some way to partially specialize the implementation, such that if you have

impl<T: Ord> Foo for Bar<T> { ... }

then you could say

impl<T: Ord+ToPrimitive> Foo for Bar<T> { ... }

and selectively re-implement only the methods that you care to do; anything not implemented would use the "parent"'s implementation.

Naturally, this falls down if you then say

impl<T: Ord+Quux> Foo for Bar<T> { ... }

and some type implements both ToPrimitive and Quux. But surely there must be some alternative way to solve this. Perhaps some way to simply test within the implementation block if T implements ToPrimitive and only implement size_hint() if it does.

For example:

impl<T> Iterator<T> for Range<T> {
    fn next() -> Option<T> { ... }
    if!<T: ToPrimitive> {
        fn size_hint() -> (uint, Option<uint>) { ... }
    }
}

Or maybe something like this:

impl<T> Iterator<T> for Range<T> {
    fn next() -> Option<T> { ... }
    fn<R: T+ToPrimitive> size_hint() -> (uint, Option<uint>) { ... }
    // in this case, size_hint() must either have a default impl, or a second impl using negative trait bounds must be provided
}

@sfackler
Copy link
Member

You can avoid most of the duplication by just having next delegate to some private method.

@vks
Copy link
Contributor

vks commented Nov 8, 2014

size_hint is currently only correct for integers, see #17010. I think we should use Int instead of ToPrimitive, because the current implementation is wrong for floats because of rounding.

Do we really need range for non-integers?

@Gankra
Copy link
Contributor

Gankra commented Nov 8, 2014

I believe the plan is to support range on anything that implements some kind of enumerable trait. Although rust-lang/rfcs/pull/439 introduces some generalizations to slicing that we might want to favour instead.

@steveklabnik
Copy link
Member

Ranges have changed a bunch, and I can't seem to find an implementation of ToPrimitive on anything but primitives anymore: http://doc.rust-lang.org/nightly/std/num/trait.ToPrimitive.html

Is this issue still relevant?

@Gankra
Copy link
Contributor

Gankra commented Jan 20, 2015

The (old) iter::Range is on its way out, so I think this is fine to kill.

@Gankra Gankra closed this as completed Jan 20, 2015
flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 6, 2023
add large future lint

Closes rust-lang#5263

---

changelog: new lint: [`large_futures`]
[rust-lang#10414](rust-lang/rust-clippy#10414)
<!-- changelog_checked -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot.
Projects
None yet
Development

No branches or pull requests

7 participants