Fix Hash implementation for Constant

Author: mcarton

src/consts.rs

@@ -2,22 +2,21 @@
 
 use rustc::lint::LateContext;
 use rustc::middle::const_eval::lookup_const_by_id;
-use rustc::middle::def::PathResolution;
-use rustc::middle::def::Def;
+use rustc::middle::def::{Def, PathResolution};
 use rustc_front::hir::*;
-use syntax::ptr::P;
-use std::cmp::PartialOrd;
 use std::cmp::Ordering::{self, Greater, Less, Equal};
-use std::rc::Rc;
+use std::cmp::PartialOrd;
+use std::hash::{Hash, Hasher};
+use std::mem;
 use std::ops::Deref;
-
-use syntax::ast::Lit_;
-use syntax::ast::LitIntType;
-use syntax::ast::{UintTy, FloatTy, StrStyle};
+use std::rc::Rc;
+use syntax::ast::{LitIntType, Lit_};
 use syntax::ast::Sign::{self, Plus, Minus};
+use syntax::ast::{UintTy, FloatTy, StrStyle};
+use syntax::ptr::P;
 
 
-#[derive(PartialEq, Eq, Debug, Copy, Clone, Hash)]
+#[derive(Debug, Copy, Clone)]
 pub enum FloatWidth {
     Fw32,
     Fw64,
@@ -34,7 +33,7 @@ impl From<FloatTy> for FloatWidth {
 }
 
 /// a Lit_-like enum to fold constant `Expr`s into
-#[derive(Eq, Debug, Clone, Hash)]
+#[derive(Debug, Clone)]
 pub enum Constant {
     /// a String "abc"
     Str(String, StrStyle),
@@ -100,18 +99,12 @@ impl PartialEq for Constant {
             (&Constant::Int(lv, lty), &Constant::Int(rv, rty)) => {
                 lv == rv && (is_negative(lty) & (lv != 0)) == (is_negative(rty) & (rv != 0))
             }
-            (&Constant::Float(ref ls, lw), &Constant::Float(ref rs, rw)) => {
-                use self::FloatWidth::*;
-                if match (lw, rw) {
-                    (FwAny, _) | (_, FwAny) | (Fw32, Fw32) | (Fw64, Fw64) => true,
+            (&Constant::Float(ref ls, _), &Constant::Float(ref rs, _)) => {
+                // we want `Fw32 == FwAny` and `FwAny == Fw64`, by transitivity we must have
+                // `Fw32 == Fw64` so don’t compare them
+                match (ls.parse::<f64>(), rs.parse::<f64>()) {
+                    (Ok(l), Ok(r)) => l.eq(&r),
                     _ => false,
-                } {
-                    match (ls.parse::<f64>(), rs.parse::<f64>()) {
-                        (Ok(l), Ok(r)) => l.eq(&r),
-                        _ => false,
-                    }
-                } else {
-                    false
                 }
             }
             (&Constant::Bool(l), &Constant::Bool(r)) => l == r,
@@ -123,6 +116,46 @@ impl PartialEq for Constant {
     }
 }
 
+impl Hash for Constant {
+    fn hash<H>(&self, state: &mut H) where H: Hasher {
+        match *self {
+            Constant::Str(ref s, ref k) => {
+                s.hash(state);
+                k.hash(state);
+            }
+            Constant::Binary(ref b) => {
+                b.hash(state);
+            }
+            Constant::Byte(u) => {
+                u.hash(state);
+            }
+            Constant::Char(c) => {
+                c.hash(state);
+            }
+            Constant::Int(u, t) => {
+                u.hash(state);
+                t.hash(state);
+            }
+            Constant::Float(ref f, _) => {
+                // don’t use the width here because of PartialEq implementation
+                if let Ok(f) = f.parse::<f64>() {
+                    unsafe { mem::transmute::<f64, u64>(f) }.hash(state);
+                }
+            }
+            Constant::Bool(b) => {
+                b.hash(state);
+            }
+            Constant::Vec(ref v) | Constant::Tuple(ref v)=> {
+                v.hash(state);
+            }
+            Constant::Repeat(ref c, l) => {
+                c.hash(state);
+                l.hash(state);
+            }
+        }
+    }
+}
+
 impl PartialOrd for Constant {
     fn partial_cmp(&self, other: &Constant) -> Option<Ordering> {
         match (self, other) {
@@ -143,18 +176,10 @@ impl PartialOrd for Constant {
                     (false, true) => Greater,
                 })
             }
-            (&Constant::Float(ref ls, lw), &Constant::Float(ref rs, rw)) => {
-                use self::FloatWidth::*;
-                if match (lw, rw) {
-                    (FwAny, _) | (_, FwAny) | (Fw32, Fw32) | (Fw64, Fw64) => true,
-                    _ => false,
-                } {
-                    match (ls.parse::<f64>(), rs.parse::<f64>()) {
-                        (Ok(ref l), Ok(ref r)) => l.partial_cmp(r),
-                        _ => None,
-                    }
-                } else {
-                    None
+            (&Constant::Float(ref ls, _), &Constant::Float(ref rs, _)) => {
+                match (ls.parse::<f64>(), rs.parse::<f64>()) {
+                    (Ok(ref l), Ok(ref r)) => l.partial_cmp(r),
+                    _ => None,
                 }
             }
             (&Constant::Bool(ref l), &Constant::Bool(ref r)) => Some(l.cmp(r)),

tests/consts.rs

@@ -17,7 +17,7 @@ use syntax::ast::LitIntType::*;
 use syntax::ast::StrStyle::*;
 use syntax::ast::Sign::*;
 
-use clippy::consts::{constant_simple, Constant};
+use clippy::consts::{constant_simple, Constant, FloatWidth};
 
 fn spanned<T>(t: T) -> Spanned<T> {
     Spanned{ node: t, span: COMMAND_LINE_SP }
@@ -78,4 +78,12 @@ fn test_ops() {
     check(ONE, &binop(BiSub, litone.clone(), litzero.clone()));
     check(ONE, &binop(BiMul, litone.clone(), litone.clone()));
     check(ONE, &binop(BiDiv, litone.clone(), litone.clone()));
+
+    let half_any = Constant::Float("0.5".into(), FloatWidth::FwAny);
+    let half32 = Constant::Float("0.5".into(), FloatWidth::Fw32);
+    let half64 = Constant::Float("0.5".into(), FloatWidth::Fw64);
+
+    assert_eq!(half_any, half32);
+    assert_eq!(half_any, half64);
+    assert_eq!(half32, half64); // for transitivity
 }