Skip to content

Commit 6bac560

Browse files
committed
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 #12456) I opened #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)
2 parents e28f081 + 6200e76 commit 6bac560

File tree

26 files changed

+75
-78
lines changed

26 files changed

+75
-78
lines changed

src/libcollections/dlist.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -582,16 +582,16 @@ impl<A> DoubleEndedIterator<A> for MoveItems<A> {
582582
}
583583

584584
impl<A> FromIterator<A> for DList<A> {
585-
fn from_iterator<T: Iterator<A>>(iterator: &mut T) -> DList<A> {
585+
fn from_iterator<T: Iterator<A>>(iterator: T) -> DList<A> {
586586
let mut ret = DList::new();
587587
ret.extend(iterator);
588588
ret
589589
}
590590
}
591591

592592
impl<A> Extendable<A> for DList<A> {
593-
fn extend<T: Iterator<A>>(&mut self, iterator: &mut T) {
594-
for elt in *iterator { self.push_back(elt); }
593+
fn extend<T: Iterator<A>>(&mut self, mut iterator: T) {
594+
for elt in iterator { self.push_back(elt); }
595595
}
596596
}
597597

src/libcollections/hashmap.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1356,7 +1356,7 @@ pub type Values<'a, K, V> =
13561356
iter::Map<'static, (&'a K, &'a V), &'a V, Entries<'a, K, V>>;
13571357

13581358
impl<K: TotalEq + Hash<S>, V, S, H: Hasher<S> + Default> FromIterator<(K, V)> for HashMap<K, V, H> {
1359-
fn from_iterator<T: Iterator<(K, V)>>(iter: &mut T) -> HashMap<K, V, H> {
1359+
fn from_iterator<T: Iterator<(K, V)>>(iter: T) -> HashMap<K, V, H> {
13601360
let (lower, _) = iter.size_hint();
13611361
let mut map = HashMap::with_capacity_and_hasher(lower, Default::default());
13621362
map.extend(iter);
@@ -1365,8 +1365,8 @@ impl<K: TotalEq + Hash<S>, V, S, H: Hasher<S> + Default> FromIterator<(K, V)> fo
13651365
}
13661366

13671367
impl<K: TotalEq + Hash<S>, V, S, H: Hasher<S> + Default> Extendable<(K, V)> for HashMap<K, V, H> {
1368-
fn extend<T: Iterator<(K, V)>>(&mut self, iter: &mut T) {
1369-
for (k, v) in *iter {
1368+
fn extend<T: Iterator<(K, V)>>(&mut self, mut iter: T) {
1369+
for (k, v) in iter {
13701370
self.insert(k, v);
13711371
}
13721372
}
@@ -1540,7 +1540,7 @@ impl<T: TotalEq + Hash<S> + fmt::Show, S, H: Hasher<S>> fmt::Show for HashSet<T,
15401540
}
15411541

15421542
impl<T: TotalEq + Hash<S>, S, H: Hasher<S> + Default> FromIterator<T> for HashSet<T, H> {
1543-
fn from_iterator<I: Iterator<T>>(iter: &mut I) -> HashSet<T, H> {
1543+
fn from_iterator<I: Iterator<T>>(iter: I) -> HashSet<T, H> {
15441544
let (lower, _) = iter.size_hint();
15451545
let mut set = HashSet::with_capacity_and_hasher(lower, Default::default());
15461546
set.extend(iter);
@@ -1549,8 +1549,8 @@ impl<T: TotalEq + Hash<S>, S, H: Hasher<S> + Default> FromIterator<T> for HashSe
15491549
}
15501550

15511551
impl<T: TotalEq + Hash<S>, S, H: Hasher<S> + Default> Extendable<T> for HashSet<T, H> {
1552-
fn extend<I: Iterator<T>>(&mut self, iter: &mut I) {
1553-
for k in *iter {
1552+
fn extend<I: Iterator<T>>(&mut self, mut iter: I) {
1553+
for k in iter {
15541554
self.insert(k);
15551555
}
15561556
}

src/libcollections/priority_queue.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -193,22 +193,21 @@ impl<'a, T> Iterator<&'a T> for Items<'a, T> {
193193
}
194194

195195
impl<T: Ord> FromIterator<T> for PriorityQueue<T> {
196-
fn from_iterator<Iter: Iterator<T>>(iter: &mut Iter) -> PriorityQueue<T> {
196+
fn from_iterator<Iter: Iterator<T>>(iter: Iter) -> PriorityQueue<T> {
197197
let mut q = PriorityQueue::new();
198198
q.extend(iter);
199-
200199
q
201200
}
202201
}
203202

204203
impl<T: Ord> Extendable<T> for PriorityQueue<T> {
205-
fn extend<Iter: Iterator<T>>(&mut self, iter: &mut Iter) {
204+
fn extend<Iter: Iterator<T>>(&mut self, mut iter: Iter) {
206205
let (lower, _) = iter.size_hint();
207206

208207
let len = self.capacity();
209208
self.reserve(len + lower);
210209

211-
for elem in *iter {
210+
for elem in iter {
212211
self.push(elem);
213212
}
214213
}

src/libcollections/ringbuf.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ impl<A: Eq> Eq for RingBuf<A> {
386386
}
387387

388388
impl<A> FromIterator<A> for RingBuf<A> {
389-
fn from_iterator<T: Iterator<A>>(iterator: &mut T) -> RingBuf<A> {
389+
fn from_iterator<T: Iterator<A>>(iterator: T) -> RingBuf<A> {
390390
let (lower, _) = iterator.size_hint();
391391
let mut deq = RingBuf::with_capacity(lower);
392392
deq.extend(iterator);
@@ -395,8 +395,8 @@ impl<A> FromIterator<A> for RingBuf<A> {
395395
}
396396

397397
impl<A> Extendable<A> for RingBuf<A> {
398-
fn extend<T: Iterator<A>>(&mut self, iterator: &mut T) {
399-
for elt in *iterator {
398+
fn extend<T: Iterator<A>>(&mut self, mut iterator: T) {
399+
for elt in iterator {
400400
self.push_back(elt);
401401
}
402402
}

src/libcollections/treemap.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -971,7 +971,7 @@ fn remove<K: TotalOrd, V>(node: &mut Option<~TreeNode<K, V>>,
971971
}
972972

973973
impl<K: TotalOrd, V> FromIterator<(K, V)> for TreeMap<K, V> {
974-
fn from_iterator<T: Iterator<(K, V)>>(iter: &mut T) -> TreeMap<K, V> {
974+
fn from_iterator<T: Iterator<(K, V)>>(iter: T) -> TreeMap<K, V> {
975975
let mut map = TreeMap::new();
976976
map.extend(iter);
977977
map
@@ -980,15 +980,15 @@ impl<K: TotalOrd, V> FromIterator<(K, V)> for TreeMap<K, V> {
980980

981981
impl<K: TotalOrd, V> Extendable<(K, V)> for TreeMap<K, V> {
982982
#[inline]
983-
fn extend<T: Iterator<(K, V)>>(&mut self, iter: &mut T) {
984-
for (k, v) in *iter {
983+
fn extend<T: Iterator<(K, V)>>(&mut self, mut iter: T) {
984+
for (k, v) in iter {
985985
self.insert(k, v);
986986
}
987987
}
988988
}
989989

990990
impl<T: TotalOrd> FromIterator<T> for TreeSet<T> {
991-
fn from_iterator<Iter: Iterator<T>>(iter: &mut Iter) -> TreeSet<T> {
991+
fn from_iterator<Iter: Iterator<T>>(iter: Iter) -> TreeSet<T> {
992992
let mut set = TreeSet::new();
993993
set.extend(iter);
994994
set
@@ -997,8 +997,8 @@ impl<T: TotalOrd> FromIterator<T> for TreeSet<T> {
997997

998998
impl<T: TotalOrd> Extendable<T> for TreeSet<T> {
999999
#[inline]
1000-
fn extend<Iter: Iterator<T>>(&mut self, iter: &mut Iter) {
1001-
for elem in *iter {
1000+
fn extend<Iter: Iterator<T>>(&mut self, mut iter: Iter) {
1001+
for elem in iter {
10021002
self.insert(elem);
10031003
}
10041004
}

src/libcollections/trie.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -261,16 +261,16 @@ impl<T> TrieMap<T> {
261261
}
262262

263263
impl<T> FromIterator<(uint, T)> for TrieMap<T> {
264-
fn from_iterator<Iter: Iterator<(uint, T)>>(iter: &mut Iter) -> TrieMap<T> {
264+
fn from_iterator<Iter: Iterator<(uint, T)>>(iter: Iter) -> TrieMap<T> {
265265
let mut map = TrieMap::new();
266266
map.extend(iter);
267267
map
268268
}
269269
}
270270

271271
impl<T> Extendable<(uint, T)> for TrieMap<T> {
272-
fn extend<Iter: Iterator<(uint, T)>>(&mut self, iter: &mut Iter) {
273-
for (k, v) in *iter {
272+
fn extend<Iter: Iterator<(uint, T)>>(&mut self, mut iter: Iter) {
273+
for (k, v) in iter {
274274
self.insert(k, v);
275275
}
276276
}
@@ -346,16 +346,16 @@ impl TrieSet {
346346
}
347347

348348
impl FromIterator<uint> for TrieSet {
349-
fn from_iterator<Iter: Iterator<uint>>(iter: &mut Iter) -> TrieSet {
349+
fn from_iterator<Iter: Iterator<uint>>(iter: Iter) -> TrieSet {
350350
let mut set = TrieSet::new();
351351
set.extend(iter);
352352
set
353353
}
354354
}
355355

356356
impl Extendable<uint> for TrieSet {
357-
fn extend<Iter: Iterator<uint>>(&mut self, iter: &mut Iter) {
358-
for elem in *iter {
357+
fn extend<Iter: Iterator<uint>>(&mut self, mut iter: Iter) {
358+
for elem in iter {
359359
self.insert(elem);
360360
}
361361
}

src/libglob/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl Iterator<Path> for Paths {
153153
// so we don't need to check the children
154154
return Some(path);
155155
} else {
156-
self.todo.extend(&mut list_dir_sorted(&path).move_iter().map(|x|(x,idx+1)));
156+
self.todo.extend(list_dir_sorted(&path).move_iter().map(|x|(x,idx+1)));
157157
}
158158
}
159159
}

src/libnum/bigint.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2185,7 +2185,7 @@ mod bigint_tests {
21852185
nums.push(BigInt::from_slice(Minus, *s));
21862186
}
21872187
nums.push(Zero::zero());
2188-
nums.extend(&mut vs.iter().map(|s| BigInt::from_slice(Plus, *s)));
2188+
nums.extend(vs.iter().map(|s| BigInt::from_slice(Plus, *s)));
21892189
21902190
for (i, ni) in nums.iter().enumerate() {
21912191
for (j0, nj) in nums.slice(i, nums.len()).iter().enumerate() {

src/librustc/back/archive.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ fn run_ar(sess: &Session, args: &str, cwd: Option<&Path>,
4141
let ar = get_ar_prog(sess);
4242

4343
let mut args = vec!(args.to_owned());
44-
let mut paths = paths.iter().map(|p| p.as_str().unwrap().to_owned());
45-
args.extend(&mut paths);
44+
let paths = paths.iter().map(|p| p.as_str().unwrap().to_owned());
45+
args.extend(paths);
4646
debug!("{} {}", ar, args.connect(" "));
4747
match cwd {
4848
Some(p) => { debug!("inside {}", p.display()); }
@@ -190,7 +190,7 @@ impl<'a> Archive<'a> {
190190

191191
// Finally, add all the renamed files to this archive
192192
let mut args = vec!(&self.dst);
193-
args.extend(&mut inputs.iter());
193+
args.extend(inputs.iter());
194194
run_ar(self.sess, "r", None, args.as_slice());
195195
Ok(())
196196
}

src/librustc/driver/session.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,7 @@ pub fn collect_crate_types(session: &Session,
496496
return vec!(CrateTypeExecutable)
497497
}
498498
let mut base = session.opts.crate_types.clone();
499-
let mut iter = attrs.iter().filter_map(|a| {
499+
let iter = attrs.iter().filter_map(|a| {
500500
if a.name().equiv(&("crate_type")) {
501501
match a.value_str() {
502502
Some(ref n) if n.equiv(&("rlib")) => Some(CrateTypeRlib),
@@ -525,7 +525,7 @@ pub fn collect_crate_types(session: &Session,
525525
None
526526
}
527527
});
528-
base.extend(&mut iter);
528+
base.extend(iter);
529529
if base.len() == 0 {
530530
base.push(CrateTypeExecutable);
531531
}

0 commit comments

Comments
 (0)