From 88970ec8cf8645e4af826f54341486b04f853846 Mon Sep 17 00:00:00 2001
From: Oliver Schneider <git-spam-no-reply9815368754983@oli-obk.de>
Date: Fri, 9 Feb 2018 14:22:41 +0100
Subject: [PATCH 1/3] Remove rarely used `type_size` helper function
---
clippy_lints/src/escape.rs | 5 +++--
clippy_lints/src/large_enum_variant.rs | 14 ++++++--------
clippy_lints/src/types.rs | 5 +++--
clippy_lints/src/utils/mod.rs | 7 -------
4 files changed, 12 insertions(+), 19 deletions(-)
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/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<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
From ff32d5f7343934f4267af4e270017c716fb9d4ad Mon Sep 17 00:00:00 2001
From: Oliver Schneider <git-spam-no-reply9815368754983@oli-obk.de>
Date: Fri, 9 Feb 2018 14:22:50 +0100
Subject: [PATCH 2/3] Fix #2427
---
clippy_lints/src/utils/hir_utils.rs | 6 +++---
tests/run-pass/match_same_arms_const.rs | 16 ++++++++++++++++
2 files changed, 19 insertions(+), 3 deletions(-)
create mode 100644 tests/run-pass/match_same_arms_const.rs
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/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() {}
From 6feb0dd9824e81c2ee9a2389e53e564d25bdca2d Mon Sep 17 00:00:00 2001
From: Oliver Schneider <git-spam-no-reply9815368754983@oli-obk.de>
Date: Fri, 9 Feb 2018 15:23:51 +0100
Subject: [PATCH 3/3] Fixes #2426 (if_same_then_else false positive)
---
clippy_lints/src/copies.rs | 21 ++++++++-----
tests/run-pass/if_same_then_else.rs | 13 ++++++++
tests/ui/copies.rs | 4 +--
tests/ui/copies.stderr | 48 +----------------------------
4 files changed, 30 insertions(+), 56 deletions(-)
create mode 100644 tests/run-pass/if_same_then_else.rs
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<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,
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/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