-
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
RFC for adding an Iterable
trait family
#17
Closed
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
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,155 @@ | ||
- Start Date: 2014-03-20 | ||
- RFC PR #: (leave this empty) | ||
- Rust Issue #: (leave this empty) | ||
|
||
# Summary | ||
|
||
Add an `Iterable` family of traits that will allow a function to consume both | ||
an iterator or a value that can be converted into an iterator. | ||
|
||
# Motivation | ||
|
||
We have some unnecessary redundancy in some of our collection APIs: | ||
|
||
* `Vec<T>::push_all` is designed to take one `Vec<T>` and merge it into another. | ||
* `Extendable::extend` takes an Iterator and merges it into a value. | ||
* `vec::append` copies and merges one `Vec<T>` with anther `Vec<T>`. | ||
|
||
# Detailed design | ||
|
||
This redundancy can be eliminated with a simple trait: | ||
|
||
```rust | ||
/// Any type that implements `Iterable<T, I>` can be iterated over with the | ||
/// iterator `I` to yield values of type `T`. Because of Rusts move semantics, | ||
/// this means that `self` needs to be taken by value, which for moving types | ||
/// means turning `self` into a iterator. | ||
trait Iterable<T, I: Iterator<T>> { | ||
fn into_iter(self) -> I; | ||
|
||
fn map<'r, B>(self, f: 'r |A| -> B) -> iter::Map<'r, A, B, I> { | ||
self.into_iter().map(f) | ||
} | ||
|
||
fn collect<B: iter::FromIterator<A>>(self) -> B { | ||
iter::FromIterator::from_iterator(&mut self.into_iter()) | ||
} | ||
|
||
// ... all old Iterator adapters that take `self` and make sense | ||
// to provide on a data structure directly | ||
} | ||
``` | ||
|
||
This trait is implementable by both values and iterators. Values would opt-in | ||
to implementing `Iterable`, where we would move all `.move_iter()`-style | ||
methods to impls of `Iterable`: | ||
|
||
```rust | ||
use std::vec_ng::{Vec, Items, MutItems, MoveItems}; | ||
|
||
impl<T> Iterable<T, MoveItems<T>> for Vec<T> { | ||
fn into_iter(self) -> MoveItems<T> { | ||
self.move_iter() | ||
} | ||
} | ||
``` | ||
|
||
Iterators would now be required to implement `Iterable`: | ||
|
||
```rust | ||
trait Iterator<A>: Iterable<A, Self> { | ||
... | ||
} | ||
|
||
impl<T> Iteratable<T, MoveItems<T>> for MoveItems<T> { | ||
fn into_iter(self) -> MoveItems<T> { self } | ||
} | ||
``` | ||
|
||
Since every Iterator will have the same implementation so this could be a good | ||
use case for a macro. | ||
|
||
Additionally, we would add two similar traits to handle returning references: | ||
|
||
```rust | ||
trait RefIterable<'a, A, I: iter::Iterator<A>> { | ||
fn refs(&'a self) -> I; | ||
} | ||
|
||
trait MutIterable<'a, A, I: iter::Iterator<A>> { | ||
fn mut_refs(&'a mut self) -> I; | ||
} | ||
``` | ||
|
||
We also could support more optimal iterators for collections of `Pod`s. This | ||
allows users to replaces calls of `x.refs().map(|x| *x)` with `x.values()`: | ||
|
||
```rust | ||
/// Automatically implemented for all `RefIterable<&A>` with `A: Pod` | ||
trait PodIterable<'a, A: Pod, I: iter::Iterator<A>> { | ||
fn values(&'a self) -> Values<I>; | ||
} | ||
|
||
// Automatically implement for all `RefIterable<&A>` with `A: Pod` | ||
impl<'a, T: Pod, RefIter: iter::Iterator<&'a T>, Iter: RefIterable<'a, &'a T, RefIter>> | ||
PodIterable<'a, &'a T, RefIter> for Iter { | ||
fn values(&'a self) -> Values<RefIter> { Values { iter: self.refs() } } | ||
} | ||
|
||
///////////////////////////////////////////////////////////////////////////// | ||
|
||
struct Values<I> { iter: I } | ||
impl<'a, T: Pod, I: iter::Iterator<&'a T>> iter::Iterator<T> for Values<I> { | ||
fn next(&mut self) -> Option<T> { self.iter.next().map(|x| *x) } | ||
fn size_hint(&self) -> (uint, Option<uint>) { self.iter.size_hint() } | ||
} | ||
impl<'a, T: Pod, I: iter::DoubleEndedIterator<&'a T>> iter::DoubleEndedIterator<T> for Values<I> { | ||
fn next_back(&mut self) -> Option<T> { self.iter.next_back().map(|x| *x) } | ||
} | ||
impl<'a, T: Pod, I: iter::ExactSize<&'a T>> iter::ExactSize<T> for Values<I> {} | ||
``` | ||
|
||
Finally, here is a demonstration of using these traits to reimplement `Extendable`: | ||
|
||
```rust | ||
trait Extendable<T> { | ||
fn extend<I: Iterator<T>, Iter: Iterable<T, I>>(&mut self, x: Iter); | ||
} | ||
|
||
impl<T> Extendable<T> for Vec<T> { | ||
fn extend<I: Iterator<T>, Iter: Iterable<T, I>>(&mut self, iter: Iter) { | ||
let mut iter = iter.into_iter(); | ||
let (size, _) = iter.size_hint(); | ||
self.reserve_additional(size); | ||
|
||
for x in &mut iter { | ||
self.push(x); | ||
} | ||
} | ||
} | ||
|
||
fn main() { | ||
let mut a = Vec::new(); | ||
a.my_extend(vec!(1, 2)); | ||
a.my_extend(vec!(3, 4).into_iter()); | ||
a.my_extend(vec!(5, 6).refs().map(|x| x.clone())); | ||
a.my_extend(vec!(7, 8).values()); | ||
println!("extend: {}", a); | ||
} | ||
``` | ||
|
||
# Alternatives | ||
|
||
* We just implement everything to accept iterators. It does lead to a simpler, | ||
but less powerful, interface. | ||
* Felix Klock (pnkfelix) suggested an alternative name of `Pushable` instead of | ||
`Extendable`. This trait would also provide a `.push()` method for adding a | ||
single element. | ||
|
||
# Unresolved questions | ||
|
||
* This RFC should be revisited if/when we gain associated items. It could | ||
reduce some of the function declaration syntactic overhead. | ||
* This RFC should be revisited if we modify the coherence rules to allow impls like | ||
`impl<I: Iterator> Iterable for I { ... }` while still allowing for other | ||
impls of `Iterable`. |
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.
or a deriving?
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.
Yeah, this would be a perfect fit for
#[deriving(Iterable)]
, though it might be confusing because its only forIterators
.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.
Yuck. This is a really nasty hack, and people will forget a
#[deriving(Iterable)]
from time to time, leaving it much less usable. The only reasonable solution is to automatically implement it, which probably requires function specialisation to be implemented (rust-lang/rust#7059). Would it be feasible for us to start looking in that direction rather than piling#[deriving]
upon#[deriving]
, line upon line, here a little, there a little?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.
@chris-morgan Note that it is not possible to forget the
Iterable
impl on iterators:Iterator<A>
inherits fromIterable<A, Self>
.So all a deriving mode would enable is to easily provide the implementation as long as the coherence rules prohibit a automatic generic implementation:
instead of
Because of its special status of only being for iterators themself, a different name than
deriving
might be more appropriate though.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 added a custom attribute syntax extension to my example gist https://gist.github.com/Kimundi/9683769.
It implements a
#[iterable]
attribute that you'd need to put on every implementation ofIterator
like this:It would expand to this trivial implementation: