Merge pull request #2448 from rust-lang-nursery/fixes

Fixes

Author: oli-obk

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<Interned
     result
 }
 
+
+fn search_same_sequenced<T, Eq>(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<T, Hash, Eq>(exprs: &[T], hash: Hash, eq: Eq) -> Option<(&T, &T)>
 where
     Hash: Fn(&T) -> u64,

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
         }

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();
 

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 {

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);
         }
 

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<u64> {
-    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

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
+    }
+}

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() {}

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..]);
     }

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