From 21e783d3b605354ec508b3395e7d3429b4b24075 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sat, 12 May 2018 13:41:03 +0200 Subject: [PATCH 1/3] Add run-pass tests for SpanlessEq/SpanlessHash ICE --- tests/run-pass/ice-1782.rs | 17 +++++++++++++++++ tests/run-pass/ice-2499.rs | 24 ++++++++++++++++++++++++ tests/run-pass/ice-2594.rs | 20 ++++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 tests/run-pass/ice-1782.rs create mode 100644 tests/run-pass/ice-2499.rs create mode 100644 tests/run-pass/ice-2594.rs diff --git a/tests/run-pass/ice-1782.rs b/tests/run-pass/ice-1782.rs new file mode 100644 index 00000000000..fcd3e7cf530 --- /dev/null +++ b/tests/run-pass/ice-1782.rs @@ -0,0 +1,17 @@ +#![allow(dead_code, unused_variables)] + +/// Should not trigger an ICE in `SpanlessEq` / `consts::constant` +/// +/// Issue: https://github.com/rust-lang-nursery/rust-clippy/issues/1782 + +use std::{mem, ptr}; + +fn spanless_eq_ice() { + let txt = "something"; + match txt { + "something" => unsafe { ptr::write(ptr::null_mut() as *mut u32, mem::transmute::<[u8; 4], _>([0, 0, 0, 255])) }, + _ => unsafe { ptr::write(ptr::null_mut() as *mut u32, mem::transmute::<[u8; 4], _>([13, 246, 24, 255])) }, + } +} + +fn main() {} diff --git a/tests/run-pass/ice-2499.rs b/tests/run-pass/ice-2499.rs new file mode 100644 index 00000000000..d012d548f20 --- /dev/null +++ b/tests/run-pass/ice-2499.rs @@ -0,0 +1,24 @@ +#![allow(dead_code)] + +/// Should not trigger an ICE in `SpanlessHash` / `consts::constant` +/// +/// Issue: https://github.com/rust-lang-nursery/rust-clippy/issues/2499 + +fn f(s: &[u8]) -> bool { + let t = s[0] as char; + + match t { + 'E' | 'W' => {} + 'T' => if &s[0..(0 + 4)] != &['0' as u8; 4] { + return false; + } else { + return true; + }, + _ => { + return false; + } + } + true +} + +fn main() {} diff --git a/tests/run-pass/ice-2594.rs b/tests/run-pass/ice-2594.rs new file mode 100644 index 00000000000..7cd30b6d946 --- /dev/null +++ b/tests/run-pass/ice-2594.rs @@ -0,0 +1,20 @@ +#![allow(dead_code, unused_variables)] + +/// Should not trigger an ICE in `SpanlessHash` / `consts::constant` +/// +/// Issue: https://github.com/rust-lang-nursery/rust-clippy/issues/2594 + +fn spanless_hash_ice() { + let txt = "something"; + let empty_header: [u8; 1] = [1; 1]; + + match txt { + "something" => { + let mut headers = [empty_header; 1]; + } + "" => (), + _ => (), + } +} + +fn main() {} From 6eb07cc5b699a28f479012f5271ef9da2f60a6a3 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sun, 13 May 2018 13:16:31 +0200 Subject: [PATCH 2/3] Fix ICE for issue 2594 --- clippy_lints/src/array_indexing.rs | 6 +++--- clippy_lints/src/bit_mask.rs | 2 +- clippy_lints/src/consts.rs | 10 +++++----- clippy_lints/src/copies.rs | 4 ++-- clippy_lints/src/erasing_op.rs | 2 +- clippy_lints/src/identity_op.rs | 2 +- clippy_lints/src/loops.rs | 6 +++--- clippy_lints/src/matches.rs | 6 +++--- clippy_lints/src/methods.rs | 2 +- clippy_lints/src/minmax.rs | 6 +++--- clippy_lints/src/misc.rs | 4 ++-- clippy_lints/src/ranges.rs | 2 +- clippy_lints/src/regex.rs | 2 +- clippy_lints/src/types.rs | 4 ++-- clippy_lints/src/utils/hir_utils.rs | 13 ++++++++++--- clippy_lints/src/vec.rs | 2 +- clippy_lints/src/zero_div_zero.rs | 4 ++-- 17 files changed, 42 insertions(+), 35 deletions(-) diff --git a/clippy_lints/src/array_indexing.rs b/clippy_lints/src/array_indexing.rs index b51a9209c4c..b5a42f03e9a 100644 --- a/clippy_lints/src/array_indexing.rs +++ b/clippy_lints/src/array_indexing.rs @@ -64,7 +64,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIndexing { let size = size.assert_usize(cx.tcx).unwrap().into(); // Index is a constant uint - if let Some((Constant::Int(const_index), _)) = constant(cx, index) { + if let Some((Constant::Int(const_index), _)) = constant(cx, cx.tables, index) { if size <= const_index { utils::span_lint(cx, OUT_OF_BOUNDS_INDEXING, e.span, "const index is out of bounds"); } @@ -101,14 +101,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ArrayIndexing { /// Returns an option containing a tuple with the start and end (exclusive) of /// the range. fn to_const_range<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, range: Range, array_size: u128) -> Option<(u128, u128)> { - let s = range.start.map(|expr| constant(cx, expr).map(|(c, _)| c)); + let s = range.start.map(|expr| constant(cx, cx.tables, expr).map(|(c, _)| c)); let start = match s { Some(Some(Constant::Int(x))) => x, Some(_) => return None, None => 0, }; - let e = range.end.map(|expr| constant(cx, expr).map(|(c, _)| c)); + let e = range.end.map(|expr| constant(cx, cx.tables, expr).map(|(c, _)| c)); let end = match e { Some(Some(Constant::Int(x))) => if range.limits == RangeLimits::Closed { x + 1 diff --git a/clippy_lints/src/bit_mask.rs b/clippy_lints/src/bit_mask.rs index b6adbf1bd86..4f38fb92831 100644 --- a/clippy_lints/src/bit_mask.rs +++ b/clippy_lints/src/bit_mask.rs @@ -301,7 +301,7 @@ fn check_ineffective_gt(cx: &LateContext, span: Span, m: u128, c: u128, op: &str } fn fetch_int_literal(cx: &LateContext, lit: &Expr) -> Option { - match constant(cx, lit)?.0 { + match constant(cx, cx.tables, lit)?.0 { Constant::Int(n) => Some(n), _ => None, } diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index 3b178b02563..bc57be94cdb 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -163,10 +163,10 @@ pub fn lit_to_constant<'tcx>(lit: &LitKind, ty: Ty<'tcx>) -> Constant { } } -pub fn constant(lcx: &LateContext, e: &Expr) -> Option<(Constant, bool)> { +pub fn constant<'c, 'cc>(lcx: &LateContext<'c, 'cc>, tables: &'c ty::TypeckTables<'cc>, e: &Expr) -> Option<(Constant, bool)> { let mut cx = ConstEvalLateContext { tcx: lcx.tcx, - tables: lcx.tables, + tables, param_env: lcx.param_env, needed_resolution: false, substs: lcx.tcx.intern_substs(&[]), @@ -174,12 +174,12 @@ pub fn constant(lcx: &LateContext, e: &Expr) -> Option<(Constant, bool)> { cx.expr(e).map(|cst| (cst, cx.needed_resolution)) } -pub fn constant_simple(lcx: &LateContext, e: &Expr) -> Option { - constant(lcx, e).and_then(|(cst, res)| if res { None } else { Some(cst) }) +pub fn constant_simple<'c, 'cc>(lcx: &LateContext<'c, 'cc>, tables: &'c ty::TypeckTables<'cc>, e: &Expr) -> Option { + constant(lcx, tables, e).and_then(|(cst, res)| if res { None } else { Some(cst) }) } /// Creates a `ConstEvalLateContext` from the given `LateContext` and `TypeckTables` -pub fn constant_context<'c, 'cc>(lcx: &LateContext<'c, 'cc>, tables: &'cc ty::TypeckTables<'cc>) -> ConstEvalLateContext<'c, 'cc> { +pub fn constant_context<'c, 'cc>(lcx: &LateContext<'c, 'cc>, tables: &'c ty::TypeckTables<'cc>) -> ConstEvalLateContext<'c, 'cc> { ConstEvalLateContext { tcx: lcx.tcx, tables, diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 35c87beecef..80601fa92fa 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -151,7 +151,7 @@ fn lint_same_then_else(cx: &LateContext, blocks: &[&Block]) { /// Implementation of `IFS_SAME_COND`. fn lint_same_cond(cx: &LateContext, conds: &[&Expr]) { let hash: &Fn(&&Expr) -> u64 = &|expr| -> u64 { - let mut h = SpanlessHash::new(cx); + let mut h = SpanlessHash::new(cx, cx.tables); h.hash_expr(expr); h.finish() }; @@ -174,7 +174,7 @@ fn lint_same_cond(cx: &LateContext, conds: &[&Expr]) { fn lint_match_arms(cx: &LateContext, expr: &Expr) { if let ExprMatch(_, ref arms, MatchSource::Normal) = expr.node { let hash = |&(_, arm): &(usize, &Arm)| -> u64 { - let mut h = SpanlessHash::new(cx); + let mut h = SpanlessHash::new(cx, cx.tables); h.hash_expr(&arm.body); h.finish() }; diff --git a/clippy_lints/src/erasing_op.rs b/clippy_lints/src/erasing_op.rs index 9cd4f3ada3b..ae6e078ddae 100644 --- a/clippy_lints/src/erasing_op.rs +++ b/clippy_lints/src/erasing_op.rs @@ -50,7 +50,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ErasingOp { } fn check(cx: &LateContext, e: &Expr, span: Span) { - if let Some(Constant::Int(v)) = constant_simple(cx, e) { + if let Some(Constant::Int(v)) = constant_simple(cx, cx.tables, e) { if v == 0 { span_lint( cx, diff --git a/clippy_lints/src/identity_op.rs b/clippy_lints/src/identity_op.rs index c6b4f9f1af3..24e5f823a35 100644 --- a/clippy_lints/src/identity_op.rs +++ b/clippy_lints/src/identity_op.rs @@ -60,7 +60,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityOp { #[allow(cast_possible_wrap)] fn check(cx: &LateContext, e: &Expr, m: i8, span: Span, arg: Span) { - if let Some(Constant::Int(v)) = constant_simple(cx, e) { + if let Some(Constant::Int(v)) = constant_simple(cx, cx.tables, e) { let check = match cx.tables.expr_ty(e).sty { ty::TyInt(ity) => unsext(cx.tcx, -1i128, ity), ty::TyUint(uty) => clip(cx.tcx, !0, uty), diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index ea5ef3be478..b0f39937668 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1120,8 +1120,8 @@ fn check_for_loop_reverse_range<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arg: &'tcx }) = higher::range(cx, arg) { // ...and both sides are compile-time constant integers... - if let Some((start_idx, _)) = constant(cx, start) { - if let Some((end_idx, _)) = constant(cx, end) { + if let Some((start_idx, _)) = constant(cx, cx.tables, start) { + if let Some((end_idx, _)) = constant(cx, cx.tables, end) { // ...and the start index is greater than the end index, // this loop will never run. This is often confusing for developers // who think that this will iterate from the larger value to the @@ -2146,7 +2146,7 @@ fn path_name(e: &Expr) -> Option { } fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, block: &'tcx Block, expr: &'tcx Expr) { - if constant(cx, cond).is_some() { + if constant(cx, cx.tables, cond).is_some() { // A pure constant condition (e.g. while false) is not linted. return; } diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index be68e9b1a2f..b593c330503 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -470,8 +470,8 @@ fn all_ranges<'a, 'tcx>( [].iter() }.filter_map(|pat| { if let PatKind::Range(ref lhs, ref rhs, ref range_end) = pat.node { - let lhs = constant(cx, lhs)?.0; - let rhs = constant(cx, rhs)?.0; + let lhs = constant(cx, cx.tables, lhs)?.0; + let rhs = constant(cx, cx.tables, rhs)?.0; let rhs = match *range_end { RangeEnd::Included => Bound::Included(rhs), RangeEnd::Excluded => Bound::Excluded(rhs), @@ -480,7 +480,7 @@ fn all_ranges<'a, 'tcx>( } if let PatKind::Lit(ref value) = pat.node { - let value = constant(cx, value)?.0; + let value = constant(cx, cx.tables, value)?.0; return Some(SpannedRange { span: pat.span, node: (value.clone(), Bound::Included(value)) }); } diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index c7702e6bced..4c80fdb01c5 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -1751,7 +1751,7 @@ fn lint_chars_last_cmp_with_unwrap<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, info: & /// lint for length-1 `str`s for methods in `PATTERN_METHODS` fn lint_single_char_pattern<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, arg: &'tcx hir::Expr) { - if let Some((Constant::Str(r), _)) = constant(cx, arg) { + if let Some((Constant::Str(r), _)) = constant(cx, cx.tables, arg) { if r.len() == 1 { let c = r.chars().next().unwrap(); let snip = snippet(cx, expr.span, ".."); diff --git a/clippy_lints/src/minmax.rs b/clippy_lints/src/minmax.rs index 8c19f627f53..289c5c77ff3 100644 --- a/clippy_lints/src/minmax.rs +++ b/clippy_lints/src/minmax.rs @@ -81,14 +81,14 @@ fn fetch_const<'a>(cx: &LateContext, args: &'a [Expr], m: MinMax) -> Option<(Min if args.len() != 2 { return None; } - if let Some(c) = constant_simple(cx, &args[0]) { - if constant_simple(cx, &args[1]).is_none() { + if let Some(c) = constant_simple(cx, cx.tables, &args[0]) { + if constant_simple(cx, cx.tables, &args[1]).is_none() { // otherwise ignore Some((m, c, &args[1])) } else { None } - } else if let Some(c) = constant_simple(cx, &args[1]) { + } else if let Some(c) = constant_simple(cx, cx.tables, &args[1]) { Some((m, c, &args[0])) } else { None diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 797a871d72b..0ecd0ac1895 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -446,7 +446,7 @@ fn check_nan(cx: &LateContext, path: &Path, expr: &Expr) { } fn is_named_constant<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> bool { - if let Some((_, res)) = constant(cx, expr) { + if let Some((_, res)) = constant(cx, cx.tables, expr) { res } else { false @@ -454,7 +454,7 @@ fn is_named_constant<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> } fn is_allowed<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> bool { - match constant(cx, expr) { + match constant(cx, cx.tables, expr) { Some((Constant::F32(f), _)) => f == 0.0 || f.is_infinite(), Some((Constant::F64(f), _)) => f == 0.0 || f.is_infinite(), _ => false, diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index bc1ffc57003..5d13b3e1ae6 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -94,7 +94,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { // Range with step_by(0). if name == "step_by" && args.len() == 2 && has_step_by(cx, &args[0]) { use consts::{constant, Constant}; - if let Some((Constant::Int(0), _)) = constant(cx, &args[1]) { + if let Some((Constant::Int(0), _)) = constant(cx, cx.tables, &args[1]) { span_lint( cx, ITERATOR_STEP_BY_ZERO, diff --git a/clippy_lints/src/regex.rs b/clippy_lints/src/regex.rs index 20ccc6ccfcc..522c2b9ac29 100644 --- a/clippy_lints/src/regex.rs +++ b/clippy_lints/src/regex.rs @@ -138,7 +138,7 @@ fn str_span(base: Span, c: regex_syntax::ast::Span, offset: u16) -> Span { } fn const_str<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) -> Option { - constant(cx, e).and_then(|(c, _)| match c { + constant(cx, cx.tables, e).and_then(|(c, _)| match c { Constant::Str(s) => Some(s), _ => None, }) diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index ffc49722aad..168086d574a 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -1341,7 +1341,7 @@ fn detect_extreme_expr<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) - let ty = cx.tables.expr_ty(expr); - let cv = constant(cx, expr)?.0; + let cv = constant(cx, cx.tables, expr)?.0; let which = match (&ty.sty, cv) { (&ty::TyBool, Constant::Bool(false)) | @@ -1526,7 +1526,7 @@ fn numeric_cast_precast_bounds<'a>(cx: &LateContext, expr: &'a Expr) -> Option<( } fn node_as_const_fullint<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> Option { - let val = constant(cx, expr)?.0; + let val = constant(cx, cx.tables, expr)?.0; if let Constant::Int(const_int) = val { match cx.tables.expr_ty(expr).sty { ty::TyInt(ity) => Some(FullInt::S(sext(cx.tcx, const_int, ity))), diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 63e7757b12e..faf61f4ed9c 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -1,6 +1,7 @@ use consts::{constant_simple, constant_context}; use rustc::lint::*; use rustc::hir::*; +use rustc::ty::{TypeckTables}; use std::hash::{Hash, Hasher}; use std::collections::hash_map::DefaultHasher; use syntax::ast::Name; @@ -16,6 +17,7 @@ use utils::differing_macro_contexts; pub struct SpanlessEq<'a, 'tcx: 'a> { /// Context used to evaluate constant expressions. cx: &'a LateContext<'a, 'tcx>, + tables: &'a TypeckTables<'tcx>, /// If is true, never consider as equal expressions containing function /// calls. ignore_fn: bool, @@ -25,6 +27,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { pub fn new(cx: &'a LateContext<'a, 'tcx>) -> Self { Self { cx, + tables: cx.tables, ignore_fn: false, } } @@ -32,6 +35,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { pub fn ignore_fn(self) -> Self { Self { cx: self.cx, + tables: self.cx.tables, ignore_fn: true, } } @@ -64,7 +68,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { return false; } - if let (Some(l), Some(r)) = (constant_simple(self.cx, left), constant_simple(self.cx, right)) { + if let (Some(l), Some(r)) = (constant_simple(self.cx, self.tables, left), constant_simple(self.cx, self.tables, right)) { if l == r { return true; } @@ -288,13 +292,15 @@ where pub struct SpanlessHash<'a, 'tcx: 'a> { /// Context used to evaluate constant expressions. cx: &'a LateContext<'a, 'tcx>, + tables: &'a TypeckTables<'tcx>, s: DefaultHasher, } impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> { - pub fn new(cx: &'a LateContext<'a, 'tcx>) -> Self { + pub fn new(cx: &'a LateContext<'a, 'tcx>, tables: &'a TypeckTables<'tcx>) -> Self { Self { cx, + tables, s: DefaultHasher::new(), } } @@ -317,7 +323,7 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> { #[allow(many_single_char_names)] pub fn hash_expr(&mut self, e: &Expr) { - if let Some(e) = constant_simple(self.cx, e) { + if let Some(e) = constant_simple(self.cx, self.tables, e) { return e.hash(&mut self.s); } @@ -461,6 +467,7 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> { let c: fn(_, _) -> _ = ExprRepeat; c.hash(&mut self.s); self.hash_expr(e); + self.tables = self.cx.tcx.body_tables(l_id); self.hash_expr(&self.cx.tcx.hir.body(l_id).value); }, ExprRet(ref e) => { diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index d5ed4cb712d..6aff0ebc9f7 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -66,7 +66,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { fn check_vec_macro<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, vec_args: &higher::VecArgs<'tcx>, span: Span) { let snippet = match *vec_args { higher::VecArgs::Repeat(elem, len) => { - if constant(cx, len).is_some() { + if constant(cx, cx.tables, len).is_some() { format!("&[{}; {}]", snippet(cx, elem.span, "elem"), snippet(cx, len.span, "len")) } else { return; diff --git a/clippy_lints/src/zero_div_zero.rs b/clippy_lints/src/zero_div_zero.rs index 16c12702c6c..fc28815c70e 100644 --- a/clippy_lints/src/zero_div_zero.rs +++ b/clippy_lints/src/zero_div_zero.rs @@ -37,8 +37,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { // TODO - constant_simple does not fold many operations involving floats. // That's probably fine for this lint - it's pretty unlikely that someone would // do something like 0.0/(2.0 - 2.0), but it would be nice to warn on that case too. - if let Some(lhs_value) = constant_simple(cx, left); - if let Some(rhs_value) = constant_simple(cx, right); + if let Some(lhs_value) = constant_simple(cx, cx.tables, left); + if let Some(rhs_value) = constant_simple(cx, cx.tables, right); if Constant::F32(0.0) == lhs_value || Constant::F64(0.0) == lhs_value; if Constant::F32(0.0) == rhs_value || Constant::F64(0.0) == rhs_value; then { From ed885dc2b320e26f47b15ef50f442e4e40cce954 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Thu, 17 May 2018 20:17:21 +0200 Subject: [PATCH 3/3] Fix ICE for issues 2767, 2499, 1782 --- clippy_lints/src/double_comparison.rs | 2 +- clippy_lints/src/utils/hir_utils.rs | 43 +++++++++++++++++---------- tests/run-pass/ice-2499.rs | 4 +-- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/double_comparison.rs b/clippy_lints/src/double_comparison.rs index 06d0cf1d09e..1ccc5708185 100644 --- a/clippy_lints/src/double_comparison.rs +++ b/clippy_lints/src/double_comparison.rs @@ -52,7 +52,7 @@ impl<'a, 'tcx> DoubleComparisonPass { } _ => return, }; - let spanless_eq = SpanlessEq::new(cx).ignore_fn(); + let mut spanless_eq = SpanlessEq::new(cx).ignore_fn(); if !(spanless_eq.eq_expr(&llhs, &rlhs) && spanless_eq.eq_expr(&lrhs, &rrhs)) { return; } diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index faf61f4ed9c..7d9945cdddc 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -41,7 +41,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { } /// Check whether two statements are the same. - pub fn eq_stmt(&self, left: &Stmt, right: &Stmt) -> bool { + pub fn eq_stmt(&mut self, left: &Stmt, right: &Stmt) -> bool { match (&left.node, &right.node) { (&StmtDecl(ref l, _), &StmtDecl(ref r, _)) => { if let (&DeclLocal(ref l), &DeclLocal(ref r)) = (&l.node, &r.node) { @@ -58,12 +58,12 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { } /// Check whether two blocks are the same. - pub fn eq_block(&self, left: &Block, right: &Block) -> bool { + pub fn eq_block(&mut self, left: &Block, right: &Block) -> bool { over(&left.stmts, &right.stmts, |l, r| self.eq_stmt(l, r)) && both(&left.expr, &right.expr, |l, r| self.eq_expr(l, r)) } - pub fn eq_expr(&self, left: &Expr, right: &Expr) -> bool { + pub fn eq_expr(&mut self, left: &Expr, right: &Expr) -> bool { if self.ignore_fn && differing_macro_contexts(left.span, right.span) { return false; } @@ -144,20 +144,20 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { } } - fn eq_exprs(&self, left: &P<[Expr]>, right: &P<[Expr]>) -> bool { + fn eq_exprs(&mut self, left: &P<[Expr]>, right: &P<[Expr]>) -> bool { over(left, right, |l, r| self.eq_expr(l, r)) } - fn eq_field(&self, left: &Field, right: &Field) -> bool { + fn eq_field(&mut self, left: &Field, right: &Field) -> bool { left.name.node == right.name.node && self.eq_expr(&left.expr, &right.expr) } - fn eq_lifetime(&self, left: &Lifetime, right: &Lifetime) -> bool { + fn eq_lifetime(&mut self, left: &Lifetime, right: &Lifetime) -> bool { left.name == right.name } /// Check whether two patterns are the same. - pub fn eq_pat(&self, left: &Pat, right: &Pat) -> bool { + pub fn eq_pat(&mut self, left: &Pat, right: &Pat) -> bool { match (&left.node, &right.node) { (&PatKind::Box(ref l), &PatKind::Box(ref r)) => self.eq_pat(l, r), (&PatKind::TupleStruct(ref lp, ref la, ls), &PatKind::TupleStruct(ref rp, ref ra, rs)) => { @@ -184,7 +184,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { } } - fn eq_qpath(&self, left: &QPath, right: &QPath) -> bool { + fn eq_qpath(&mut self, left: &QPath, right: &QPath) -> bool { match (left, right) { (&QPath::Resolved(ref lty, ref lpath), &QPath::Resolved(ref rty, ref rpath)) => { both(lty, rty, |l, r| self.eq_ty(l, r)) && self.eq_path(lpath, rpath) @@ -196,12 +196,12 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { } } - fn eq_path(&self, left: &Path, right: &Path) -> bool { + fn eq_path(&mut self, left: &Path, right: &Path) -> bool { left.is_global() == right.is_global() && over(&left.segments, &right.segments, |l, r| self.eq_path_segment(l, r)) } - fn eq_path_parameters(&self, left: &PathParameters, right: &PathParameters) -> bool { + fn eq_path_parameters(&mut self, left: &PathParameters, right: &PathParameters) -> bool { if !(left.parenthesized || right.parenthesized) { over(&left.lifetimes, &right.lifetimes, |l, r| self.eq_lifetime(l, r)) && over(&left.types, &right.types, |l, r| self.eq_ty(l, r)) @@ -218,7 +218,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { } } - fn eq_path_segment(&self, left: &PathSegment, right: &PathSegment) -> bool { + fn eq_path_segment(&mut self, left: &PathSegment, right: &PathSegment) -> bool { // The == of idents doesn't work with different contexts, // we have to be explicit about hygiene if left.name.as_str() != right.name.as_str() { @@ -231,12 +231,23 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { } } - fn eq_ty(&self, left: &Ty, right: &Ty) -> bool { + fn eq_ty(&mut self, left: &Ty, right: &Ty) -> bool { match (&left.node, &right.node) { (&TySlice(ref l_vec), &TySlice(ref r_vec)) => self.eq_ty(l_vec, r_vec), (&TyArray(ref lt, ll_id), &TyArray(ref rt, rl_id)) => { - self.eq_ty(lt, rt) - && self.eq_expr(&self.cx.tcx.hir.body(ll_id).value, &self.cx.tcx.hir.body(rl_id).value) + let full_table = self.tables; + + let mut celcx = constant_context(self.cx, self.cx.tcx.body_tables(ll_id)); + self.tables = self.cx.tcx.body_tables(ll_id); + let ll = celcx.expr(&self.cx.tcx.hir.body(ll_id).value); + + let mut celcx = constant_context(self.cx, self.cx.tcx.body_tables(rl_id)); + self.tables = self.cx.tcx.body_tables(rl_id); + let rl = celcx.expr(&self.cx.tcx.hir.body(rl_id).value); + + let eq_ty = self.eq_ty(lt, rt); + self.tables = full_table; + eq_ty && ll == rl }, (&TyPtr(ref l_mut), &TyPtr(ref r_mut)) => l_mut.mutbl == r_mut.mutbl && self.eq_ty(&*l_mut.ty, &*r_mut.ty), (&TyRptr(_, ref l_rmut), &TyRptr(_, ref r_rmut)) => { @@ -249,7 +260,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { } } - fn eq_type_binding(&self, left: &TypeBinding, right: &TypeBinding) -> bool { + fn eq_type_binding(&mut self, left: &TypeBinding, right: &TypeBinding) -> bool { left.name == right.name && self.eq_ty(&left.ty, &right.ty) } } @@ -467,8 +478,10 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> { let c: fn(_, _) -> _ = ExprRepeat; c.hash(&mut self.s); self.hash_expr(e); + let full_table = self.tables; self.tables = self.cx.tcx.body_tables(l_id); self.hash_expr(&self.cx.tcx.hir.body(l_id).value); + self.tables = full_table; }, ExprRet(ref e) => { let c: fn(_) -> _ = ExprRet; diff --git a/tests/run-pass/ice-2499.rs b/tests/run-pass/ice-2499.rs index d012d548f20..01deb7abfc1 100644 --- a/tests/run-pass/ice-2499.rs +++ b/tests/run-pass/ice-2499.rs @@ -1,4 +1,4 @@ -#![allow(dead_code)] +#![allow(dead_code, char_lit_as_u8, needless_bool)] /// Should not trigger an ICE in `SpanlessHash` / `consts::constant` /// @@ -9,7 +9,7 @@ fn f(s: &[u8]) -> bool { match t { 'E' | 'W' => {} - 'T' => if &s[0..(0 + 4)] != &['0' as u8; 4] { + 'T' => if s[0..4] != ['0' as u8; 4] { return false; } else { return true;