diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index f6a68f267fee..eac72e12e90c 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -6,6 +6,7 @@ use rustc_const_eval::lookup_const_by_id; use rustc_const_math::ConstInt; use rustc::hir::*; use rustc::ty::{self, TyCtxt}; +use rustc::ty::subst::{Substs, Subst}; use std::cmp::Ordering::{self, Equal}; use std::cmp::PartialOrd; use std::hash::{Hash, Hasher}; @@ -225,6 +226,7 @@ pub fn constant(lcx: &LateContext, e: &Expr) -> Option<(Constant, bool)> { tcx: lcx.tcx, tables: lcx.tables, needed_resolution: false, + substs: lcx.tcx.intern_substs(&[]), }; cx.expr(e).map(|cst| (cst, cx.needed_resolution)) } @@ -237,6 +239,7 @@ struct ConstEvalLateContext<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, tables: &'a ty::TypeckTables<'tcx>, needed_resolution: bool, + substs: &'tcx Substs<'tcx>, } impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { @@ -286,11 +289,17 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { let substs = self.tables .node_id_item_substs(id) .unwrap_or_else(|| self.tcx.intern_substs(&[])); - if let Some((const_expr, _)) = lookup_const_by_id(self.tcx, def_id, substs) { + let substs = if self.substs.is_empty() { + substs + } else { + substs.subst(self.tcx, self.substs) + }; + if let Some((def_id, substs)) = lookup_const_by_id(self.tcx, def_id, substs) { let mut cx = ConstEvalLateContext { tcx: self.tcx, - tables: self.tcx.typeck_tables_of(const_expr), + tables: self.tcx.typeck_tables_of(def_id), needed_resolution: false, + substs, }; let body = if let Some(id) = self.tcx.hir.as_local_node_id(def_id) { self.tcx.mir_const_qualif(def_id); diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index dc1747fd3eb7..1710871a31f7 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -1,11 +1,10 @@ use rustc::lint::*; -use rustc::ty::subst::Subst; use rustc::ty::TypeVariants; use rustc::ty; use rustc::hir::*; use syntax::codemap::Span; use utils::paths; -use utils::{is_automatically_derived, span_lint_and_then, match_path_old}; +use utils::{is_automatically_derived, span_lint_and_then, match_path_old, is_copy}; /// **What it does:** Checks for deriving `Hash` but implementing `PartialEq` /// explicitly. @@ -137,11 +136,8 @@ fn check_hash_peq<'a, 'tcx>( /// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint. fn check_copy_clone<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, item: &Item, trait_ref: &TraitRef, ty: ty::Ty<'tcx>) { if match_path_old(&trait_ref.path, &paths::CLONE_TRAIT) { - let parameter_environment = ty::ParameterEnvironment::for_item(cx.tcx, item.id); - let subst_ty = ty.subst(cx.tcx, parameter_environment.free_substs); - - if subst_ty.moves_by_default(cx.tcx.global_tcx(), ¶meter_environment, item.span) { - return; // ty is not Copy + if !is_copy(cx, ty, item.id) { + return; } match ty.sty { diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs index 764bcded5ede..6dedbe4b673e 100644 --- a/clippy_lints/src/eq_op.rs +++ b/clippy_lints/src/eq_op.rs @@ -1,7 +1,6 @@ use rustc::hir::*; use rustc::lint::*; -use utils::{SpanlessEq, span_lint, span_lint_and_then, multispan_sugg, snippet, implements_trait}; -use utils::sugg::Sugg; +use utils::{SpanlessEq, span_lint, span_lint_and_then, multispan_sugg, snippet, implements_trait, is_copy}; /// **What it does:** Checks for equal operands to comparison, logical and /// bitwise, difference and division binary operators (`==`, `>`, etc., `&&`, @@ -53,89 +52,99 @@ impl LintPass for EqOp { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { if let ExprBinary(ref op, ref left, ref right) = e.node { - if is_valid_operator(op) { - if SpanlessEq::new(cx).ignore_fn().eq_expr(left, right) { - span_lint(cx, - EQ_OP, - e.span, - &format!("equal expressions as operands to `{}`", op.node.as_str())); - } else { - let trait_id = match op.node { - BiAdd => cx.tcx.lang_items.add_trait(), - BiSub => cx.tcx.lang_items.sub_trait(), - BiMul => cx.tcx.lang_items.mul_trait(), - BiDiv => cx.tcx.lang_items.div_trait(), - BiRem => cx.tcx.lang_items.rem_trait(), - BiAnd | BiOr => None, - BiBitXor => cx.tcx.lang_items.bitxor_trait(), - BiBitAnd => cx.tcx.lang_items.bitand_trait(), - BiBitOr => cx.tcx.lang_items.bitor_trait(), - BiShl => cx.tcx.lang_items.shl_trait(), - BiShr => cx.tcx.lang_items.shr_trait(), - BiNe | BiEq => cx.tcx.lang_items.eq_trait(), - BiLt | BiLe | BiGe | BiGt => cx.tcx.lang_items.ord_trait(), - }; - if let Some(trait_id) = trait_id { - #[allow(match_same_arms)] - match (&left.node, &right.node) { - // do not suggest to dereference literals - (&ExprLit(..), _) | - (_, &ExprLit(..)) => {}, - // &foo == &bar - (&ExprAddrOf(_, ref l), &ExprAddrOf(_, ref r)) => { - if implements_trait(cx, cx.tables.expr_ty(l), trait_id, &[cx.tables.expr_ty(r)], None) { - span_lint_and_then(cx, - OP_REF, - e.span, - "taken reference of both operands, which is done automatically \ - by the operator anyway", - |db| { - let lsnip = snippet(cx, l.span, "...").to_string(); - let rsnip = snippet(cx, r.span, "...").to_string(); - multispan_sugg(db, - "use the values directly".to_string(), - vec![(left.span, lsnip), - (right.span, rsnip)]); - }) - } - }, - // &foo == bar - (&ExprAddrOf(_, ref l), _) => { - if implements_trait(cx, - cx.tables.expr_ty(l), - trait_id, - &[cx.tables.expr_ty(right)], - None) { - span_lint_and_then(cx, OP_REF, e.span, "taken reference of left operand", |db| { - let lsnip = snippet(cx, l.span, "...").to_string(); - let rsnip = Sugg::hir(cx, right, "...").deref().to_string(); - multispan_sugg(db, - "dereference the right operand instead".to_string(), - vec![(left.span, lsnip), - (right.span, rsnip)]); - }) - } - }, - // foo == &bar - (_, &ExprAddrOf(_, ref r)) => { - if implements_trait(cx, - cx.tables.expr_ty(left), - trait_id, - &[cx.tables.expr_ty(r)], - None) { - span_lint_and_then(cx, OP_REF, e.span, "taken reference of right operand", |db| { - let lsnip = Sugg::hir(cx, left, "...").deref().to_string(); - let rsnip = snippet(cx, r.span, "...").to_string(); - multispan_sugg(db, - "dereference the left operand instead".to_string(), - vec![(left.span, lsnip), - (right.span, rsnip)]); - }) - } - }, - _ => {}, + if is_valid_operator(op) && SpanlessEq::new(cx).ignore_fn().eq_expr(left, right) { + span_lint(cx, + EQ_OP, + e.span, + &format!("equal expressions as operands to `{}`", op.node.as_str())); + return; + } + let (trait_id, requires_ref) = match op.node { + BiAdd => (cx.tcx.lang_items.add_trait(), false), + BiSub => (cx.tcx.lang_items.sub_trait(), false), + BiMul => (cx.tcx.lang_items.mul_trait(), false), + BiDiv => (cx.tcx.lang_items.div_trait(), false), + BiRem => (cx.tcx.lang_items.rem_trait(), false), + // don't lint short circuiting ops + BiAnd | BiOr => return, + BiBitXor => (cx.tcx.lang_items.bitxor_trait(), false), + BiBitAnd => (cx.tcx.lang_items.bitand_trait(), false), + BiBitOr => (cx.tcx.lang_items.bitor_trait(), false), + BiShl => (cx.tcx.lang_items.shl_trait(), false), + BiShr => (cx.tcx.lang_items.shr_trait(), false), + BiNe | BiEq => (cx.tcx.lang_items.eq_trait(), true), + BiLt | BiLe | BiGe | BiGt => (cx.tcx.lang_items.ord_trait(), true), + }; + let parent = cx.tcx.hir.get_parent(e.id); + if let Some(trait_id) = trait_id { + #[allow(match_same_arms)] + match (&left.node, &right.node) { + // do not suggest to dereference literals + (&ExprLit(..), _) | + (_, &ExprLit(..)) => {}, + // &foo == &bar + (&ExprAddrOf(_, ref l), &ExprAddrOf(_, ref r)) => { + let lty = cx.tables.expr_ty(l); + let rty = cx.tables.expr_ty(r); + let lcpy = is_copy(cx, lty, parent); + let rcpy = is_copy(cx, rty, parent); + // either operator autorefs or both args are copyable + if (requires_ref || (lcpy && rcpy)) && implements_trait(cx, lty, trait_id, &[rty], None) { + span_lint_and_then(cx, + OP_REF, + e.span, + "needlessly taken reference of both operands", + |db| { + let lsnip = snippet(cx, l.span, "...").to_string(); + let rsnip = snippet(cx, r.span, "...").to_string(); + multispan_sugg(db, + "use the values directly".to_string(), + vec![(left.span, lsnip), + (right.span, rsnip)]); + }) + } else if lcpy && !rcpy && implements_trait(cx, lty, trait_id, &[cx.tables.expr_ty(right)], None) { + span_lint_and_then(cx, + OP_REF, + e.span, + "needlessly taken reference of left operand", + |db| { + let lsnip = snippet(cx, l.span, "...").to_string(); + db.span_suggestion(left.span, "use the left value directly", lsnip); + }) + } else if !lcpy && rcpy && implements_trait(cx, cx.tables.expr_ty(left), trait_id, &[rty], None) { + span_lint_and_then(cx, + OP_REF, + e.span, + "needlessly taken reference of right operand", + |db| { + let rsnip = snippet(cx, r.span, "...").to_string(); + db.span_suggestion(right.span, "use the right value directly", rsnip); + }) } - } + }, + // &foo == bar + (&ExprAddrOf(_, ref l), _) => { + let lty = cx.tables.expr_ty(l); + let lcpy = is_copy(cx, lty, parent); + if (requires_ref || lcpy) && implements_trait(cx, lty, trait_id, &[cx.tables.expr_ty(right)], None) { + span_lint_and_then(cx, OP_REF, e.span, "needlessly taken reference of left operand", |db| { + let lsnip = snippet(cx, l.span, "...").to_string(); + db.span_suggestion(left.span, "use the left value directly", lsnip); + }) + } + }, + // foo == &bar + (_, &ExprAddrOf(_, ref r)) => { + let rty = cx.tables.expr_ty(r); + let rcpy = is_copy(cx, rty, parent); + if (requires_ref || rcpy) && implements_trait(cx, cx.tables.expr_ty(left), trait_id, &[rty], None) { + span_lint_and_then(cx, OP_REF, e.span, "taken reference of right operand", |db| { + let rsnip = snippet(cx, r.span, "...").to_string(); + db.span_suggestion(left.span, "use the right value directly", rsnip); + }) + } + }, + _ => {}, } } } diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs index 7e1acd8b6731..64a8f72ea016 100644 --- a/clippy_lints/src/methods.rs +++ b/clippy_lints/src/methods.rs @@ -821,7 +821,6 @@ fn lint_or_fun_call(cx: &LateContext, expr: &hir::Expr, name: &str, args: &[hir: fn lint_clone_on_copy(cx: &LateContext, expr: &hir::Expr, arg: &hir::Expr, arg_ty: ty::Ty) { let ty = cx.tables.expr_ty(expr); let parent = cx.tcx.hir.get_parent(expr.id); - let parameter_environment = ty::ParameterEnvironment::for_item(cx.tcx, parent); if let ty::TyRef(_, ty::TypeAndMut { ty: inner, .. }) = arg_ty.sty { if let ty::TyRef(..) = inner.sty { span_lint_and_then(cx, @@ -838,7 +837,7 @@ fn lint_clone_on_copy(cx: &LateContext, expr: &hir::Expr, arg: &hir::Expr, arg_t } } - if !ty.moves_by_default(cx.tcx.global_tcx(), ¶meter_environment, expr.span) { + if is_copy(cx, ty, parent) { span_lint_and_then(cx, CLONE_ON_COPY, expr.span, diff --git a/tests/run-pass/associated-constant-ice.rs b/tests/run-pass/associated-constant-ice.rs new file mode 100644 index 000000000000..a61cd93211d6 --- /dev/null +++ b/tests/run-pass/associated-constant-ice.rs @@ -0,0 +1,15 @@ +#![feature(associated_consts)] +#![feature(plugin)] +#![plugin(clippy)] + +pub trait Trait { + const CONSTANT: u8; +} + +impl Trait for u8 { + const CONSTANT: u8 = 2; +} + +fn main() { + println!("{}", u8::CONSTANT * 10); +} diff --git a/tests/ui/eq_op.rs b/tests/ui/eq_op.rs index 842e5729fcd8..cbe12b066c9b 100644 --- a/tests/ui/eq_op.rs +++ b/tests/ui/eq_op.rs @@ -2,7 +2,7 @@ #![plugin(clippy)] #[deny(eq_op)] -#[allow(identity_op, double_parens)] +#[allow(identity_op, double_parens, many_single_char_names)] #[allow(no_effect, unused_variables, unnecessary_operation, short_circuit_statement)] #[deny(nonminimal_bool)] fn main() { @@ -59,4 +59,40 @@ fn main() { a == a; 2*a.len() == 2*a.len(); // ok, functions a.pop() == a.pop(); // ok, functions + + use std::ops::BitAnd; + struct X(i32); + impl BitAnd for X { + type Output = X; + fn bitand(self, rhs: X) -> X { + X(self.0 & rhs.0) + } + } + impl<'a> BitAnd<&'a X> for X { + type Output = X; + fn bitand(self, rhs: &'a X) -> X { + X(self.0 & rhs.0) + } + } + let x = X(1); + let y = X(2); + let z = x & &y; + + #[derive(Copy, Clone)] + struct Y(i32); + impl BitAnd for Y { + type Output = Y; + fn bitand(self, rhs: Y) -> Y { + Y(self.0 & rhs.0) + } + } + impl<'a> BitAnd<&'a Y> for Y { + type Output = Y; + fn bitand(self, rhs: &'a Y) -> Y { + Y(self.0 & rhs.0) + } + } + let x = Y(1); + let y = Y(2); + let z = x & &y; } diff --git a/tests/ui/eq_op.stderr b/tests/ui/eq_op.stderr index dc2df524df25..b4ece862e00b 100644 --- a/tests/ui/eq_op.stderr +++ b/tests/ui/eq_op.stderr @@ -219,5 +219,15 @@ error: equal expressions as operands to `==` 59 | a == a; | ^^^^^^ +warning: taken reference of right operand + --> $DIR/eq_op.rs:97:13 + | +97 | let z = x & &y; + | ^^^^^^ + | + = note: #[warn(op_ref)] on by default +help: use the right value directly + | let z = y & &y; + error: aborting due to 32 previous errors diff --git a/tests/ui/op_ref.stderr b/tests/ui/op_ref.stderr index 6e6ca457e03e..5448429f1647 100644 --- a/tests/ui/op_ref.stderr +++ b/tests/ui/op_ref.stderr @@ -1,4 +1,4 @@ -warning: taken reference of both operands, which is done automatically by the operator anyway +warning: needlessly taken reference of both operands --> $DIR/op_ref.rs:13:15 | 13 | let foo = &5 - &6;