Use more deterministic data structures

src/date.rs

@@ -6,7 +6,7 @@ use ast::{DatePattern, DateToken, show_datepattern};
 use chrono::format::Parsed;
 use chrono::{Weekday, DateTime, UTC, FixedOffset, Duration, Date};
 use eval::Context;
-use number::Number;
+use number::{Number, Dim};
 use std::iter::Peekable;
 
 pub fn parse_date<I>(
@@ -225,7 +225,7 @@ pub fn to_duration(num: &Number) -> Result<Duration, String> {
     use gmp::mpq::Mpq;
     use gmp::mpz::Mpz;
 
-    if num.1.len() != 1 || num.1.get(&"s".to_owned()) != Some(&1) {
+    if num.1.len() != 1 || num.1.get("s") != Some(&1) {
         return Err(format!("Expected seconds"))
     }
     let max = Mpq::ratio(&Mpz::from(i64::max_value() / 1000), &Mpz::one());
@@ -239,12 +239,11 @@ pub fn to_duration(num: &Number) -> Result<Duration, String> {
 pub fn from_duration(duration: &Duration) -> Result<Number, String> {
     use gmp::mpq::Mpq;
     use gmp::mpz::Mpz;
-    use std::rc::Rc;
 
     let ns = try!(duration.num_nanoseconds()
                   .ok_or(format!("Implementation error: Duration is out of range")));
     let div = Mpz::from(1_000_000_000);
-    Ok(Number::new_unit(Mpq::ratio(&Mpz::from(ns), &div), Rc::new("s".to_owned())))
+    Ok(Number::new_unit(Mpq::ratio(&Mpz::from(ns), &div), Dim::new("s")))
 }
 
 pub fn now() -> DateTime<FixedOffset> {

src/eval.rs

@@ -2,11 +2,10 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use std::collections::HashMap;
-use std::collections::BTreeMap;
+use std::collections::{HashMap, BTreeMap, BTreeSet};
 use gmp::mpq::Mpq;
 use chrono::{DateTime, FixedOffset};
-use number::{Number, Unit};
+use number::{Number, Unit, Dim};
 use date;
 use ast::{DatePattern, Expr, SuffixOp, Def, Defs, Query, Conversion};
 use std::ops::{Add, Div, Mul, Neg, Sub};
@@ -22,10 +21,10 @@ pub enum Value {
 /// The evaluation context that contains unit definitions.
 #[derive(Debug)]
 pub struct Context {
-    pub dimensions: Vec<Rc<String>>,
+    pub dimensions: BTreeSet<Dim>,
     pub units: HashMap<String, Number>,
     pub aliases: HashMap<Unit, String>,
-    pub reverse: HashMap<Unit, String>,
+    pub reverse: BTreeMap<Unit, String>,
     pub prefixes: Vec<(String, Number)>,
     pub definitions: HashMap<String, Expr>,
     pub datepatterns: Vec<Vec<DatePattern>>,
@@ -161,10 +160,8 @@ impl Context {
     /// Given a unit name, returns its value if it exists. Supports SI
     /// prefixes, plurals, bare dimensions like length, and aliases.
     pub fn lookup(&self, name: &str) -> Option<Number> {
-        for k in &self.dimensions {
-            if name == &***k {
-                return Some(Number::one_unit(k.to_owned()))
-            }
+        if let Some(k) = self.dimensions.get(name) {
+            return Some(Number::one_unit(k.to_owned()))
         }
         if let Some(v) = self.units.get(name).cloned() {
             return Some(v)
@@ -191,10 +188,8 @@ impl Context {
 
     /// Given a unit name, try to return a canonical name (expanding aliases and such)
     pub fn canonicalize(&self, name: &str) -> Option<String> {
-        for k in &self.dimensions {
-            if name == &***k {
-                return Some((**k).clone())
-            }
+        if let Some(k) = self.dimensions.get(name) {
+            return Some((*k.0).clone())
         }
         if let Some(v) = self.definitions.get(name) {
             if let Expr::Unit(ref name) = *v {
@@ -337,7 +332,7 @@ impl Context {
         match *expr {
             Expr::Unit(ref name) if name == "now" => Ok(Value::DateTime(date::now())),
             Expr::Unit(ref name) => self.lookup(name).ok_or(format!("Unknown unit {}", name)).map(Value::Number),
-            Expr::Quote(ref name) => Ok(Value::Number(Number::one_unit(Rc::new(name.clone())))),
+            Expr::Quote(ref name) => Ok(Value::Number(Number::one_unit(Dim::new(&**name)))),
             Expr::Const(ref num, ref frac, ref exp) =>
                 Number::from_parts(
                     num,
@@ -775,10 +770,10 @@ impl Context {
     /// Creates a new, empty context
     pub fn new() -> Context {
         Context {
-            dimensions: Vec::new(),
+            dimensions: BTreeSet::new(),
             units: HashMap::new(),
             aliases: HashMap::new(),
-            reverse: HashMap::new(),
+            reverse: BTreeMap::new(),
             prefixes: Vec::new(),
             definitions: HashMap::new(),
             datepatterns: Vec::new(),
@@ -958,8 +953,7 @@ impl Context {
             };
             match *def {
                 Def::Dimension(ref dname) => {
-                    let dname = Rc::new(dname.clone());
-                    self.dimensions.push(dname.clone());
+                    self.dimensions.insert(Dim::new(&**dname));
                 },
                 Def::Unit(ref expr) => match self.eval(expr) {
                     Ok(Value::Number(v)) => {

src/factorize.rs

@@ -2,9 +2,9 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use std::collections::{BTreeMap, BinaryHeap, HashMap};
+use std::collections::{BTreeMap, BinaryHeap};
 use std::rc::Rc;
-use number::{Number, Unit};
+use number::{Number, Unit, Dim};
 use std::cmp;
 use gmp::mpq::Mpq;
 
@@ -23,7 +23,7 @@ impl cmp::Ord for Factors {
     }
 }
 
-pub fn fast_decompose(value: &Number, aliases: &HashMap<Unit, String>) -> Unit {
+pub fn fast_decompose(value: &Number, aliases: &BTreeMap<Unit, String>) -> Unit {
     let mut best = None;
     for (unit, name) in aliases.iter() {
         let num = Number(Mpq::one(), unit.clone());
@@ -39,7 +39,7 @@ pub fn fast_decompose(value: &Number, aliases: &HashMap<Unit, String>) -> Unit {
     if let Some((name, unit, pow, score)) = best {
         if score < value.complexity_score() {
             let mut res = (value / &Number(Mpq::one(), unit.clone()).powi(pow)).unwrap().1;
-            res.insert(Rc::new(name.clone()), pow as i64);
+            res.insert(Dim::new(&**name), pow as i64);
             return res
         }
     }

src/number.rs

@@ -9,18 +9,39 @@ use eval::Show;
 use std::ops::{Add, Div, Mul, Neg, Sub};
 use std::rc::Rc;
 use std::fmt;
+use std::borrow::Borrow;
 
 /// Number type
 pub type Num = Mpq;
-/// A simple alias to add semantic meaning for when we pass around dimension IDs.
-pub type Dim = Rc<String>;
 /// Alias for the primary representation of dimensionality.
 pub type Unit = BTreeMap<Dim, i64>;
 
+/// A newtype for a string dimension ID, so that we can implement traits for it.
+#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
+pub struct Dim(pub Rc<String>);
+
 /// The basic representation of a number with a unit.
 #[derive(Clone, PartialEq, Eq)]
 pub struct Number(pub Num, pub Unit);
 
+impl Borrow<str> for Dim {
+    fn borrow(&self) -> &str {
+        &**self.0
+    }
+}
+
+impl fmt::Display for Dim {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        self.0.fmt(fmt)
+    }
+}
+
+impl Dim {
+    pub fn new(dim: &str) -> Dim {
+        Dim(Rc::new(dim.to_owned()))
+    }
+}
+
 fn one() -> Mpq {
     Mpq::one()
 }
@@ -320,7 +341,7 @@ impl Show for Number {
                 let e = value.1.iter().next().unwrap();
                 let ref n = *e.0;
                 if *e.1 == 1 {
-                    Some((**n).clone())
+                    Some((&*n.0).clone())
                 } else {
                     Some(format!("{}^{}", n, e.1))
                 }

tests/query.rs

@@ -16,9 +16,26 @@ fn test(input: &str, output: &str) {
 }
 
 #[test]
-fn test_queries() {
+fn test_definition() {
     test("watt", "Definition: watt = J / s = 1 watt (power; kg m^2 / s^3)");
+}
+
+#[test]
+fn test_eval() {
     test("5 inch", "0.127 m (length)");
-    test("5 inch -> cm", "12.7 cm (length)");
+}
+
+#[test]
+fn test_convert() {
+    test("5 inch -> cm", "12.7 centim (length)");
+}
+
+#[test]
+fn test_temp() {
     test("2 degC 2 -> degC", "277.15 °C (temperature)");
 }
+
+#[test]
+fn test_determinism() {
+    test("pascal m", "1 A tesla (spectral_irradiance_frequency)");
+}