-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Have collections impl Extend<&T> where T: copy #839
Merged
alexcrichton
merged 6 commits into
rust-lang:master
from
Gankra:embrace-extend-and-extinguish
Jun 2, 2015
Merged
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
3641ade
extend upgroid
Gankra c539412
Scare quotes make me feel good
Gankra 5843e5c
Update 0000-embrace-extend-extinguish.md
Gankra 05ef23c
Add Copy alternative.
Gankra 8e7f04f
Clone -> Copy and address discussion
Gankra 6874dab
fixup
Gankra File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
- Feature Name: embrace-extend-extinguish | ||
- Start Date: 2015-02-13 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
|
||
Extend the Extend trait to take IntoIterator, and make all collections | ||
`impl<'a, T: Clone> Extend<&'a T>`. This enables both `vec.extend(&[1, 2, 3])`, and | ||
`vec.extend(&hash_set)`. This provides a more expressive replacement for | ||
`Vec::push_all` with literally no ergonomic loss, while leveraging established APIs. | ||
|
||
# Motivation | ||
|
||
Vec::push_all is kinda random and specific. Partially motivated by performance concerns, | ||
but largely just "nice" to not have to do something like | ||
`vec.extend([1, 2, 3].iter().cloned())`. The performance argument falls flat | ||
(we *must* make iterators fast, and trusted_len should get us there). The ergonomics | ||
argument is salient, though. Working with Plain Old Data types in Rust is super annoying | ||
because generic APIs and semantics are tailored for non-Copy types. | ||
|
||
Even with Extend upgraded to take IntoIterator, that won't work with &[Copy], | ||
because a slice can't be moved out of. Collections would have to take `IntoIterator<&T>`, | ||
and clone out of the reference. So, do exactly that. | ||
|
||
As a bonus, this is more expressive than `push_all`, because you can feed in any | ||
collection by-reference to clone the data out of it. | ||
|
||
# Detailed design | ||
|
||
Here's a quick hack to get this working today: | ||
|
||
``` | ||
/// A type growable from an `Iterator` implementation | ||
pub trait Extend<T> { | ||
fn extend<It: Iterator<Item=T>, I: IntoIterator<IntoIter=It>> | ||
(&mut self, iterator: I); | ||
} | ||
``` | ||
|
||
This isn't the signature we'd like longterm, but it's what works with today's | ||
IntoIterator and where clauses. Longterm (like, tomorrow) this should work: | ||
|
||
``` | ||
/// A type growable from an `Iterator` implementation | ||
pub trait Extend<T> { | ||
fn extend<I: IntoIterator<Item=T>>(&mut self, iterator: I); | ||
} | ||
``` | ||
|
||
And here's usage: | ||
|
||
``` | ||
use std::iter::IntoIterator; | ||
|
||
impl<'a, T: Clone> Extend<&'a T> for Vec<T> { | ||
fn extend<It: Iterator<Item=&'a T>, I: IntoIterator<IntoIter=It>> | ||
(&mut self, iterator: I){ | ||
self.extend(iterator.into_iter().cloned()) | ||
} | ||
} | ||
|
||
|
||
fn main() { | ||
let mut foo = vec![1]; | ||
foo.extend(&[1, 2, 3, 4]); | ||
let bar = vec![1, 2, 3]; | ||
foo.extend(&bar); | ||
foo.extend(bar.iter()); | ||
|
||
println!("{:?}", foo); | ||
} | ||
``` | ||
|
||
# Drawbacks | ||
|
||
Mo' generics, mo' magic. How you gonna discover it? | ||
|
||
Hidden clones? | ||
|
||
# Alternatives | ||
|
||
Nope. | ||
|
||
# Unresolved questions | ||
|
||
FromIterator could also be extended in the same manner, but this is less useuful for | ||
two reasons: | ||
|
||
* FromIterator is always called by calling `collect`, and IntoIterator doesn't really | ||
"work" right in `self` position. | ||
* Introduces ambiguities in some cases. What is `let foo: Vec<_> = [1, 2, 3].iter().collect()`? | ||
|
||
Of course, context might disambiguate in many cases, and | ||
`let foo: Vec<i32> = [1, 2, 3].iter().collect()` might still be nicer than | ||
`let foo: Vec<_> = [1, 2, 3].iter().cloned().collect()`. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I'd be fine just dropping this definition entirely from the RFC, I think the definition below should work shortly as well.