From bff4c30ac7614c636a085e9ae65e1dec2714804e Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Sat, 11 Feb 2017 00:32:12 +0100 Subject: [PATCH] added test, fixed message & description, rustfmt --- clippy_lints/src/ptr.rs | 30 +++++++++++++++++++----------- tests/ui/mut_from_ref.rs | 4 ++++ tests/ui/mut_from_ref.stderr | 21 +++++++++++++-------- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 7a7631d9769..a30d632f159 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -44,13 +44,15 @@ declare_lint! { "comparing a pointer to a null pointer, suggesting to use `.is_null()` instead." } -/// **What it does:** This lint checks for functions that take immutable refs and return +/// **What it does:** This lint checks for functions that take immutable references and return /// mutable ones. /// -/// **Why is this bad?** This is trivially unsound, as one can create two mutable refs -/// from the same source. +/// **Why is this bad?** This is trivially unsound, as one can create two mutable references +/// from the same (immutable!) source. This [error](https://github.com/rust-lang/rust/issues/39465) +/// actually lead to an interim Rust release 1.15.1. /// -/// **Known problems:** This lint will overlook functions where input and output lifetimes differ +/// **Known problems:** To be on the conservative side, if there's at least one mutable reference +/// with the output lifetime, this lint will not trigger. In practice, this case is unlikely anyway. /// /// **Example:** /// ```rust @@ -131,25 +133,31 @@ fn check_fn(cx: &LateContext, decl: &FnDecl, fn_id: NodeId) { if let FunctionRetTy::Return(ref ty) = decl.output { if let Some((out, MutMutable)) = get_rptr_lm(ty) { - if let Some(MutImmutable) = decl.inputs.iter() - .filter_map(|ty| get_rptr_lm(ty)) - .filter(|&(lt, _)| lt.name == out.name) - .fold(None, |x, (_, m)| match (x, m) { + if let Some(MutImmutable) = + decl.inputs + .iter() + .filter_map(|ty| get_rptr_lm(ty)) + .filter(|&(lt, _)| lt.name == out.name) + .fold(None, |x, (_, m)| match (x, m) { (Some(MutMutable), _) | (_, MutMutable) => Some(MutMutable), (_, m) => Some(m), - }) { + }) { span_lint(cx, MUT_FROM_REF, ty.span, - "this function takes an immutable ref to return a mutable one:"); + "this function takes an immutable ref to return a mutable one"); } } } } fn get_rptr_lm(ty: &Ty) -> Option<(&Lifetime, Mutability)> { - if let Ty_::TyRptr(ref lt, ref m) = ty.node { Some((lt, m.mutbl)) } else { None } + if let Ty_::TyRptr(ref lt, ref m) = ty.node { + Some((lt, m.mutbl)) + } else { + None + } } fn is_null_path(expr: &Expr) -> bool { diff --git a/tests/ui/mut_from_ref.rs b/tests/ui/mut_from_ref.rs index 09cc8a6061c..24b73eacc32 100644 --- a/tests/ui/mut_from_ref.rs +++ b/tests/ui/mut_from_ref.rs @@ -25,6 +25,10 @@ fn fail(x: &u32) -> &mut u16 { unimplemented!() } +fn fail_lifetime<'a>(x: &'a u32, y: &mut u32) -> &'a mut u32 { + unimplemented!() +} + // this is OK, because the result borrows y fn works<'a>(x: &u32, y: &'a mut u32) -> &'a mut u32 { unimplemented!() diff --git a/tests/ui/mut_from_ref.stderr b/tests/ui/mut_from_ref.stderr index 23fc20d9a4b..799adb00714 100644 --- a/tests/ui/mut_from_ref.stderr +++ b/tests/ui/mut_from_ref.stderr @@ -1,26 +1,31 @@ -error: this function takes an immutable ref to return a mutable one: - --> $DIR/mut_from_ref.rs:9:39 +error: this function takes an immutable ref to return a mutable one + --> tests/ui/mut_from_ref.rs:9:39 | 9 | fn this_wont_hurt_a_bit(&self) -> &mut Foo { | ^^^^^^^^ | note: lint level defined here - --> $DIR/mut_from_ref.rs:4:9 + --> tests/ui/mut_from_ref.rs:4:9 | 4 | #![deny(mut_from_ref)] | ^^^^^^^^^^^^ -error: this function takes an immutable ref to return a mutable one: - --> $DIR/mut_from_ref.rs:15:25 +error: this function takes an immutable ref to return a mutable one + --> tests/ui/mut_from_ref.rs:15:25 | 15 | fn ouch(x: &Foo) -> &mut Foo; | ^^^^^^^^ -error: this function takes an immutable ref to return a mutable one: - --> $DIR/mut_from_ref.rs:24:21 +error: this function takes an immutable ref to return a mutable one + --> tests/ui/mut_from_ref.rs:24:21 | 24 | fn fail(x: &u32) -> &mut u16 { | ^^^^^^^^ -error: aborting due to 3 previous errors +error: this function takes an immutable ref to return a mutable one + --> tests/ui/mut_from_ref.rs:28:50 + | +28 | fn fail_lifetime<'a>(x: &'a u32, y: &mut u32) -> &'a mut u32 { + | ^^^^^^^^^^^ +error: aborting due to 4 previous errors