From bb552dc96fc46ff0b33bc48f7c3c48f3acf2a306 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Wed, 26 Aug 2015 17:09:37 +0200 Subject: [PATCH] eta_reduction: fix false positive for unsafe fns (fixes #243) --- src/eta_reduction.rs | 22 ++++++++++++++++------ tests/compile-fail/eta.rs | 7 ++++++- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/eta_reduction.rs b/src/eta_reduction.rs index 481512abc62..da2149f0539 100644 --- a/src/eta_reduction.rs +++ b/src/eta_reduction.rs @@ -1,8 +1,8 @@ use rustc::lint::*; use syntax::ast::*; -use syntax::print::pprust::expr_to_string; +use rustc::middle::ty; -use utils::span_lint; +use utils::{snippet, span_lint}; #[allow(missing_copy_implementations)] @@ -47,7 +47,17 @@ fn check_closure(cx: &Context, expr: &Expr) { // is no way the closure is the same as the function return; } - if args.iter().any(|arg| is_adjusted(cx, arg)) { return; } + if args.iter().any(|arg| is_adjusted(cx, arg)) { + // Are the arguments type-adjusted? Then we need the closure + return; + } + let fn_ty = cx.tcx.expr_ty(caller); + if let ty::TyBareFn(_, fn_ty) = fn_ty.sty { + // Is it an unsafe function? They don't implement the closure traits + if fn_ty.unsafety == Unsafety::Unsafe { + return; + } + } for (ref a1, ref a2) in decl.inputs.iter().zip(args) { if let PatIdent(_, ident, _) = a1.pat.node { // XXXManishearth Should I be checking the binding mode here? @@ -67,9 +77,9 @@ fn check_closure(cx: &Context, expr: &Expr) { return } } - span_lint(cx, REDUNDANT_CLOSURE, expr.span, - &format!("redundant closure found. Consider using `{}` in its place", - expr_to_string(caller))[..]) + span_lint(cx, REDUNDANT_CLOSURE, expr.span, &format!( + "redundant closure found. Consider using `{}` in its place", + snippet(cx, caller.span, ".."))); } } } diff --git a/tests/compile-fail/eta.rs b/tests/compile-fail/eta.rs index bf6ecd79617..d53ea4e97d7 100755 --- a/tests/compile-fail/eta.rs +++ b/tests/compile-fail/eta.rs @@ -9,9 +9,12 @@ fn main() { meta(|a| foo(a)); //~^ ERROR redundant closure found. Consider using `foo` in its place let c = Some(1u8).map(|a| {1+2; foo}(a)); - //~^ ERROR redundant closure found. Consider using `{ 1 + 2; foo }` in its place + //~^ ERROR redundant closure found. Consider using `{1+2; foo}` in its place let d = Some(1u8).map(|a| foo((|b| foo2(b))(a))); //is adjusted? all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted + unsafe { + Some(1u8).map(|a| unsafe_fn(a)); // unsafe fn + } } fn meta(f: F) where F: Fn(u8) { @@ -32,3 +35,5 @@ where F: Fn(&X, &X) -> bool { } fn below(x: &u8, y: &u8) -> bool { x < y } + +unsafe fn unsafe_fn(_: u8) { }