Merge #311

311: rational: make sure Hash agrees with Eq r=cuviper

We can't use a derived `Hash` when we have a manual `Eq`, because we
need to uphold the invariant `a == b` → `h(a) == h(b)`.  Since `Eq`
doesn't require them to be in reduced form, `Hash` also needs to be
normalized.

Fixes #310.

rational/src/lib.rs

@@ -27,8 +27,7 @@ extern crate num_integer as integer;
 use std::cmp;
 use std::error::Error;
 use std::fmt;
-#[cfg(test)]
+use std::hash::{Hash, Hasher};
-use std::hash;
 use std::ops::{Add, Div, Mul, Neg, Rem, Sub};
 use std::str::FromStr;
 
@@ -39,7 +38,7 @@ use integer::Integer;
 use traits::{FromPrimitive, Float, PrimInt, Num, Signed, Zero, One, Bounded, NumCast};
 
 /// Represents the ratio between 2 numbers.
-#[derive(Copy, Clone, Hash, Debug)]
+#[derive(Copy, Clone, Debug)]
 #[cfg_attr(feature = "rustc-serialize", derive(RustcEncodable, RustcDecodable))]
 #[allow(missing_docs)]
 pub struct Ratio<T> {
@@ -347,6 +346,24 @@ impl<T: Clone + Integer> PartialEq for Ratio<T> {
 
 impl<T: Clone + Integer> Eq for Ratio<T> {}
 
+// NB: We can't just `#[derive(Hash)]`, because it needs to agree
+// with `Eq` even for non-reduced ratios.
+impl<T: Clone + Integer + Hash> Hash for Ratio<T> {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        recurse(&self.numer, &self.denom, state);
+
+        fn recurse<T: Integer + Hash, H: Hasher>(numer: &T, denom: &T, state: &mut H) {
+            if !denom.is_zero() {
+                let (int, rem) = numer.div_mod_floor(denom);
+                int.hash(state);
+                recurse(denom, &rem, state);
+            } else {
+                denom.hash(state);
+            }
+        }
+    }
+}
+
 
 macro_rules! forward_val_val_binop {
     (impl $imp:ident, $method:ident) => {
@@ -843,8 +860,8 @@ fn approximate_float_unsigned<T, F>(val: F, max_error: F, max_iterations: usize)
 }
 
 #[cfg(test)]
-fn hash<T: hash::Hash>(x: &T) -> u64 {
+fn hash<T: Hash>(x: &T) -> u64 {
-    use std::hash::{BuildHasher, Hasher};
+    use std::hash::BuildHasher;
     use std::collections::hash_map::RandomState;
     let mut hasher = <RandomState as BuildHasher>::Hasher::new();
     x.hash(&mut hasher);
@@ -1366,6 +1383,17 @@ mod test {
     fn test_hash() {
         assert!(::hash(&_0) != ::hash(&_1));
         assert!(::hash(&_0) != ::hash(&_3_2));
+
+        // a == b -> hash(a) == hash(b)
+        let a = Rational::new_raw(4, 2);
+        let b = Rational::new_raw(6, 3);
+        assert_eq!(a, b);
+        assert_eq!(::hash(&a), ::hash(&b));
+
+        let a = Rational::new_raw(123456789, 1000);
+        let b = Rational::new_raw(123456789 * 5, 5000);
+        assert_eq!(a, b);
+        assert_eq!(::hash(&a), ::hash(&b));
     }
 
     #[test]