diff options
| -rw-r--r-- | clippy_lints/src/ptr.rs | 58 | ||||
| -rw-r--r-- | tests/ui/mut_from_ref.rs | 34 | ||||
| -rw-r--r-- | tests/ui/mut_from_ref.stderr | 50 |
3 files changed, 113 insertions, 29 deletions
diff --git a/clippy_lints/src/ptr.rs b/clippy_lints/src/ptr.rs index 13e651c7a76..b64f527baa1 100644 --- a/clippy_lints/src/ptr.rs +++ b/clippy_lints/src/ptr.rs @@ -498,29 +498,33 @@ fn check_fn_args<'cx, 'tcx: 'cx>( } fn check_mut_from_ref<'tcx>(cx: &LateContext<'tcx>, sig: &FnSig<'_>, body: Option<&Body<'tcx>>) { - if let FnRetTy::Return(ty) = sig.decl.output - && let Some((out, Mutability::Mut, _)) = get_ref_lm(ty) - { + let FnRetTy::Return(ty) = sig.decl.output else { return }; + for (out, mutability, out_span) in get_lifetimes(ty) { + if mutability != Some(Mutability::Mut) { + continue; + } let out_region = cx.tcx.named_bound_var(out.hir_id); - let args: Option<Vec<_>> = sig + // `None` if one of the types contains `&'a mut T` or `T<'a>`. + // Else, contains all the locations of `&'a T` types. + let args_immut_refs: Option<Vec<Span>> = sig .decl .inputs .iter() - .filter_map(get_ref_lm) + .flat_map(get_lifetimes) .filter(|&(lt, _, _)| cx.tcx.named_bound_var(lt.hir_id) == out_region) - .map(|(_, mutability, span)| (mutability == Mutability::Not).then_some(span)) + .map(|(_, mutability, span)| (mutability == Some(Mutability::Not)).then_some(span)) .collect(); - if let Some(args) = args - && !args.is_empty() + if let Some(args_immut_refs) = args_immut_refs + && !args_immut_refs.is_empty() && body.is_none_or(|body| sig.header.is_unsafe() || contains_unsafe_block(cx, body.value)) { span_lint_and_then( cx, MUT_FROM_REF, - ty.span, + out_span, "mutable borrow from immutable input(s)", |diag| { - let ms = MultiSpan::from_spans(args); + let ms = MultiSpan::from_spans(args_immut_refs); diag.span_note(ms, "immutable borrow here"); }, ); @@ -686,12 +690,36 @@ fn matches_preds<'tcx>( }) } -fn get_ref_lm<'tcx>(ty: &'tcx hir::Ty<'tcx>) -> Option<(&'tcx Lifetime, Mutability, Span)> { - if let TyKind::Ref(lt, ref m) = ty.kind { - Some((lt, m.mutbl, ty.span)) - } else { - None +struct LifetimeVisitor<'tcx> { + result: Vec<(&'tcx Lifetime, Option<Mutability>, Span)>, +} + +impl<'tcx> Visitor<'tcx> for LifetimeVisitor<'tcx> { + fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx, hir::AmbigArg>) { + if let TyKind::Ref(lt, ref m) = ty.kind { + self.result.push((lt, Some(m.mutbl), ty.span)); + } + hir::intravisit::walk_ty(self, ty); } + + fn visit_generic_arg(&mut self, generic_arg: &'tcx GenericArg<'tcx>) { + if let GenericArg::Lifetime(lt) = generic_arg { + self.result.push((lt, None, generic_arg.span())); + } + hir::intravisit::walk_generic_arg(self, generic_arg); + } +} + +/// Visit `ty` and collect the all the lifetimes appearing in it, implicit or not. +/// +/// The second field of the vector's elements indicate if the lifetime is attached to a +/// shared reference, a mutable reference, or neither. +fn get_lifetimes<'tcx>(ty: &'tcx hir::Ty<'tcx>) -> Vec<(&'tcx Lifetime, Option<Mutability>, Span)> { + use hir::intravisit::VisitorExt as _; + + let mut visitor = LifetimeVisitor { result: Vec::new() }; + visitor.visit_ty_unambig(ty); + visitor.result } fn is_null_path(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { diff --git a/tests/ui/mut_from_ref.rs b/tests/ui/mut_from_ref.rs index b8c10f3eeb8..1b0b351518c 100644 --- a/tests/ui/mut_from_ref.rs +++ b/tests/ui/mut_from_ref.rs @@ -1,4 +1,10 @@ -#![allow(unused, clippy::needless_lifetimes, clippy::needless_pass_by_ref_mut)] +#![allow( + unused, + clippy::needless_lifetimes, + clippy::needless_pass_by_ref_mut, + clippy::redundant_allocation, + clippy::boxed_local +)] #![warn(clippy::mut_from_ref)] struct Foo; @@ -40,6 +46,18 @@ fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 { unsafe { unimplemented!() } } +fn fail_tuples<'a>(x: (&'a u32, &'a u32)) -> &'a mut u32 { + //~^ mut_from_ref + + unsafe { unimplemented!() } +} + +fn fail_box<'a>(x: Box<&'a u32>) -> &'a mut u32 { + //~^ mut_from_ref + + unsafe { unimplemented!() } +} + // this is OK, because the result borrows y fn works<'a>(x: &u32, y: &'a mut u32) -> &'a mut u32 { unsafe { unimplemented!() } @@ -50,6 +68,20 @@ fn also_works<'a>(x: &'a u32, y: &'a mut u32) -> &'a mut u32 { unsafe { unimplemented!() } } +fn works_tuples<'a>(x: (&'a u32, &'a mut u32)) -> &'a mut u32 { + unsafe { unimplemented!() } +} + +fn works_box<'a>(x: &'a u32, y: Box<&'a mut u32>) -> &'a mut u32 { + unsafe { unimplemented!() } +} + +struct RefMut<'a>(&'a mut u32); + +fn works_parameter<'a>(x: &'a u32, y: RefMut<'a>) -> &'a mut u32 { + unsafe { unimplemented!() } +} + unsafe fn also_broken(x: &u32) -> &mut u32 { //~^ mut_from_ref diff --git a/tests/ui/mut_from_ref.stderr b/tests/ui/mut_from_ref.stderr index 8c3c8e0c3d8..09742687346 100644 --- a/tests/ui/mut_from_ref.stderr +++ b/tests/ui/mut_from_ref.stderr @@ -1,11 +1,11 @@ error: mutable borrow from immutable input(s) - --> tests/ui/mut_from_ref.rs:7:39 + --> tests/ui/mut_from_ref.rs:13:39 | LL | fn this_wont_hurt_a_bit(&self) -> &mut Foo { | ^^^^^^^^ | note: immutable borrow here - --> tests/ui/mut_from_ref.rs:7:29 + --> tests/ui/mut_from_ref.rs:13:29 | LL | fn this_wont_hurt_a_bit(&self) -> &mut Foo { | ^^^^^ @@ -13,64 +13,88 @@ LL | fn this_wont_hurt_a_bit(&self) -> &mut Foo { = help: to override `-D warnings` add `#[allow(clippy::mut_from_ref)]` error: mutable borrow from immutable input(s) - --> tests/ui/mut_from_ref.rs:15:25 + --> tests/ui/mut_from_ref.rs:21:25 | LL | fn ouch(x: &Foo) -> &mut Foo; | ^^^^^^^^ | note: immutable borrow here - --> tests/ui/mut_from_ref.rs:15:16 + --> tests/ui/mut_from_ref.rs:21:16 | LL | fn ouch(x: &Foo) -> &mut Foo; | ^^^^ error: mutable borrow from immutable input(s) - --> tests/ui/mut_from_ref.rs:25:21 + --> tests/ui/mut_from_ref.rs:31:21 | LL | fn fail(x: &u32) -> &mut u16 { | ^^^^^^^^ | note: immutable borrow here - --> tests/ui/mut_from_ref.rs:25:12 + --> tests/ui/mut_from_ref.rs:31:12 | LL | fn fail(x: &u32) -> &mut u16 { | ^^^^ error: mutable borrow from immutable input(s) - --> tests/ui/mut_from_ref.rs:31:50 + --> tests/ui/mut_from_ref.rs:37:50 | LL | fn fail_lifetime<'a>(x: &'a u32, y: &mut u32) -> &'a mut u32 { | ^^^^^^^^^^^ | note: immutable borrow here - --> tests/ui/mut_from_ref.rs:31:25 + --> tests/ui/mut_from_ref.rs:37:25 | LL | fn fail_lifetime<'a>(x: &'a u32, y: &mut u32) -> &'a mut u32 { | ^^^^^^^ error: mutable borrow from immutable input(s) - --> tests/ui/mut_from_ref.rs:37:67 + --> tests/ui/mut_from_ref.rs:43:67 | LL | fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 { | ^^^^^^^^^^^ | note: immutable borrow here - --> tests/ui/mut_from_ref.rs:37:27 + --> tests/ui/mut_from_ref.rs:43:27 | LL | fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 { | ^^^^^^^ ^^^^^^^ error: mutable borrow from immutable input(s) - --> tests/ui/mut_from_ref.rs:53:35 + --> tests/ui/mut_from_ref.rs:49:46 + | +LL | fn fail_tuples<'a>(x: (&'a u32, &'a u32)) -> &'a mut u32 { + | ^^^^^^^^^^^ + | +note: immutable borrow here + --> tests/ui/mut_from_ref.rs:49:24 + | +LL | fn fail_tuples<'a>(x: (&'a u32, &'a u32)) -> &'a mut u32 { + | ^^^^^^^ ^^^^^^^ + +error: mutable borrow from immutable input(s) + --> tests/ui/mut_from_ref.rs:55:37 + | +LL | fn fail_box<'a>(x: Box<&'a u32>) -> &'a mut u32 { + | ^^^^^^^^^^^ + | +note: immutable borrow here + --> tests/ui/mut_from_ref.rs:55:24 + | +LL | fn fail_box<'a>(x: Box<&'a u32>) -> &'a mut u32 { + | ^^^^^^^ + +error: mutable borrow from immutable input(s) + --> tests/ui/mut_from_ref.rs:85:35 | LL | unsafe fn also_broken(x: &u32) -> &mut u32 { | ^^^^^^^^ | note: immutable borrow here - --> tests/ui/mut_from_ref.rs:53:26 + --> tests/ui/mut_from_ref.rs:85:26 | LL | unsafe fn also_broken(x: &u32) -> &mut u32 { | ^^^^ -error: aborting due to 6 previous errors +error: aborting due to 8 previous errors |