modernize smallintmap

* switch to explicit self
* get rid of the @ box
* replace DVec with ~[] (to get rid of the mutable field)
* implement the new container::Map trait

Author: thestinger

src/libstd/smallintmap.rs

@@ -15,77 +15,20 @@
 #[forbid(deprecated_mode)];
 
 use core::container::{Container, Mutable, Map, Set};
-use core::dvec::DVec;
-use core::ops;
 use core::option::{Some, None};
-use core::option;
 use core::prelude::*;
 
-// FIXME (#2347): Should not be @; there's a bug somewhere in rustc that
-// requires this to be.
-struct SmallIntMap_<T> {
-    v: DVec<Option<T>>,
-}
-
-pub enum SmallIntMap<T> {
-    SmallIntMap_(@SmallIntMap_<T>)
-}
-
-/// Create a smallintmap
-pub fn mk<T: Copy>() -> SmallIntMap<T> {
-    let v = DVec();
-    SmallIntMap_(@SmallIntMap_ { v: v } )
-}
-
-/**
- * Add a value to the map. If the map already contains a value for
- * the specified key then the original value is replaced.
- */
-#[inline(always)]
-pub fn insert<T: Copy>(self: SmallIntMap<T>, key: uint, val: T) {
-    //io::println(fmt!("%?", key));
-    self.v.grow_set_elt(key, &None, Some(val));
-}
-
-/**
- * Get the value for the specified key. If the key does not exist
- * in the map then returns none
- */
-pub pure fn find<T: Copy>(self: SmallIntMap<T>, key: uint) -> Option<T> {
-    if key < self.v.len() { return self.v.get_elt(key); }
-    return None::<T>;
-}
-
-/**
- * Get the value for the specified key
- *
- * # Failure
- *
- * If the key does not exist in the map
- */
-pub pure fn get<T: Copy>(self: SmallIntMap<T>, key: uint) -> T {
-    match find(self, key) {
-      None => {
-        error!("smallintmap::get(): key not present");
-        fail;
-      }
-      Some(move v) => return v
-    }
-}
-
-/// Returns true if the map contains a value for the specified key
-pub pure fn contains_key<T: Copy>(self: SmallIntMap<T>, key: uint) -> bool {
-    return !find(self, key).is_none();
+pub struct SmallIntMap<T> {
+    priv v: ~[Option<T>],
 }
 
 impl<V> SmallIntMap<V>: Container {
     /// Return the number of elements in the map
     pure fn len(&self) -> uint {
-        let mut sz = 0u;
+        let mut sz = 0;
         for self.v.each |item| {
-            match *item {
-              Some(_) => sz += 1u,
-              _ => ()
+            if item.is_some() {
+                sz += 1;
             }
         }
         sz
@@ -96,118 +39,136 @@ impl<V> SmallIntMap<V>: Container {
 }
 
 impl<V> SmallIntMap<V>: Mutable {
-    fn clear(&mut self) { self.v.set(~[]) }
+    /// Clear the map, removing all key-value pairs.
+    fn clear(&mut self) { self.v.clear() }
 }
 
-/// Implements the map::map interface for smallintmap
-impl<V: Copy> SmallIntMap<V> {
-    #[inline(always)]
-    fn insert(key: uint, value: V) -> bool {
-        let exists = contains_key(self, key);
-        insert(self, key, value);
-        return !exists;
+impl<V> SmallIntMap<V>: Map<uint, V> {
+    /// Return true if the map contains a value for the specified key
+    pure fn contains_key(&self, key: &uint) -> bool {
+        self.find(key).is_some()
     }
-    fn remove(key: uint) -> bool {
-        if key >= self.v.len() {
-            return false;
+
+    /// Visit all key-value pairs
+    pure fn each(&self, it: fn(key: &uint, value: &V) -> bool) {
+        for uint::range(0, self.v.len()) |i| {
+            match self.v[i] {
+              Some(ref elt) => if !it(&i, elt) { break },
+              None => ()
+            }
         }
-        let old = self.v.get_elt(key);
-        self.v.set_elt(key, None);
-        old.is_some()
     }
-    pure fn contains_key(key: uint) -> bool {
-        contains_key(self, key)
+
+    /// Visit all keys
+    pure fn each_key(&self, blk: fn(key: &uint) -> bool) {
+        self.each(|k, _| blk(k))
     }
-    pure fn contains_key_ref(key: &uint) -> bool {
-        contains_key(self, *key)
+
+    /// Visit all values
+    pure fn each_value(&self, blk: fn(value: &V) -> bool) {
+        self.each(|_, v| blk(v))
     }
-    pure fn get(key: uint) -> V { get(self, key) }
-    pure fn find(key: uint) -> Option<V> { find(self, key) }
 
-    fn update_with_key(key: uint, val: V, ff: fn(uint, V, V) -> V) -> bool {
-        match self.find(key) {
-            None            => return self.insert(key, val),
-            Some(copy orig) => return self.insert(key, ff(key, orig, val)),
+    /// Return the value corresponding to the key in the map
+    pure fn find(&self, key: &uint) -> Option<&self/V> {
+        if *key < self.v.len() {
+            match self.v[*key] {
+              Some(ref value) => Some(value),
+              None => None
+            }
+        } else {
+            None
         }
     }
 
-    fn update(key: uint, newval: V, ff: fn(V, V) -> V) -> bool {
-        return self.update_with_key(key, newval, |_k, v, v1| ff(v,v1));
+    /// Insert a key-value pair into the map. An existing value for a
+    /// key is replaced by the new value. Return true if the key did
+    /// not already exist in the map.
+    fn insert(&mut self, key: uint, value: V) -> bool {
+        let exists = self.contains_key(&key);
+        let len = self.v.len();
+        if len <= key {
+            vec::grow_fn(&mut self.v, key - len + 1, |_| None);
+        }
+        self.v[key] = Some(value);
+        !exists
     }
 
-    pure fn each(it: fn(key: uint, value: V) -> bool) {
-        self.each_ref(|k, v| it(*k, *v))
-    }
-    pure fn each_key(it: fn(key: uint) -> bool) {
-        self.each_ref(|k, _v| it(*k))
-    }
-    pure fn each_value(it: fn(value: V) -> bool) {
-        self.each_ref(|_k, v| it(*v))
-    }
-    pure fn each_ref(it: fn(key: &uint, value: &V) -> bool) {
-        let mut idx = 0u, l = self.v.len();
-        while idx < l {
-            match self.v.get_elt(idx) {
-              Some(ref elt) => if !it(&idx, elt) { break },
-              None => ()
-            }
-            idx += 1u;
+    /// Remove a key-value pair from the map. Return true if the key
+    /// was present in the map, otherwise false.
+    fn remove(&mut self, key: &uint) -> bool {
+        if *key >= self.v.len() {
+            return false;
         }
+        let removed = self.v[*key].is_some();
+        self.v[*key] = None;
+        removed
     }
-    pure fn each_key_ref(blk: fn(key: &uint) -> bool) {
-        self.each_ref(|k, _v| blk(k))
-    }
-    pure fn each_value_ref(blk: fn(value: &V) -> bool) {
-        self.each_ref(|_k, v| blk(v))
+}
+
+pub impl<V> SmallIntMap<V> {
+    /// Create an empty SmallIntMap
+    static pure fn new() -> SmallIntMap<V> { SmallIntMap{v: ~[]} }
+
+    pure fn get(&self, key: &uint) -> &self/V {
+        self.find(key).expect("key not present")
     }
 }
 
-impl<V: Copy> SmallIntMap<V>: ops::Index<uint, V> {
-    pure fn index(&self, key: uint) -> V {
-        unsafe {
-            get(*self, key)
+pub impl<V: Copy> SmallIntMap<V> {
+    // FIXME: #4733, remove after the next snapshot
+    #[cfg(stage2)]
+    fn update_with_key(&mut self, key: uint, val: V,
+                       ff: fn(uint, V, V) -> V) -> bool {
+        match self.find(&key) {
+          None => self.insert(key, val),
+          Some(orig) => self.insert(key, ff(key, copy *orig, val)),
         }
     }
+
+    // FIXME: #4733, remove after the next snapshot
+    #[cfg(stage2)]
+    fn update(&mut self, key: uint, newval: V, ff: fn(V, V) -> V) -> bool {
+        self.update_with_key(key, newval, |_k, v, v1| ff(v,v1))
+    }
 }
 
 #[cfg(test)]
 mod tests {
-    use super::{mk, SmallIntMap};
-
-    use core::option::None;
+    use super::SmallIntMap;
 
     #[test]
     fn test_len() {
-        let mut map = mk();
+        let mut map = SmallIntMap::new();
         assert map.len() == 0;
         assert map.is_empty();
-        map.insert(5, 20);
+        assert map.insert(5, 20);
         assert map.len() == 1;
         assert !map.is_empty();
-        map.insert(11, 12);
+        assert map.insert(11, 12);
         assert map.len() == 2;
         assert !map.is_empty();
-        map.insert(14, 22);
+        assert map.insert(14, 22);
         assert map.len() == 3;
         assert !map.is_empty();
     }
 
     #[test]
     fn test_clear() {
-        let mut map = mk();
-        map.insert(5, 20);
-        map.insert(11, 12);
-        map.insert(14, 22);
+        let mut map = SmallIntMap::new();
+        assert map.insert(5, 20);
+        assert map.insert(11, 12);
+        assert map.insert(14, 22);
         map.clear();
         assert map.is_empty();
-        assert map.find(5).is_none();
-        assert map.find(11).is_none();
-        assert map.find(14).is_none();
+        assert map.find(&5).is_none();
+        assert map.find(&11).is_none();
+        assert map.find(&14).is_none();
     }
 
     #[test]
     fn test_insert_with_key() {
-        let map: SmallIntMap<uint> = mk();
+        let mut map = SmallIntMap::new();
 
         // given a new key, initialize it with this new count, given
         // given an existing key, add more to its count
@@ -227,11 +188,11 @@ mod tests {
         map.update_with_key(3, 2, addMoreToCount);
 
         // check the total counts
-        assert map.find(3).get() == 10;
-        assert map.find(5).get() == 3;
-        assert map.find(9).get() == 1;
+        assert map.find(&3).get() == &10;
+        assert map.find(&5).get() == &3;
+        assert map.find(&9).get() == &1;
 
         // sadly, no sevens were counted
-        assert None == map.find(7);
+        assert map.find(&7).is_none();
     }
 }

src/libstd/std.rc

@@ -82,6 +82,7 @@ pub mod list;
 pub mod map;
 pub mod priority_queue;
 pub mod rope;
+pub mod smallintmap;
 pub mod oldsmallintmap;
 pub mod sort;
 pub mod treemap;

src/test/bench/std-smallintmap.rs

@@ -8,22 +8,21 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// Microbenchmark for the oldsmallintmap library
+// Microbenchmark for the smallintmap library
 
 extern mod std;
-use std::oldsmallintmap;
-use std::oldsmallintmap::SmallIntMap;
+use std::smallintmap::SmallIntMap;
 use io::WriterUtil;
 
-fn append_sequential(min: uint, max: uint, map: SmallIntMap<uint>) {
+fn append_sequential(min: uint, max: uint, map: &mut SmallIntMap<uint>) {
     for uint::range(min, max) |i| {
         map.insert(i, i + 22u);
     }
 }
 
-fn check_sequential(min: uint, max: uint, map: SmallIntMap<uint>) {
+fn check_sequential(min: uint, max: uint, map: &SmallIntMap<uint>) {
     for uint::range(min, max) |i| {
-        assert map.get(i) == i + 22u;
+        assert *map.get(&i) == i + 22u;
     }
 }
 
@@ -43,11 +42,11 @@ fn main() {
     let mut appendf = 0.0;
 
     for uint::range(0u, rep) |_r| {
-        let map = oldsmallintmap::mk();
+        let mut map = SmallIntMap::new();
         let start = std::time::precise_time_s();
-        append_sequential(0u, max, map);
+        append_sequential(0u, max, &mut map);
         let mid = std::time::precise_time_s();
-        check_sequential(0u, max, map);
+        check_sequential(0u, max, &map);
         let end = std::time::precise_time_s();
 
         checkf += (end - mid) as float;