auto merge of #13039 : Kimundi/rust/iter_by_value_extend, r=alexcrichton

# Summary 
Changed `iter::Extendable` and `iter::FromIterator` to take a `Iterator` by value.
These functions always exhaust the passed `Iterator`, and are often used for transferring the values of a new `Iterator` directly into a data structure, so using them usually require the use of the `&mut` operator:

```
foo.extend(&mut bar.move_iter()); // Transfer content from bar into foo

let mut iter = ...;
foo.extend(&mut iter); // iter is now empty
```
This patch changes both the `FromIterator` and `Extendable` traits to take the iterator by value instead, which makes the common case of using these traits less heavy:

```
foo.extend(bar.move_iter()); // Transfer content from bar into foo

let iter = ...;
foo.extend(iter);
// iter is now inaccessible if it moved
// or unchanged if it was Pod and copied.
```
# Composability
This technically makes the traits less flexible from a type system pov, because they now require ownership. 

However, because `Iterator` provides the `ByRef` adapter, there is no loss of functionality:
```
foo.extend(iter.by_ref()); // Same semantic as today, for the few situations where you need it.
```

# Motivation
This change makes it less painful to use iterators for shuffling values around between collections, which makes it more acceptable to always use them for this, enabling more flexibility.

For example, `foo.extend(bar.move_iter())` can generally be the fastest way to append an collections content to another one, without both needing to have the same type. Making this easy to use would allow the removal of special cased methods like `push_all()` on vectors. (See https://github.com/mozilla/rust/issues/12456)

I opened https://github.com/mozilla/rust/issues/13038 as well, to discuss this change in general if people object to it.

# Further work
This didn't change the `collect()` method to take by value `self`, nor any of the other adapters that also exhaust their iterator argument. For consistency this should probably happen in the long term, but for now this is too much trouble, as every use of them would need to be checked for accidentally changed semantic by going `&mut self -> self`. (which allows for the possibility that a `Pod` iterator got copied instead of exhausted without generating a type error by the change)
This commit is contained in:
bors 2014-03-25 23:41:57 -07:00
commit 6bac5607c9
26 changed files with 75 additions and 78 deletions

View File

@ -582,7 +582,7 @@ impl<A> DoubleEndedIterator<A> for MoveItems<A> {
}
impl<A> FromIterator<A> for DList<A> {
fn from_iterator<T: Iterator<A>>(iterator: &mut T) -> DList<A> {
fn from_iterator<T: Iterator<A>>(iterator: T) -> DList<A> {
let mut ret = DList::new();
ret.extend(iterator);
ret
@ -590,8 +590,8 @@ impl<A> FromIterator<A> for DList<A> {
}
impl<A> Extendable<A> for DList<A> {
fn extend<T: Iterator<A>>(&mut self, iterator: &mut T) {
for elt in *iterator { self.push_back(elt); }
fn extend<T: Iterator<A>>(&mut self, mut iterator: T) {
for elt in iterator { self.push_back(elt); }
}
}

View File

@ -1356,7 +1356,7 @@ pub type Values<'a, K, V> =
iter::Map<'static, (&'a K, &'a V), &'a V, Entries<'a, K, V>>;
impl<K: TotalEq + Hash<S>, V, S, H: Hasher<S> + Default> FromIterator<(K, V)> for HashMap<K, V, H> {
fn from_iterator<T: Iterator<(K, V)>>(iter: &mut T) -> HashMap<K, V, H> {
fn from_iterator<T: Iterator<(K, V)>>(iter: T) -> HashMap<K, V, H> {
let (lower, _) = iter.size_hint();
let mut map = HashMap::with_capacity_and_hasher(lower, Default::default());
map.extend(iter);
@ -1365,8 +1365,8 @@ impl<K: TotalEq + Hash<S>, V, S, H: Hasher<S> + Default> FromIterator<(K, V)> fo
}
impl<K: TotalEq + Hash<S>, V, S, H: Hasher<S> + Default> Extendable<(K, V)> for HashMap<K, V, H> {
fn extend<T: Iterator<(K, V)>>(&mut self, iter: &mut T) {
for (k, v) in *iter {
fn extend<T: Iterator<(K, V)>>(&mut self, mut iter: T) {
for (k, v) in iter {
self.insert(k, v);
}
}
@ -1540,7 +1540,7 @@ impl<T: TotalEq + Hash<S> + fmt::Show, S, H: Hasher<S>> fmt::Show for HashSet<T,
}
impl<T: TotalEq + Hash<S>, S, H: Hasher<S> + Default> FromIterator<T> for HashSet<T, H> {
fn from_iterator<I: Iterator<T>>(iter: &mut I) -> HashSet<T, H> {
fn from_iterator<I: Iterator<T>>(iter: I) -> HashSet<T, H> {
let (lower, _) = iter.size_hint();
let mut set = HashSet::with_capacity_and_hasher(lower, Default::default());
set.extend(iter);
@ -1549,8 +1549,8 @@ impl<T: TotalEq + Hash<S>, S, H: Hasher<S> + Default> FromIterator<T> for HashSe
}
impl<T: TotalEq + Hash<S>, S, H: Hasher<S> + Default> Extendable<T> for HashSet<T, H> {
fn extend<I: Iterator<T>>(&mut self, iter: &mut I) {
for k in *iter {
fn extend<I: Iterator<T>>(&mut self, mut iter: I) {
for k in iter {
self.insert(k);
}
}

View File

@ -193,22 +193,21 @@ impl<'a, T> Iterator<&'a T> for Items<'a, T> {
}
impl<T: Ord> FromIterator<T> for PriorityQueue<T> {
fn from_iterator<Iter: Iterator<T>>(iter: &mut Iter) -> PriorityQueue<T> {
fn from_iterator<Iter: Iterator<T>>(iter: Iter) -> PriorityQueue<T> {
let mut q = PriorityQueue::new();
q.extend(iter);
q
}
}
impl<T: Ord> Extendable<T> for PriorityQueue<T> {
fn extend<Iter: Iterator<T>>(&mut self, iter: &mut Iter) {
fn extend<Iter: Iterator<T>>(&mut self, mut iter: Iter) {
let (lower, _) = iter.size_hint();
let len = self.capacity();
self.reserve(len + lower);
for elem in *iter {
for elem in iter {
self.push(elem);
}
}

View File

@ -386,7 +386,7 @@ impl<A: Eq> Eq for RingBuf<A> {
}
impl<A> FromIterator<A> for RingBuf<A> {
fn from_iterator<T: Iterator<A>>(iterator: &mut T) -> RingBuf<A> {
fn from_iterator<T: Iterator<A>>(iterator: T) -> RingBuf<A> {
let (lower, _) = iterator.size_hint();
let mut deq = RingBuf::with_capacity(lower);
deq.extend(iterator);
@ -395,8 +395,8 @@ impl<A> FromIterator<A> for RingBuf<A> {
}
impl<A> Extendable<A> for RingBuf<A> {
fn extend<T: Iterator<A>>(&mut self, iterator: &mut T) {
for elt in *iterator {
fn extend<T: Iterator<A>>(&mut self, mut iterator: T) {
for elt in iterator {
self.push_back(elt);
}
}

View File

@ -971,7 +971,7 @@ fn remove<K: TotalOrd, V>(node: &mut Option<~TreeNode<K, V>>,
}
impl<K: TotalOrd, V> FromIterator<(K, V)> for TreeMap<K, V> {
fn from_iterator<T: Iterator<(K, V)>>(iter: &mut T) -> TreeMap<K, V> {
fn from_iterator<T: Iterator<(K, V)>>(iter: T) -> TreeMap<K, V> {
let mut map = TreeMap::new();
map.extend(iter);
map
@ -980,15 +980,15 @@ impl<K: TotalOrd, V> FromIterator<(K, V)> for TreeMap<K, V> {
impl<K: TotalOrd, V> Extendable<(K, V)> for TreeMap<K, V> {
#[inline]
fn extend<T: Iterator<(K, V)>>(&mut self, iter: &mut T) {
for (k, v) in *iter {
fn extend<T: Iterator<(K, V)>>(&mut self, mut iter: T) {
for (k, v) in iter {
self.insert(k, v);
}
}
}
impl<T: TotalOrd> FromIterator<T> for TreeSet<T> {
fn from_iterator<Iter: Iterator<T>>(iter: &mut Iter) -> TreeSet<T> {
fn from_iterator<Iter: Iterator<T>>(iter: Iter) -> TreeSet<T> {
let mut set = TreeSet::new();
set.extend(iter);
set
@ -997,8 +997,8 @@ impl<T: TotalOrd> FromIterator<T> for TreeSet<T> {
impl<T: TotalOrd> Extendable<T> for TreeSet<T> {
#[inline]
fn extend<Iter: Iterator<T>>(&mut self, iter: &mut Iter) {
for elem in *iter {
fn extend<Iter: Iterator<T>>(&mut self, mut iter: Iter) {
for elem in iter {
self.insert(elem);
}
}

View File

@ -261,7 +261,7 @@ impl<T> TrieMap<T> {
}
impl<T> FromIterator<(uint, T)> for TrieMap<T> {
fn from_iterator<Iter: Iterator<(uint, T)>>(iter: &mut Iter) -> TrieMap<T> {
fn from_iterator<Iter: Iterator<(uint, T)>>(iter: Iter) -> TrieMap<T> {
let mut map = TrieMap::new();
map.extend(iter);
map
@ -269,8 +269,8 @@ impl<T> FromIterator<(uint, T)> for TrieMap<T> {
}
impl<T> Extendable<(uint, T)> for TrieMap<T> {
fn extend<Iter: Iterator<(uint, T)>>(&mut self, iter: &mut Iter) {
for (k, v) in *iter {
fn extend<Iter: Iterator<(uint, T)>>(&mut self, mut iter: Iter) {
for (k, v) in iter {
self.insert(k, v);
}
}
@ -346,7 +346,7 @@ impl TrieSet {
}
impl FromIterator<uint> for TrieSet {
fn from_iterator<Iter: Iterator<uint>>(iter: &mut Iter) -> TrieSet {
fn from_iterator<Iter: Iterator<uint>>(iter: Iter) -> TrieSet {
let mut set = TrieSet::new();
set.extend(iter);
set
@ -354,8 +354,8 @@ impl FromIterator<uint> for TrieSet {
}
impl Extendable<uint> for TrieSet {
fn extend<Iter: Iterator<uint>>(&mut self, iter: &mut Iter) {
for elem in *iter {
fn extend<Iter: Iterator<uint>>(&mut self, mut iter: Iter) {
for elem in iter {
self.insert(elem);
}
}

View File

@ -153,7 +153,7 @@ impl Iterator<Path> for Paths {
// so we don't need to check the children
return Some(path);
} else {
self.todo.extend(&mut list_dir_sorted(&path).move_iter().map(|x|(x,idx+1)));
self.todo.extend(list_dir_sorted(&path).move_iter().map(|x|(x,idx+1)));
}
}
}

View File

@ -2185,7 +2185,7 @@ mod bigint_tests {
nums.push(BigInt::from_slice(Minus, *s));
}
nums.push(Zero::zero());
nums.extend(&mut vs.iter().map(|s| BigInt::from_slice(Plus, *s)));
nums.extend(vs.iter().map(|s| BigInt::from_slice(Plus, *s)));
for (i, ni) in nums.iter().enumerate() {
for (j0, nj) in nums.slice(i, nums.len()).iter().enumerate() {

View File

@ -41,8 +41,8 @@ fn run_ar(sess: &Session, args: &str, cwd: Option<&Path>,
let ar = get_ar_prog(sess);
let mut args = vec!(args.to_owned());
let mut paths = paths.iter().map(|p| p.as_str().unwrap().to_owned());
args.extend(&mut paths);
let paths = paths.iter().map(|p| p.as_str().unwrap().to_owned());
args.extend(paths);
debug!("{} {}", ar, args.connect(" "));
match cwd {
Some(p) => { debug!("inside {}", p.display()); }
@ -190,7 +190,7 @@ impl<'a> Archive<'a> {
// Finally, add all the renamed files to this archive
let mut args = vec!(&self.dst);
args.extend(&mut inputs.iter());
args.extend(inputs.iter());
run_ar(self.sess, "r", None, args.as_slice());
Ok(())
}

View File

@ -496,7 +496,7 @@ pub fn collect_crate_types(session: &Session,
return vec!(CrateTypeExecutable)
}
let mut base = session.opts.crate_types.clone();
let mut iter = attrs.iter().filter_map(|a| {
let iter = attrs.iter().filter_map(|a| {
if a.name().equiv(&("crate_type")) {
match a.value_str() {
Some(ref n) if n.equiv(&("rlib")) => Some(CrateTypeRlib),
@ -525,7 +525,7 @@ pub fn collect_crate_types(session: &Session,
None
}
});
base.extend(&mut iter);
base.extend(iter);
if base.len() == 0 {
base.push(CrateTypeExecutable);
}

View File

@ -476,7 +476,7 @@ pub fn get_wrapper_for_bare_fn(ccx: &CrateContext,
}
None => {}
}
llargs.extend(&mut args.iter().map(|arg| arg.val));
llargs.extend(args.iter().map(|arg| arg.val));
let retval = Call(bcx, fn_ptr, llargs.as_slice(), []);
if type_is_zero_size(ccx, f.sig.output) || fcx.llretptr.get().is_some() {

View File

@ -57,8 +57,8 @@ pub fn type_of_rust_fn(cx: &CrateContext, has_env: bool,
}
// ... then explicit args.
let mut input_tys = inputs.iter().map(|&arg_ty| type_of_explicit_arg(cx, arg_ty));
atys.extend(&mut input_tys);
let input_tys = inputs.iter().map(|&arg_ty| type_of_explicit_arg(cx, arg_ty));
atys.extend(input_tys);
// Use the output as the actual return value if it's immediate.
if use_out_pointer || return_type_is_void(cx, output) {

View File

@ -25,7 +25,7 @@ pub trait DocFolder {
StructItem(mut i) => {
let mut foo = Vec::new(); swap(&mut foo, &mut i.fields);
let num_fields = foo.len();
i.fields.extend(&mut foo.move_iter().filter_map(|x| self.fold_item(x)));
i.fields.extend(foo.move_iter().filter_map(|x| self.fold_item(x)));
i.fields_stripped |= num_fields != i.fields.len();
StructItem(i)
},
@ -35,7 +35,7 @@ pub trait DocFolder {
EnumItem(mut i) => {
let mut foo = Vec::new(); swap(&mut foo, &mut i.variants);
let num_variants = foo.len();
i.variants.extend(&mut foo.move_iter().filter_map(|x| self.fold_item(x)));
i.variants.extend(foo.move_iter().filter_map(|x| self.fold_item(x)));
i.variants_stripped |= num_variants != i.variants.len();
EnumItem(i)
},
@ -57,12 +57,12 @@ pub trait DocFolder {
}
}
let mut foo = Vec::new(); swap(&mut foo, &mut i.methods);
i.methods.extend(&mut foo.move_iter().filter_map(|x| vtrm(self, x)));
i.methods.extend(foo.move_iter().filter_map(|x| vtrm(self, x)));
TraitItem(i)
},
ImplItem(mut i) => {
let mut foo = Vec::new(); swap(&mut foo, &mut i.methods);
i.methods.extend(&mut foo.move_iter().filter_map(|x| self.fold_item(x)));
i.methods.extend(foo.move_iter().filter_map(|x| self.fold_item(x)));
ImplItem(i)
},
VariantItem(i) => {
@ -72,7 +72,7 @@ pub trait DocFolder {
let mut foo = Vec::new(); swap(&mut foo, &mut j.fields);
let num_fields = foo.len();
let c = |x| self.fold_item(x);
j.fields.extend(&mut foo.move_iter().filter_map(c));
j.fields.extend(foo.move_iter().filter_map(c));
j.fields_stripped |= num_fields != j.fields.len();
VariantItem(Variant {kind: StructVariant(j), ..i2})
},

View File

@ -76,13 +76,13 @@ use mem;
/// Conversion from an `Iterator`
pub trait FromIterator<A> {
/// Build a container with elements from an external iterator.
fn from_iterator<T: Iterator<A>>(iterator: &mut T) -> Self;
fn from_iterator<T: Iterator<A>>(iterator: T) -> Self;
}
/// A type growable from an `Iterator` implementation
pub trait Extendable<A>: FromIterator<A> {
/// Extend a container with the elements yielded by an iterator
fn extend<T: Iterator<A>>(&mut self, iterator: &mut T);
fn extend<T: Iterator<A>>(&mut self, iterator: T);
}
/// An interface for dealing with "external iterators". These types of iterators
@ -460,7 +460,7 @@ pub trait Iterator<A> {
/// ```
#[inline]
fn collect<B: FromIterator<A>>(&mut self) -> B {
FromIterator::from_iterator(self)
FromIterator::from_iterator(self.by_ref())
}
/// Loops through `n` iterations, returning the `n`th element of the
@ -2336,7 +2336,7 @@ mod tests {
#[test]
fn test_counter_from_iter() {
let mut it = count(0, 5).take(10);
let xs: ~[int] = FromIterator::from_iterator(&mut it);
let xs: ~[int] = FromIterator::from_iterator(it);
assert_eq!(xs, ~[0, 5, 10, 15, 20, 25, 30, 35, 40, 45]);
}

View File

@ -593,7 +593,7 @@ pub fn collect<T, Iter: Iterator<Option<T>>, V: FromIterator<T>>(iter: Iter) ->
}
});
let v: V = FromIterator::from_iterator(&mut iter);
let v: V = FromIterator::from_iterator(iter.by_ref());
if iter.state {
None

View File

@ -277,7 +277,7 @@ impl GenericPath for Path {
(None, None) => break,
(Some(a), None) => {
comps.push(a);
comps.extend(&mut ita);
comps.extend(ita.by_ref());
break;
}
(None, _) => comps.push(dot_dot_static),
@ -290,7 +290,7 @@ impl GenericPath for Path {
comps.push(dot_dot_static);
}
comps.push(a);
comps.extend(&mut ita);
comps.extend(ita.by_ref());
break;
}
}

View File

@ -539,7 +539,7 @@ impl GenericPath for Path {
(Some(a), None) => {
comps.push(a);
if !a_verb {
comps.extend(&mut ita);
comps.extend(ita.by_ref());
break;
}
}
@ -561,7 +561,7 @@ impl GenericPath for Path {
}
comps.push(a);
if !a_verb {
comps.extend(&mut ita);
comps.extend(ita.by_ref());
break;
}
}

View File

@ -230,7 +230,7 @@ pub fn collect<T, E, Iter: Iterator<Result<T, E>>, V: FromIterator<T>>(iter: Ite
}
});
let v: V = FromIterator::from_iterator(&mut iter);
let v: V = FromIterator::from_iterator(iter.by_ref());
match iter.state {
Some(err) => Err(err),

View File

@ -2919,10 +2919,10 @@ impl<T> Drop for MoveItems<T> {
pub type RevMoveItems<T> = Rev<MoveItems<T>>;
impl<A> FromIterator<A> for ~[A] {
fn from_iterator<T: Iterator<A>>(iterator: &mut T) -> ~[A] {
fn from_iterator<T: Iterator<A>>(mut iterator: T) -> ~[A] {
let (lower, _) = iterator.size_hint();
let mut xs = with_capacity(lower);
for x in *iterator {
for x in iterator {
xs.push(x);
}
xs
@ -2930,11 +2930,11 @@ impl<A> FromIterator<A> for ~[A] {
}
impl<A> Extendable<A> for ~[A] {
fn extend<T: Iterator<A>>(&mut self, iterator: &mut T) {
fn extend<T: Iterator<A>>(&mut self, mut iterator: T) {
let (lower, _) = iterator.size_hint();
let len = self.len();
self.reserve_exact(len + lower);
for x in *iterator {
for x in iterator {
self.push(x);
}
}

View File

@ -3019,7 +3019,7 @@ impl Clone for ~str {
impl FromIterator<char> for ~str {
#[inline]
fn from_iterator<T: Iterator<char>>(iterator: &mut T) -> ~str {
fn from_iterator<T: Iterator<char>>(iterator: T) -> ~str {
let (lower, _) = iterator.size_hint();
let mut buf = with_capacity(lower);
buf.extend(iterator);
@ -3029,11 +3029,11 @@ impl FromIterator<char> for ~str {
impl Extendable<char> for ~str {
#[inline]
fn extend<T: Iterator<char>>(&mut self, iterator: &mut T) {
fn extend<T: Iterator<char>>(&mut self, mut iterator: T) {
let (lower, _) = iterator.size_hint();
let reserve = lower + self.len();
self.reserve(reserve);
for ch in *iterator {
for ch in iterator {
self.push_char(ch)
}
}
@ -3219,7 +3219,7 @@ mod tests {
let mut cpy = data.clone();
let other = "abc";
let mut it = other.chars();
cpy.extend(&mut it);
cpy.extend(it);
assert_eq!(cpy, data + other);
}

View File

@ -305,10 +305,10 @@ impl<T:Clone> Clone for Vec<T> {
}
impl<T> FromIterator<T> for Vec<T> {
fn from_iterator<I:Iterator<T>>(iterator: &mut I) -> Vec<T> {
fn from_iterator<I:Iterator<T>>(mut iterator: I) -> Vec<T> {
let (lower, _) = iterator.size_hint();
let mut vector = Vec::with_capacity(lower);
for element in *iterator {
for element in iterator {
vector.push(element)
}
vector
@ -316,10 +316,10 @@ impl<T> FromIterator<T> for Vec<T> {
}
impl<T> Extendable<T> for Vec<T> {
fn extend<I: Iterator<T>>(&mut self, iterator: &mut I) {
fn extend<I: Iterator<T>>(&mut self, mut iterator: I) {
let (lower, _) = iterator.size_hint();
self.reserve_additional(lower);
for element in *iterator {
for element in iterator {
self.push(element)
}
}
@ -1429,12 +1429,12 @@ mod tests {
let mut v = Vec::new();
let mut w = Vec::new();
v.extend(&mut range(0, 3));
v.extend(range(0, 3));
for i in range(0, 3) { w.push(i) }
assert_eq!(v, w);
v.extend(&mut range(3, 10));
v.extend(range(3, 10));
for i in range(3, 10) { w.push(i) }
assert_eq!(v, w);

View File

@ -359,7 +359,7 @@ impl<'a> ExtCtxt<'a> {
pub fn mod_path(&self) -> Vec<ast::Ident> {
let mut v = Vec::new();
v.push(token::str_to_ident(self.ecfg.crate_id.name));
v.extend(&mut self.mod_path.iter().map(|a| *a));
v.extend(self.mod_path.iter().map(|a| *a));
return v;
}
pub fn bt_push(&mut self, ei: codemap::ExpnInfo) {

View File

@ -365,10 +365,10 @@ impl<'a> TraitDef<'a> {
let mut ty_params = ty_params.into_vec();
// Copy the lifetimes
lifetimes.extend(&mut generics.lifetimes.iter().map(|l| *l));
lifetimes.extend(generics.lifetimes.iter().map(|l| *l));
// Create the type parameters.
ty_params.extend(&mut generics.ty_params.iter().map(|ty_param| {
ty_params.extend(generics.ty_params.iter().map(|ty_param| {
// I don't think this can be moved out of the loop, since
// a TyParamBound requires an ast id
let mut bounds =

View File

@ -282,7 +282,7 @@ pub fn expand_item(it: @ast::Item, fld: &mut MacroExpander)
let mut items: SmallVector<@ast::Item> = SmallVector::zero();
dec_fn(fld.cx, attr.span, attr.node.value, it,
|item| items.push(item));
decorator_items.extend(&mut items.move_iter()
decorator_items.extend(items.move_iter()
.flat_map(|item| expand_item(item, fld).move_iter()));
fld.cx.bt_pop();

View File

@ -126,7 +126,7 @@ impl<T> Container for OwnedSlice<T> {
}
impl<T> FromIterator<T> for OwnedSlice<T> {
fn from_iterator<I: Iterator<T>>(iter: &mut I) -> OwnedSlice<T> {
fn from_iterator<I: Iterator<T>>(mut iter: I) -> OwnedSlice<T> {
OwnedSlice::from_vec(iter.collect())
}
}

View File

@ -29,18 +29,16 @@ impl<T> Container for SmallVector<T> {
}
impl<T> FromIterator<T> for SmallVector<T> {
fn from_iterator<I: Iterator<T>>(iter: &mut I) -> SmallVector<T> {
fn from_iterator<I: Iterator<T>>(iter: I) -> SmallVector<T> {
let mut v = Zero;
for val in *iter {
v.push(val);
}
v.extend(iter);
v
}
}
impl<T> Extendable<T> for SmallVector<T> {
fn extend<I: Iterator<T>>(&mut self, iter: &mut I) {
for val in *iter {
fn extend<I: Iterator<T>>(&mut self, mut iter: I) {
for val in iter {
self.push(val);
}
}