Skip to content

libcollection: generalize StrVector to AsSlice<Str>. #19192

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

Merged
merged 1 commit into from
Nov 24, 2014

Conversation

nodakai
Copy link
Contributor

@nodakai nodakai commented Nov 21, 2014

A single impl supports all of [T], Vec<T> and CVec<T>.

Once Iterable is implemented, we will prefer it to SlicePrelude.
But the with_capacity() part might become tricky.

@rust-highfive
Copy link
Contributor

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@nodakai nodakai changed the title libcollection: generalize StrVector to AsSlice. libcollection: generalize StrVector to SlicePrelude. Nov 21, 2014
@nodakai nodakai force-pushed the generalize-strvector branch 3 times, most recently from d78b955 to 0189535 Compare November 21, 2014 22:11
@alexcrichton
Copy link
Member

In general the *Prelude traits are not intended to be generalized over, they're purely intended to provide the ability to define inherent methods on language-defined types. Could you elaborate a little more on the motivation for this patch? You mention that [T], Vec<T> and CVec<T> all benefit, but [T] and Vec<T> should both benefit today to to the existing impl and the Deref impl on Vec. I suspect that the CVec<T> type needs to itself be revisited and redesigned, so it may not be the best type to optimize for right now.

The impl for [T] also works as impl for slices in general.
By generalizing the impl of StrVector for Vec<Str> to that for
AsSlice<Str>, it becomes much more generic.

Once Iterable is implemented, we will prefer it to AsSlice.
But the with_capacity() part might become tricky.

Signed-off-by: NODA, Kai <nodakai@gmail.com>
@nodakai nodakai force-pushed the generalize-strvector branch from 0189535 to ef3b88c Compare November 23, 2014 12:06
@nodakai nodakai changed the title libcollection: generalize StrVector to SlicePrelude. libcollection: generalize StrVector to AsSlice<Str>. Nov 23, 2014
@nodakai
Copy link
Contributor Author

nodakai commented Nov 23, 2014

@alexcrichton I eventually found a minimum change to generalize the existing impl for Vec<Str> to AsSlice<Str> was the best. User-defined types implementing AsSlice<Str> will also benefit from StrVector methods.

Granted, it's only one method call away (mycontainer.as_slice().concat().) But since this change won't break anything and is an incremental refine, I think it's worth getting into the stdlib.

bors added a commit that referenced this pull request Nov 24, 2014
A single impl supports all of `[T]`, `Vec<T>` and `CVec<T>`.

Once `Iterable` is implemented, we will prefer it to `SlicePrelude`.
But the `with_capacity()` part might become tricky.
@bors bors closed this Nov 24, 2014
@bors bors merged commit ef3b88c into rust-lang:master Nov 24, 2014
@nodakai nodakai deleted the generalize-strvector branch November 25, 2014 10:13
lnicola added a commit to lnicola/rust that referenced this pull request Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants