Skip to content

Commit bc486dc

Browse files
committed
auto merge of #19663 : tbu-/rust/pr_fix_vecmap, r=Gankro
- Introduce a named type for the return type of `VecMap::move_iter` - Rename all type parameters to `V` for "Value". - Remove unnecessary call to an `Option::unwrap`, use pattern matching instead. - Remove incorrect `Hash` implementation which took the `VecMap`'s capacity into account. This is a [breaking-change], however whoever used the `Hash` implementation relied on an incorrect implementation.
2 parents daa2bde + 20eaf16 commit bc486dc

File tree

1 file changed

+53
-35
lines changed

1 file changed

+53
-35
lines changed

src/libcollections/vec_map.rs

+53-35
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,9 @@ use core::iter;
2121
use core::iter::{Enumerate, FilterMap};
2222
use core::mem::replace;
2323

24+
use hash::{Hash, Writer};
2425
use {vec, slice};
2526
use vec::Vec;
26-
use hash;
27-
use hash::Hash;
2827

2928
// FIXME(conventions): capacity management???
3029

@@ -62,8 +61,8 @@ use hash::Hash;
6261
/// assert!(months.is_empty());
6362
/// ```
6463
#[deriving(PartialEq, Eq)]
65-
pub struct VecMap<T> {
66-
v: Vec<Option<T>>,
64+
pub struct VecMap<V> {
65+
v: Vec<Option<V>>,
6766
}
6867

6968
impl<V> Default for VecMap<V> {
@@ -83,9 +82,16 @@ impl<V:Clone> Clone for VecMap<V> {
8382
}
8483
}
8584

86-
impl <S: hash::Writer, T: Hash<S>> Hash<S> for VecMap<T> {
85+
impl<S: Writer, V: Hash<S>> Hash<S> for VecMap<V> {
8786
fn hash(&self, state: &mut S) {
88-
self.v.hash(state)
87+
// In order to not traverse the `VecMap` twice, count the elements
88+
// during iteration.
89+
let mut count: uint = 0;
90+
for elt in self.iter() {
91+
elt.hash(state);
92+
count += 1;
93+
}
94+
count.hash(state);
8995
}
9096
}
9197

@@ -99,7 +105,7 @@ impl<V> VecMap<V> {
99105
/// let mut map: VecMap<&str> = VecMap::new();
100106
/// ```
101107
#[unstable = "matches collection reform specification, waiting for dust to settle"]
102-
pub fn new() -> VecMap<V> { VecMap{v: vec!()} }
108+
pub fn new() -> VecMap<V> { VecMap { v: vec![] } }
103109

104110
/// Creates an empty `VecMap` with space for at least `capacity`
105111
/// elements before resizing.
@@ -223,10 +229,7 @@ impl<V> VecMap<V> {
223229
/// assert_eq!(vec, vec![(1, "a"), (2, "b"), (3, "c")]);
224230
/// ```
225231
#[unstable = "matches collection reform specification, waiting for dust to settle"]
226-
pub fn into_iter(&mut self)
227-
-> FilterMap<(uint, Option<V>), (uint, V),
228-
Enumerate<vec::MoveItems<Option<V>>>>
229-
{
232+
pub fn into_iter(&mut self) -> MoveItems<V> {
230233
let values = replace(&mut self.v, vec!());
231234
values.into_iter().enumerate().filter_map(|(i, v)| {
232235
v.map(|v| (i, v))
@@ -539,16 +542,19 @@ impl<V> IndexMut<uint, V> for VecMap<V> {
539542

540543
macro_rules! iterator {
541544
(impl $name:ident -> $elem:ty, $($getter:ident),+) => {
542-
impl<'a, T> Iterator<$elem> for $name<'a, T> {
545+
impl<'a, V> Iterator<$elem> for $name<'a, V> {
543546
#[inline]
544547
fn next(&mut self) -> Option<$elem> {
545548
while self.front < self.back {
546549
match self.iter.next() {
547550
Some(elem) => {
548-
if elem.is_some() {
549-
let index = self.front;
550-
self.front += 1;
551-
return Some((index, elem $(. $getter ())+));
551+
match elem$(. $getter ())+ {
552+
Some(x) => {
553+
let index = self.front;
554+
self.front += 1;
555+
return Some((index, x));
556+
},
557+
None => {},
552558
}
553559
}
554560
_ => ()
@@ -568,15 +574,18 @@ macro_rules! iterator {
568574

569575
macro_rules! double_ended_iterator {
570576
(impl $name:ident -> $elem:ty, $($getter:ident),+) => {
571-
impl<'a, T> DoubleEndedIterator<$elem> for $name<'a, T> {
577+
impl<'a, V> DoubleEndedIterator<$elem> for $name<'a, V> {
572578
#[inline]
573579
fn next_back(&mut self) -> Option<$elem> {
574580
while self.front < self.back {
575581
match self.iter.next_back() {
576582
Some(elem) => {
577-
if elem.is_some() {
578-
self.back -= 1;
579-
return Some((self.back, elem$(. $getter ())+));
583+
match elem$(. $getter ())+ {
584+
Some(x) => {
585+
self.back -= 1;
586+
return Some((self.back, x));
587+
},
588+
None => {},
580589
}
581590
}
582591
_ => ()
@@ -590,39 +599,43 @@ macro_rules! double_ended_iterator {
590599
}
591600

592601
/// Forward iterator over a map.
593-
pub struct Entries<'a, T:'a> {
602+
pub struct Entries<'a, V:'a> {
594603
front: uint,
595604
back: uint,
596-
iter: slice::Items<'a, Option<T>>
605+
iter: slice::Items<'a, Option<V>>
597606
}
598607

599-
iterator!(impl Entries -> (uint, &'a T), as_ref, unwrap)
600-
double_ended_iterator!(impl Entries -> (uint, &'a T), as_ref, unwrap)
608+
iterator!(impl Entries -> (uint, &'a V), as_ref)
609+
double_ended_iterator!(impl Entries -> (uint, &'a V), as_ref)
601610

602611
/// Forward iterator over the key-value pairs of a map, with the
603612
/// values being mutable.
604-
pub struct MutEntries<'a, T:'a> {
613+
pub struct MutEntries<'a, V:'a> {
605614
front: uint,
606615
back: uint,
607-
iter: slice::MutItems<'a, Option<T>>
616+
iter: slice::MutItems<'a, Option<V>>
608617
}
609618

610-
iterator!(impl MutEntries -> (uint, &'a mut T), as_mut, unwrap)
611-
double_ended_iterator!(impl MutEntries -> (uint, &'a mut T), as_mut, unwrap)
619+
iterator!(impl MutEntries -> (uint, &'a mut V), as_mut)
620+
double_ended_iterator!(impl MutEntries -> (uint, &'a mut V), as_mut)
612621

613622
/// Forward iterator over the keys of a map
614-
pub type Keys<'a, T> =
615-
iter::Map<'static, (uint, &'a T), uint, Entries<'a, T>>;
623+
pub type Keys<'a, V> =
624+
iter::Map<'static, (uint, &'a V), uint, Entries<'a, V>>;
616625

617626
/// Forward iterator over the values of a map
618-
pub type Values<'a, T> =
619-
iter::Map<'static, (uint, &'a T), &'a T, Entries<'a, T>>;
627+
pub type Values<'a, V> =
628+
iter::Map<'static, (uint, &'a V), &'a V, Entries<'a, V>>;
629+
630+
/// Iterator over the key-value pairs of a map, the iterator consumes the map
631+
pub type MoveItems<V> =
632+
FilterMap<'static, (uint, Option<V>), (uint, V), Enumerate<vec::MoveItems<Option<V>>>>;
620633

621634
#[cfg(test)]
622635
mod test_map {
623636
use std::prelude::*;
624637
use vec::Vec;
625-
use hash;
638+
use hash::hash;
626639

627640
use super::VecMap;
628641

@@ -940,7 +953,7 @@ mod test_map {
940953
let mut x = VecMap::new();
941954
let mut y = VecMap::new();
942955

943-
assert!(hash::hash(&x) == hash::hash(&y));
956+
assert!(hash(&x) == hash(&y));
944957
x.insert(1, 'a');
945958
x.insert(2, 'b');
946959
x.insert(3, 'c');
@@ -949,7 +962,12 @@ mod test_map {
949962
y.insert(2, 'b');
950963
y.insert(1, 'a');
951964

952-
assert!(hash::hash(&x) == hash::hash(&y));
965+
assert!(hash(&x) == hash(&y));
966+
967+
x.insert(1000, 'd');
968+
x.remove(&1000);
969+
970+
assert!(hash(&x) == hash(&y));
953971
}
954972

955973
#[test]

0 commit comments

Comments
 (0)