Skip to content

Commit

Permalink
Change VecMap's iterators to use wrapper structs instead of typedefs.
Browse files Browse the repository at this point in the history
Using a type alias for iterator implementations is fragile since this
exposes the implementation to users of the iterator, and any changes
could break existing code.

This commit changes the iterators of `VecMap` to use
proper new types, rather than type aliases.  However, since it is
fair-game to treat a type-alias as the aliased type, this is a:

[breaking-change].
  • Loading branch information
csouth3 committed Dec 14, 2014
1 parent 444fa1b commit 81f9a31
Showing 1 changed file with 45 additions and 14 deletions.
59 changes: 45 additions & 14 deletions src/libcollections/vec_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use core::prelude::*;
use core::default::Default;
use core::fmt;
use core::iter;
use core::iter::{Enumerate, FilterMap};
use core::iter::{Enumerate, FilterMap, Map};
use core::mem::replace;
use core::ops::FnOnce;

Expand Down Expand Up @@ -144,7 +144,7 @@ impl<V> VecMap<V> {
pub fn keys<'r>(&'r self) -> Keys<'r, V> {
fn first<A, B>((a, _): (A, B)) -> A { a }

self.iter().map(first)
Keys { iter: self.iter().map(first) }
}

/// Returns an iterator visiting all values in ascending order by the keys.
Expand All @@ -153,7 +153,7 @@ impl<V> VecMap<V> {
pub fn values<'r>(&'r self) -> Values<'r, V> {
fn second<A, B>((_, b): (A, B)) -> B { b }

self.iter().map(second)
Values { iter: self.iter().map(second) }
}

/// Returns an iterator visiting all key-value pairs in ascending order by the keys.
Expand Down Expand Up @@ -240,7 +240,7 @@ impl<V> VecMap<V> {
}

let values = replace(&mut self.v, vec!());
values.into_iter().enumerate().filter_map(filter)
MoveItems { iter: values.into_iter().enumerate().filter_map(filter) }
}

/// Return the number of elements in the map.
Expand Down Expand Up @@ -603,7 +603,7 @@ macro_rules! double_ended_iterator {
}
}

/// Forward iterator over a map.
/// An iterator over the key-value pairs of a map.
pub struct Entries<'a, V:'a> {
front: uint,
back: uint,
Expand All @@ -613,7 +613,7 @@ pub struct Entries<'a, V:'a> {
iterator!(impl Entries -> (uint, &'a V), as_ref)
double_ended_iterator!(impl Entries -> (uint, &'a V), as_ref)

/// Forward iterator over the key-value pairs of a map, with the
/// An iterator over the key-value pairs of a map, with the
/// values being mutable.
pub struct MutEntries<'a, V:'a> {
front: uint,
Expand All @@ -624,19 +624,50 @@ pub struct MutEntries<'a, V:'a> {
iterator!(impl MutEntries -> (uint, &'a mut V), as_mut)
double_ended_iterator!(impl MutEntries -> (uint, &'a mut V), as_mut)

/// Forward iterator over the keys of a map
pub type Keys<'a, V> = iter::Map<(uint, &'a V), uint, Entries<'a, V>, fn((uint, &'a V)) -> uint>;
/// An iterator over the keys of a map.
pub struct Keys<'a, V: 'a> {
iter: Map<(uint, &'a V), uint, Entries<'a, V>, fn((uint, &'a V)) -> uint>
}

/// Forward iterator over the values of a map
pub type Values<'a, V> =
iter::Map<(uint, &'a V), &'a V, Entries<'a, V>, fn((uint, &'a V)) -> &'a V>;
/// An iterator over the values of a map.
pub struct Values<'a, V: 'a> {
iter: Map<(uint, &'a V), &'a V, Entries<'a, V>, fn((uint, &'a V)) -> &'a V>
}

/// Iterator over the key-value pairs of a map, the iterator consumes the map
pub type MoveItems<V> = FilterMap<
/// A consuming iterator over the key-value pairs of a map.
pub struct MoveItems<V> {
iter: FilterMap<
(uint, Option<V>),
(uint, V),
Enumerate<vec::MoveItems<Option<V>>>,
fn((uint, Option<V>)) -> Option<(uint, V)>>;
fn((uint, Option<V>)) -> Option<(uint, V)>>
}

impl<'a, V> Iterator<uint> for Keys<'a, V> {
fn next(&mut self) -> Option<uint> { self.iter.next() }
fn size_hint(&self) -> (uint, Option<uint>) { self.iter.size_hint() }
}
impl<'a, V> DoubleEndedIterator<uint> for Keys<'a, V> {
fn next_back(&mut self) -> Option<uint> { self.iter.next_back() }
}


impl<'a, V> Iterator<&'a V> for Values<'a, V> {
fn next(&mut self) -> Option<(&'a V)> { self.iter.next() }
fn size_hint(&self) -> (uint, Option<uint>) { self.iter.size_hint() }
}
impl<'a, V> DoubleEndedIterator<&'a V> for Values<'a, V> {
fn next_back(&mut self) -> Option<(&'a V)> { self.iter.next_back() }
}


impl<V> Iterator<(uint, V)> for MoveItems<V> {
fn next(&mut self) -> Option<(uint, V)> { self.iter.next() }
fn size_hint(&self) -> (uint, Option<uint>) { self.iter.size_hint() }
}
impl<V> DoubleEndedIterator<(uint, V)> for MoveItems<V> {
fn next_back(&mut self) -> Option<(uint, V)> { self.iter.next_back() }
}

#[cfg(test)]
mod test_map {
Expand Down

18 comments on commit 81f9a31

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 14, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 14, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors rollup

@csouth3
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh oh...got sent to BAD for the rollup even though my commits weren't actually the culprit this time :P heh

@barosl
Copy link

@barosl barosl commented on 81f9a31 Dec 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@csouth3 That is a pretty frightening behavior, sorry. I'll try to find a way to mark the failing PR correctly.

@csouth3
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry, it's cool :D I just need to get re- r+'ed and rollup'ed.

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors retry

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors p=-1

@csouth3
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gankro don't think bors heard your p=-1, I'm at the top of the queue in front of the rollup PR :p

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors plz

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmr needs to remove their p = 1.

@csouth3
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. I thought bors had just gained a will of his own :^)

@emberian
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors retry

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p=-1

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plz

@Gankra
Copy link

@Gankra Gankra commented on 81f9a31 Dec 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plllllzzzzzzz

@csouth3
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄 Bors is listening again!

Please sign in to comment.