Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rustup to 2016-03-11 #754

Merged
merged 2 commits into from
Mar 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "clippy"
version = "0.0.49"
version = "0.0.50"
authors = [
"Manish Goregaokar <manishsmail@gmail.com>",
"Andre Bogus <bogusandre@gmail.com>",
Expand Down
7 changes: 4 additions & 3 deletions src/cyclomatic_complexity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,14 @@ impl<'a, 'b, 'tcx> Visitor<'a> for CCHelper<'b, 'tcx> {
ExprCall(ref callee, _) => {
walk_expr(self, e);
let ty = self.tcx.node_id_to_type(callee.id);
if let ty::TyBareFn(_, ty) = ty.sty {
if ty.sig.skip_binder().output.diverges() {
match ty.sty {
ty::TyFnDef(_, _, ty) | ty::TyFnPtr(ty) if ty.sig.skip_binder().output.diverges() => {
self.divergence += 1;
}
_ => (),
}
}
ExprClosure(..) => {}
ExprClosure(..) => (),
ExprBinary(op, _, _) => {
walk_expr(self, e);
match op.node {
Expand Down
2 changes: 1 addition & 1 deletion src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ fn check_copy_clone<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, item: &Item, trait_ref
TypeVariants::TyArray(_, size) if size > 32 => {
return;
}
TypeVariants::TyBareFn(..) => {
TypeVariants::TyFnPtr(..) => {
return;
}
TypeVariants::TyTuple(ref tys) if tys.len() > 12 => {
Expand Down
9 changes: 6 additions & 3 deletions src/eta_reduction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,14 @@ fn check_closure(cx: &LateContext, expr: &Expr) {
return;
}
let fn_ty = cx.tcx.expr_ty(caller);
if let ty::TyBareFn(_, fn_ty) = fn_ty.sty {
match fn_ty.sty {
// Is it an unsafe function? They don't implement the closure traits
if fn_ty.unsafety == Unsafety::Unsafe {
return;
ty::TyFnDef(_, _, fn_ty) | ty::TyFnPtr(fn_ty) => {
if fn_ty.unsafety == Unsafety::Unsafe {
return;
}
}
_ => (),
}
for (ref a1, ref a2) in decl.inputs.iter().zip(args) {
if let PatKind::Ident(_, ident, _) = a1.pat.node {
Expand Down
27 changes: 15 additions & 12 deletions src/mut_reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,24 @@ impl LateLintPass for UnnecessaryMutPassed {
}

fn check_arguments(cx: &LateContext, arguments: &[P<Expr>], type_definition: &TyS, name: &str) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&TyS is Ty...

if let TypeVariants::TyBareFn(_, ref fn_type) = type_definition.sty {
let parameters = &fn_type.sig.skip_binder().inputs;
for (argument, parameter) in arguments.iter().zip(parameters.iter()) {
match parameter.sty {
TypeVariants::TyRef(_, TypeAndMut {mutbl: MutImmutable, ..}) |
TypeVariants::TyRawPtr(TypeAndMut {mutbl: MutImmutable, ..}) => {
if let ExprAddrOf(MutMutable, _) = argument.node {
span_lint(cx,
UNNECESSARY_MUT_PASSED,
argument.span,
&format!("The function/method \"{}\" doesn't need a mutable reference", name));
match type_definition.sty {
TypeVariants::TyFnDef(_, _, ref fn_type) | TypeVariants::TyFnPtr(ref fn_type) => {
let parameters = &fn_type.sig.skip_binder().inputs;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use .fn_sig() to get to this point without matching.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This panics for non-fn types:

thread 'rustc' panicked at 'Ty::fn_sig() called on non-fn type: Box<core::ops::Fn(collections::vec::Vec<u32>) + 'static>', ../src/librustc/middle/ty/sty.rs:1144

with I guess:

pub fn test2(foo: Box<Fn(Vec<u32>)>) { // pass if #31 is fixed
    foo(vec![1, 2, 3])
}

The matching just ignores them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a function call... that's an overloaded method and you need to do something completely different based on whether the method map has an entry. You should really handle that correctly, perhaps abstracting over it.

for (argument, parameter) in arguments.iter().zip(parameters.iter()) {
match parameter.sty {
TypeVariants::TyRef(_, TypeAndMut {mutbl: MutImmutable, ..}) |
TypeVariants::TyRawPtr(TypeAndMut {mutbl: MutImmutable, ..}) => {
if let ExprAddrOf(MutMutable, _) = argument.node {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this code using TypeVariants directly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea, I was like that. I don’t dislike it. I left it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but, it should be using ty.

span_lint(cx,
UNNECESSARY_MUT_PASSED,
argument.span,
&format!("The function/method \"{}\" doesn't need a mutable reference", name));
}
}
_ => {}
}
_ => {}
}
}
_ => (),
}
}
25 changes: 14 additions & 11 deletions tests/compile-fail/mut_reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,8 @@

#![allow(unused_variables)]

fn takes_an_immutable_reference(a: &i32) {
}


fn takes_a_mutable_reference(a: &mut i32) {
}
fn takes_an_immutable_reference(a: &i32) {}
fn takes_a_mutable_reference(a: &mut i32) {}

struct MyStruct;

Expand All @@ -24,23 +20,30 @@ impl MyStruct {
fn main() {
// Functions
takes_an_immutable_reference(&mut 42); //~ERROR The function/method "takes_an_immutable_reference" doesn't need a mutable reference

let foo: fn(&i32) = takes_an_immutable_reference;
foo(&mut 42); //~ERROR The function/method "foo" doesn't need a mutable reference

// Methods
let my_struct = MyStruct;
my_struct.takes_an_immutable_reference(&mut 42); //~ERROR The function/method "takes_an_immutable_reference" doesn't need a mutable reference


// No error

// Functions
takes_an_immutable_reference(&42);
let foo: fn(&i32) = takes_an_immutable_reference;
foo(&42);

takes_a_mutable_reference(&mut 42);
let foo: fn(&mut i32) = takes_a_mutable_reference;
foo(&mut 42);

let a = &mut 42;
takes_an_immutable_reference(a);

// Methods
my_struct.takes_an_immutable_reference(&42);
my_struct.takes_a_mutable_reference(&mut 42);
my_struct.takes_an_immutable_reference(a);

}