diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 181158a5f17..d41ea5849a8 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -134,15 +134,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste { /// Implementation of `IF_SAME_THEN_ELSE`. fn lint_same_then_else(cx: &LateContext, blocks: &[&Block]) { - let hash: &Fn(&&Block) -> u64 = &|block| -> u64 { - let mut h = SpanlessHash::new(cx); - h.hash_block(block); - h.finish() - }; - let eq: &Fn(&&Block, &&Block) -> bool = &|&lhs, &rhs| -> bool { SpanlessEq::new(cx).eq_block(lhs, rhs) }; - if let Some((i, j)) = search_same(blocks, hash, eq) { + if let Some((i, j)) = search_same_sequenced(blocks, eq) { span_note_and_lint( cx, IF_SAME_THEN_ELSE, @@ -309,6 +303,19 @@ fn bindings<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, pat: &Pat) -> HashMap(exprs: &[T], eq: Eq) -> Option<(&T, &T)> +where + Eq: Fn(&T, &T) -> bool, +{ + for win in exprs.windows(2) { + if eq(&win[0], &win[1]) { + return Some((&win[0], &win[1])); + } + } + None +} + fn search_same(exprs: &[T], hash: Hash, eq: Eq) -> Option<(&T, &T)> where Hash: Fn(&T) -> u64, diff --git a/clippy_lints/src/escape.rs b/clippy_lints/src/escape.rs index 2038a59137c..d4c91eab1c3 100644 --- a/clippy_lints/src/escape.rs +++ b/clippy_lints/src/escape.rs @@ -5,10 +5,11 @@ use rustc::lint::*; use rustc::middle::expr_use_visitor::*; use rustc::middle::mem_categorization::{cmt, Categorization}; use rustc::ty::{self, Ty}; +use rustc::ty::layout::LayoutOf; use rustc::util::nodemap::NodeSet; use syntax::ast::NodeId; use syntax::codemap::Span; -use utils::{span_lint, type_size}; +use utils::span_lint; pub struct Pass { pub too_large_for_stack: u64, @@ -164,7 +165,7 @@ impl<'a, 'tcx> EscapeDelegate<'a, 'tcx> { // Large types need to be boxed to avoid stack // overflows. if ty.is_box() { - type_size(self.cx, ty.boxed_ty()).unwrap_or(0) > self.too_large_for_stack + self.cx.layout_of(ty.boxed_ty()).ok().map_or(0, |l| l.size.bytes()) > self.too_large_for_stack } else { false } diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs index ceb0cbd6688..e13b771cf24 100644 --- a/clippy_lints/src/large_enum_variant.rs +++ b/clippy_lints/src/large_enum_variant.rs @@ -2,8 +2,8 @@ use rustc::lint::*; use rustc::hir::*; -use utils::{snippet_opt, span_lint_and_then, type_size}; -use rustc::ty::TypeFoldable; +use utils::{snippet_opt, span_lint_and_then}; +use rustc::ty::layout::LayoutOf; /// **What it does:** Checks for large size differences between variants on /// `enum`s. @@ -61,13 +61,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LargeEnumVariant { let size: u64 = variant .fields .iter() - .map(|f| { + .filter_map(|f| { let ty = cx.tcx.type_of(f.did); - if ty.needs_subst() { - 0 // we can't reason about generics, so we treat them as zero sized - } else { - type_size(cx, ty).expect("size should be computable for concrete type") - } + // don't count generics by filtering out everything + // that does not have a layout + cx.layout_of(ty).ok().map(|l| l.size.bytes()) }) .sum(); diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index c7e724625be..ba79bf4407b 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -4,6 +4,7 @@ use rustc::hir::*; use rustc::hir::intravisit::{walk_body, walk_expr, walk_ty, FnKind, NestedVisitorMap, Visitor}; use rustc::lint::*; use rustc::ty::{self, Ty, TyCtxt, TypeckTables}; +use rustc::ty::layout::LayoutOf; use rustc::ty::subst::Substs; use rustc_typeck::hir_ty_to_ty; use std::cmp::Ordering; @@ -15,7 +16,7 @@ use syntax::codemap::Span; use syntax::errors::DiagnosticBuilder; use utils::{comparisons, higher, in_constant, in_external_macro, in_macro, last_path_segment, match_def_path, match_path, multispan_sugg, opt_def_id, same_tys, snippet, snippet_opt, span_help_and_lint, span_lint, - span_lint_and_sugg, span_lint_and_then, type_size}; + span_lint_and_sugg, span_lint_and_then}; use utils::paths; /// Handles all the linting of funky types @@ -1478,7 +1479,7 @@ fn numeric_cast_precast_bounds<'a>(cx: &LateContext, expr: &'a Expr) -> Option<( let pre_cast_ty = cx.tables.expr_ty(cast_exp); let cast_ty = cx.tables.expr_ty(expr); // if it's a cast from i32 to u32 wrapping will invalidate all these checks - if type_size(cx, pre_cast_ty) == type_size(cx, cast_ty) { + if cx.layout_of(pre_cast_ty).ok().map(|l| l.size) == cx.layout_of(cast_ty).ok().map(|l| l.size) { return None; } match pre_cast_ty.sty { diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 34073d9725b..cad6ec532a6 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -1,4 +1,4 @@ -use consts::{constant, constant_context}; +use consts::{constant_simple, constant_context}; use rustc::lint::*; use rustc::hir::*; use std::hash::{Hash, Hasher}; @@ -64,7 +64,7 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { return false; } - if let (Some(l), Some(r)) = (constant(self.cx, left), constant(self.cx, right)) { + if let (Some(l), Some(r)) = (constant_simple(self.cx, left), constant_simple(self.cx, right)) { if l == r { return true; } @@ -317,7 +317,7 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> { } pub fn hash_expr(&mut self, e: &Expr) { - if let Some(e) = constant(self.cx, e) { + if let Some(e) = constant_simple(self.cx, e) { return e.hash(&mut self.s); } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index e89163fb52b..c501dadeb79 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -9,7 +9,6 @@ use rustc::lint::{LateContext, Level, Lint, LintContext}; use rustc::session::Session; use rustc::traits; use rustc::ty::{self, Ty, TyCtxt}; -use rustc::ty::layout::LayoutOf; use rustc_errors; use std::borrow::Cow; use std::env; @@ -1048,12 +1047,6 @@ pub fn is_try(expr: &Expr) -> Option<&Expr> { None } -pub fn type_size<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>) -> Option { - cx.layout_of(ty) - .ok() - .map(|layout| layout.size.bytes()) -} - /// Returns true if the lint is allowed in the current context /// /// Useful for skipping long running code when it's unnecessary diff --git a/tests/run-pass/if_same_then_else.rs b/tests/run-pass/if_same_then_else.rs new file mode 100644 index 00000000000..eb14ce80756 --- /dev/null +++ b/tests/run-pass/if_same_then_else.rs @@ -0,0 +1,13 @@ +#![deny(if_same_then_else)] + +fn main() {} + +pub fn foo(a: i32, b: i32) -> Option<&'static str> { + if a == b { + None + } else if a > b { + Some("a pfeil b") + } else { + None + } +} diff --git a/tests/run-pass/match_same_arms_const.rs b/tests/run-pass/match_same_arms_const.rs new file mode 100644 index 00000000000..08acc2bc4d8 --- /dev/null +++ b/tests/run-pass/match_same_arms_const.rs @@ -0,0 +1,16 @@ +#![deny(match_same_arms)] + +const PRICE_OF_SWEETS: u32 = 5; +const PRICE_OF_KINDNESS: u32 = 0; +const PRICE_OF_DRINKS: u32 = 5; + +pub fn price(thing: &str) -> u32 { + match thing { + "rolo" => PRICE_OF_SWEETS, + "advice" => PRICE_OF_KINDNESS, + "juice" => PRICE_OF_DRINKS, + _ => panic!() + } +} + +fn main() {} diff --git a/tests/ui/copies.rs b/tests/ui/copies.rs index e5f5810795d..0588c141103 100644 --- a/tests/ui/copies.rs +++ b/tests/ui/copies.rs @@ -160,7 +160,7 @@ fn if_same_then_else() -> Result<&'static str, ()> { else if false { foo(); } - else if foo() { //~ ERROR same body as `if` block + else if foo() { let _ = match 42 { 42 => 1, a if a > 0 => 2, @@ -336,7 +336,7 @@ fn if_same_then_else() -> Result<&'static str, ()> { let foo = "bar"; return Ok(&foo[0..]); } - else { //~ ERROR same body as `if` block + else { let foo = ""; return Ok(&foo[0..]); } diff --git a/tests/ui/copies.stderr b/tests/ui/copies.stderr index c6034a19906..5faf41b51e3 100644 --- a/tests/ui/copies.stderr +++ b/tests/ui/copies.stderr @@ -151,32 +151,6 @@ note: same as this 139 | | } | |_____^ -error: this `if` has identical blocks - --> $DIR/copies.rs:163:19 - | -163 | else if foo() { //~ ERROR same body as `if` block - | ___________________^ -164 | | let _ = match 42 { -165 | | 42 => 1, -166 | | a if a > 0 => 2, -... | -169 | | }; -170 | | } - | |_____^ - | -note: same as this - --> $DIR/copies.rs:152:13 - | -152 | if true { - | _____________^ -153 | | let _ = match 42 { -154 | | 42 => 1, -155 | | a if a > 0 => 2, -... | -158 | | }; -159 | | } - | |_____^ - error: this `if` has identical blocks --> $DIR/copies.rs:175:10 | @@ -370,26 +344,6 @@ note: same as this 326 | | } | |_____^ -error: this `if` has identical blocks - --> $DIR/copies.rs:339:10 - | -339 | else { //~ ERROR same body as `if` block - | __________^ -340 | | let foo = ""; -341 | | return Ok(&foo[0..]); -342 | | } - | |_____^ - | -note: same as this - --> $DIR/copies.rs:331:13 - | -331 | if true { - | _____________^ -332 | | let foo = ""; -333 | | return Ok(&foo[0..]); -334 | | } - | |_____^ - error: this `if` has the same condition as a previous if --> $DIR/copies.rs:353:13 | @@ -427,5 +381,5 @@ note: same as this 361 | if 2*a == 1 { | ^^^^^^^^ -error: aborting due to 22 previous errors +error: aborting due to 20 previous errors