Skip to content

Commit fb8e574

Browse files
arnaudgolfousearnaudgolfouse
committed
Consider nested lifetimes in mut_from_ref 
1 parent a5a033d commit fb8e574

File tree

3 files changed

+113
-29
lines changed

3 files changed

+113
-29
lines changed

‎clippy_lints/src/ptr.rs

+43-15
Original file line numberDiff line numberDiff line change
@@ -498,29 +498,33 @@ fn check_fn_args<'cx, 'tcx: 'cx>(
498498
}
499499

500500
fn check_mut_from_ref<'tcx>(cx: &LateContext<'tcx>, sig: &FnSig<'_>, body: Option<&Body<'tcx>>) {
501-
if let FnRetTy::Return(ty) = sig.decl.output
502-
&& let Some((out, Mutability::Mut, _)) = get_ref_lm(ty)
503-
{
501+
let FnRetTy::Return(ty) = sig.decl.output else { return };
502+
for (out, mutability, out_span) in get_lifetimes(ty) {
503+
if mutability != Some(Mutability::Mut) {
504+
continue;
505+
}
504506
let out_region = cx.tcx.named_bound_var(out.hir_id);
505-
let args: Option<Vec<_>> = sig
507+
// `None` if one of the types contains `&'a mut T` or `T<'a>`.
508+
// Else, contains all the locations of `&'a T` types.
509+
let args_immut_refs: Option<Vec<Span>> = sig
506510
.decl
507511
.inputs
508512
.iter()
509-
.filter_map(get_ref_lm)
513+
.flat_map(get_lifetimes)
510514
.filter(|&(lt, _, _)| cx.tcx.named_bound_var(lt.hir_id) == out_region)
511-
.map(|(_, mutability, span)| (mutability == Mutability::Not).then_some(span))
515+
.map(|(_, mutability, span)| (mutability == Some(Mutability::Not)).then_some(span))
512516
.collect();
513-
if let Some(args) = args
514-
&& !args.is_empty()
517+
if let Some(args_immut_refs) = args_immut_refs
518+
&& !args_immut_refs.is_empty()
515519
&& body.is_none_or(|body| sig.header.is_unsafe() || contains_unsafe_block(cx, body.value))
516520
{
517521
span_lint_and_then(
518522
cx,
519523
MUT_FROM_REF,
520-
ty.span,
524+
out_span,
521525
"mutable borrow from immutable input(s)",
522526
|diag| {
523-
let ms = MultiSpan::from_spans(args);
527+
let ms = MultiSpan::from_spans(args_immut_refs);
524528
diag.span_note(ms, "immutable borrow here");
525529
},
526530
);
@@ -686,12 +690,36 @@ fn matches_preds<'tcx>(
686690
})
687691
}
688692

689-
fn get_ref_lm<'tcx>(ty: &'tcx hir::Ty<'tcx>) -> Option<(&'tcx Lifetime, Mutability, Span)> {
690-
if let TyKind::Ref(lt, ref m) = ty.kind {
691-
Some((lt, m.mutbl, ty.span))
692-
} else {
693-
None
693+
struct LifetimeVisitor<'tcx> {
694+
result: Vec<(&'tcx Lifetime, Option<Mutability>, Span)>,
695+
}
696+
697+
impl<'tcx> Visitor<'tcx> for LifetimeVisitor<'tcx> {
698+
fn visit_ty(&mut self, ty: &'tcx hir::Ty<'tcx, hir::AmbigArg>) {
699+
if let TyKind::Ref(lt, ref m) = ty.kind {
700+
self.result.push((lt, Some(m.mutbl), ty.span));
701+
}
702+
hir::intravisit::walk_ty(self, ty);
694703
}
704+
705+
fn visit_generic_arg(&mut self, generic_arg: &'tcx GenericArg<'tcx>) {
706+
if let GenericArg::Lifetime(lt) = generic_arg {
707+
self.result.push((lt, None, generic_arg.span()));
708+
}
709+
hir::intravisit::walk_generic_arg(self, generic_arg);
710+
}
711+
}
712+
713+
/// Visit `ty` and collect the all the lifetimes appearing in it, implicit or not.
714+
///
715+
/// The second field of the vector's elements indicate if the lifetime is attached to a
716+
/// shared reference, a mutable reference, or neither.
717+
fn get_lifetimes<'tcx>(ty: &'tcx hir::Ty<'tcx>) -> Vec<(&'tcx Lifetime, Option<Mutability>, Span)> {
718+
use hir::intravisit::VisitorExt as _;
719+
720+
let mut visitor = LifetimeVisitor { result: Vec::new() };
721+
visitor.visit_ty_unambig(ty);
722+
visitor.result
695723
}
696724

697725
fn is_null_path(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {

‎tests/ui/mut_from_ref.rs

+33-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
#![allow(unused, clippy::needless_lifetimes, clippy::needless_pass_by_ref_mut)]
1+
#![allow(
2+
unused,
3+
clippy::needless_lifetimes,
4+
clippy::needless_pass_by_ref_mut,
5+
clippy::redundant_allocation,
6+
clippy::boxed_local
7+
)]
28
#![warn(clippy::mut_from_ref)]
39

410
struct Foo;
@@ -40,6 +46,18 @@ fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 {
4046
unsafe { unimplemented!() }
4147
}
4248

49+
fn fail_tuples<'a>(x: (&'a u32, &'a u32)) -> &'a mut u32 {
50+
//~^ mut_from_ref
51+
52+
unsafe { unimplemented!() }
53+
}
54+
55+
fn fail_box<'a>(x: Box<&'a u32>) -> &'a mut u32 {
56+
//~^ mut_from_ref
57+
58+
unsafe { unimplemented!() }
59+
}
60+
4361
// this is OK, because the result borrows y
4462
fn works<'a>(x: &u32, y: &'a mut u32) -> &'a mut u32 {
4563
unsafe { unimplemented!() }
@@ -50,6 +68,20 @@ fn also_works<'a>(x: &'a u32, y: &'a mut u32) -> &'a mut u32 {
5068
unsafe { unimplemented!() }
5169
}
5270

71+
fn works_tuples<'a>(x: (&'a u32, &'a mut u32)) -> &'a mut u32 {
72+
unsafe { unimplemented!() }
73+
}
74+
75+
fn works_box<'a>(x: &'a u32, y: Box<&'a mut u32>) -> &'a mut u32 {
76+
unsafe { unimplemented!() }
77+
}
78+
79+
struct RefMut<'a>(&'a mut u32);
80+
81+
fn works_parameter<'a>(x: &'a u32, y: RefMut<'a>) -> &'a mut u32 {
82+
unsafe { unimplemented!() }
83+
}
84+
5385
unsafe fn also_broken(x: &u32) -> &mut u32 {
5486
//~^ mut_from_ref
5587

‎tests/ui/mut_from_ref.stderr

+37-13
Original file line numberDiff line numberDiff line change
@@ -1,76 +1,100 @@
11
error: mutable borrow from immutable input(s)
2-
--> tests/ui/mut_from_ref.rs:7:39
2+
--> tests/ui/mut_from_ref.rs:13:39
33
|
44
LL | fn this_wont_hurt_a_bit(&self) -> &mut Foo {
55
| ^^^^^^^^
66
|
77
note: immutable borrow here
8-
--> tests/ui/mut_from_ref.rs:7:29
8+
--> tests/ui/mut_from_ref.rs:13:29
99
|
1010
LL | fn this_wont_hurt_a_bit(&self) -> &mut Foo {
1111
| ^^^^^
1212
= note: `-D clippy::mut-from-ref` implied by `-D warnings`
1313
= help: to override `-D warnings` add `#[allow(clippy::mut_from_ref)]`
1414

1515
error: mutable borrow from immutable input(s)
16-
--> tests/ui/mut_from_ref.rs:15:25
16+
--> tests/ui/mut_from_ref.rs:21:25
1717
|
1818
LL | fn ouch(x: &Foo) -> &mut Foo;
1919
| ^^^^^^^^
2020
|
2121
note: immutable borrow here
22-
--> tests/ui/mut_from_ref.rs:15:16
22+
--> tests/ui/mut_from_ref.rs:21:16
2323
|
2424
LL | fn ouch(x: &Foo) -> &mut Foo;
2525
| ^^^^
2626

2727
error: mutable borrow from immutable input(s)
28-
--> tests/ui/mut_from_ref.rs:25:21
28+
--> tests/ui/mut_from_ref.rs:31:21
2929
|
3030
LL | fn fail(x: &u32) -> &mut u16 {
3131
| ^^^^^^^^
3232
|
3333
note: immutable borrow here
34-
--> tests/ui/mut_from_ref.rs:25:12
34+
--> tests/ui/mut_from_ref.rs:31:12
3535
|
3636
LL | fn fail(x: &u32) -> &mut u16 {
3737
| ^^^^
3838

3939
error: mutable borrow from immutable input(s)
40-
--> tests/ui/mut_from_ref.rs:31:50
40+
--> tests/ui/mut_from_ref.rs:37:50
4141
|
4242
LL | fn fail_lifetime<'a>(x: &'a u32, y: &mut u32) -> &'a mut u32 {
4343
| ^^^^^^^^^^^
4444
|
4545
note: immutable borrow here
46-
--> tests/ui/mut_from_ref.rs:31:25
46+
--> tests/ui/mut_from_ref.rs:37:25
4747
|
4848
LL | fn fail_lifetime<'a>(x: &'a u32, y: &mut u32) -> &'a mut u32 {
4949
| ^^^^^^^
5050

5151
error: mutable borrow from immutable input(s)
52-
--> tests/ui/mut_from_ref.rs:37:67
52+
--> tests/ui/mut_from_ref.rs:43:67
5353
|
5454
LL | fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 {
5555
| ^^^^^^^^^^^
5656
|
5757
note: immutable borrow here
58-
--> tests/ui/mut_from_ref.rs:37:27
58+
--> tests/ui/mut_from_ref.rs:43:27
5959
|
6060
LL | fn fail_double<'a, 'b>(x: &'a u32, y: &'a u32, z: &'b mut u32) -> &'a mut u32 {
6161
| ^^^^^^^ ^^^^^^^
6262

6363
error: mutable borrow from immutable input(s)
64-
--> tests/ui/mut_from_ref.rs:53:35
64+
--> tests/ui/mut_from_ref.rs:49:46
65+
|
66+
LL | fn fail_tuples<'a>(x: (&'a u32, &'a u32)) -> &'a mut u32 {
67+
| ^^^^^^^^^^^
68+
|
69+
note: immutable borrow here
70+
--> tests/ui/mut_from_ref.rs:49:24
71+
|
72+
LL | fn fail_tuples<'a>(x: (&'a u32, &'a u32)) -> &'a mut u32 {
73+
| ^^^^^^^ ^^^^^^^
74+
75+
error: mutable borrow from immutable input(s)
76+
--> tests/ui/mut_from_ref.rs:55:37
77+
|
78+
LL | fn fail_box<'a>(x: Box<&'a u32>) -> &'a mut u32 {
79+
| ^^^^^^^^^^^
80+
|
81+
note: immutable borrow here
82+
--> tests/ui/mut_from_ref.rs:55:24
83+
|
84+
LL | fn fail_box<'a>(x: Box<&'a u32>) -> &'a mut u32 {
85+
| ^^^^^^^
86+
87+
error: mutable borrow from immutable input(s)
88+
--> tests/ui/mut_from_ref.rs:85:35
6589
|
6690
LL | unsafe fn also_broken(x: &u32) -> &mut u32 {
6791
| ^^^^^^^^
6892
|
6993
note: immutable borrow here
70-
--> tests/ui/mut_from_ref.rs:53:26
94+
--> tests/ui/mut_from_ref.rs:85:26
7195
|
7296
LL | unsafe fn also_broken(x: &u32) -> &mut u32 {
7397
| ^^^^
7498

75-
error: aborting due to 6 previous errors
99+
error: aborting due to 8 previous errors
76100

0 commit comments

Comments
 (0)