mathieu/granite-rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge pull request #1705 from Manishearth/op_ref

Browse source 
Op ref
This commit is contained in:
Martin Carton 2017-04-28 19:54:24 +02:00 • committed by GitHub
commit 50c8d4e270
8 changed files with 171 additions and 97 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
clippy_lints/src/consts.rs
View file

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

10
clippy_lints/src/derive.rs
View file

@ -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(), &parameter_environment, item.span) {
return; // ty is not Copy
if !is_copy(cx, ty, item.id) {
return;
}
match ty.sty {

177
clippy_lints/src/eq_op.rs
View file

@ -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);
})
}
},
_ => {},
}
}
}

3
clippy_lints/src/methods.rs
View file

@ -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(), &parameter_environment, expr.span) {
if is_copy(cx, ty, parent) {
span_lint_and_then(cx,
CLONE_ON_COPY,
expr.span,

15
tests/run-pass/associated-constant-ice.rs Normal file
View file

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

38
tests/ui/eq_op.rs
View file

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

10
tests/ui/eq_op.stderr
View file

@ -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

2
tests/ui/op_ref.stderr
View file

@ -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;