From 07228a104109b00ad3ec553a33eb6b496fc97451 Mon Sep 17 00:00:00 2001 From: mcarton Date: Fri, 12 Feb 2016 15:51:55 +0100 Subject: [PATCH] Fix `Hash` implementation for `Constant` --- src/consts.rs | 93 +++++++++++++++++++++++++++++++------------------ tests/consts.rs | 10 +++++- 2 files changed, 68 insertions(+), 35 deletions(-) diff --git a/src/consts.rs b/src/consts.rs index 4469853dd..416ec8279 100644 --- a/src/consts.rs +++ b/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 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::(), rs.parse::()) { + (Ok(l), Ok(r)) => l.eq(&r), _ => false, - } { - match (ls.parse::(), rs.parse::()) { - (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(&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::() { + unsafe { mem::transmute::(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 { 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::(), rs.parse::()) { - (Ok(ref l), Ok(ref r)) => l.partial_cmp(r), - _ => None, - } - } else { - None + (&Constant::Float(ref ls, _), &Constant::Float(ref rs, _)) => { + match (ls.parse::(), rs.parse::()) { + (Ok(ref l), Ok(ref r)) => l.partial_cmp(r), + _ => None, } } (&Constant::Bool(ref l), &Constant::Bool(ref r)) => Some(l.cmp(r)), diff --git a/tests/consts.rs b/tests/consts.rs index 75082e646..ab8636b81 100644 --- a/tests/consts.rs +++ b/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) -> Spanned { 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 }