-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
libcore: DST-ify AsSlice, Equiv, and ops traits #18638
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
Conversation
|
@aturon These |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Last time I looked at this code, I had the impression that this method could be changed to normalize(&[u8]) (no need for generics), but I didn't look further)
|
From my part, r+. These changes look fine to me. But @japaric is presumably right about the other impls. |
|
I pushed an update to add I've looked for generic code over these traits for further places to add @japaric Want to take a look as well and make sure I didn't miss something? |
|
LGTM |
src/libcollections/slice.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This impl breaks concat_vec for &[Vec<T>] (breaking tests below).
|
OK, this turns out to be somewhat subtle, because sometimes generic code really needs to work with impl<'a, T, Sized? U: AsSlice<T>> AsSlice<T> for &'a U {
#[inline(always)]
fn as_slice<'a>(&'a self) -> &'a [T] { self.as_slice() }
}
impl<'a, T, Sized? U: AsSlice<T>> AsSlice<T> for &'a mut U {
#[inline(always)]
fn as_slice<'a>(&'a self) -> &'a [T] { self.as_slice() }
}but this is currently not working due to, I think, some weakness in method resolution that @nikomatsakis is fixing. Will investigate. |
|
FWIW, I usually write such methods as: impl<'a, T, Sized? U: AsSlice<T>> AsSlice<T> for &'a U {
#[inline(always)]
fn as_slice<'a>(&'a self) -> &'a [T] { AsSlice::as_slice(*self) }
}Otherwise, I end up hitting infinite recursion errors. |
|
(In particular, adding these impls causes the |
|
@japaric Ah yes, of course, thanks. |
|
ping @aturon, needs a rebase |
|
@alexcrichton This is blocked on @nikomatsakis's method dispatch changes landing. |
|
@aturon unblocked now, I guess? |
|
I'm going to unassign myself and assign to @aturon since it just needs rebasing and tender love and care at this point. |
This commit changes `AsSlice` to work on unsized types, and changes the
`impl` for `&[T]` to `[T]`. Aside from making the trait more general,
this also helps some ongoing work with method resolution changes.
This is a breaking change: code that uses generics bounded by `AsSlice`
will have to change. In particular, such code previously often took
arguments of type `V` where `V: AsSlice<T>` by value. These should now
be taken by reference:
```rust
fn foo<Sized? V: AsSlice<T>>(v: &V) { .. }
```
A few std lib functions have been changed accordingly.
[breaking-change]
This commit relaxes constraints on generics and traits within the `core::ops` module and for the `Equiv` trait.
This PR changes `AsSlice` to work on unsized types, and changes the
`impl` for `&[T]` to `[T]`. Aside from making the trait more general,
this also helps some ongoing work with method resolution changes.
This is a breaking change: code that uses generics bounded by `AsSlice`
will have to change. In particular, such code previously often took
arguments of type `V` where `V: AsSlice<T>` by value. These should now
be taken by reference:
```rust
fn foo<Sized? V: AsSlice<T>>(v: &V) { .. }
```
A few std lib functions have been changed accordingly.
The PR also relaxes constraints on generics and traits within the
`core::ops` module and for the `Equiv` trait.
[breaking-change]
r? @nikomatsakis
cc @japaric
|
Hmm, I think this means we can remove the horrible "auto-ref-ref" hack in the method resolution code. |
This PR changes
AsSliceto work on unsized types, and changes theimplfor&[T]to[T]. Aside from making the trait more general,this also helps some ongoing work with method resolution changes.
This is a breaking change: code that uses generics bounded by
AsSlicewill have to change. In particular, such code previously often took
arguments of type
VwhereV: AsSlice<T>by value. These should nowbe taken by reference:
A few std lib functions have been changed accordingly.
The PR also relaxes constraints on generics and traits within the
core::opsmodule and for theEquivtrait.[breaking-change]
r? @nikomatsakis
cc @japaric