Skip to content

Commit e0d9d47

Browse files
reddevilmidzyreddevilmidzy
committed
Fix invalid mut T suggestion for &mut T in missing lifetime error
* Find ref prefix span for owned suggestions * Improve missing lifetime suggestions for `&mut str`
1 parent 466ea4e commit e0d9d47

3 files changed

Lines changed: 231 additions & 13 deletions

File tree

‎compiler/rustc_resolve/src/late/diagnostics.rs‎

Lines changed: 65 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3837,25 +3837,32 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
38373837
"instead, you are more likely to want"
38383838
};
38393839
let mut owned_sugg = lt.kind == MissingLifetimeKind::Ampersand;
3840+
let mut sugg_is_str_to_string = false;
38403841
let mut sugg = vec![(lt.span, String::new())];
38413842
if let Some((kind, _span)) = self.diag_metadata.current_function
38423843
&& let FnKind::Fn(_, _, ast::Fn { sig, .. }) = kind
3843-
&& let ast::FnRetTy::Ty(ty) = &sig.decl.output
38443844
{
38453845
let mut lt_finder =
38463846
LifetimeFinder { lifetime: lt.span, found: None, seen: vec![] };
3847-
lt_finder.visit_ty(&ty);
3848-
3849-
if let [Ty { span, kind: TyKind::Ref(_, mut_ty), .. }] =
3850-
&lt_finder.seen[..]
3851-
{
3852-
// We might have a situation like
3853-
// fn g(mut x: impl Iterator<Item = &'_ ()>) -> Option<&'_ ()>
3854-
// but `lt.span` only points at `'_`, so to suggest `-> Option<()>`
3855-
// we need to find a more accurate span to end up with
3856-
// fn g<'a>(mut x: impl Iterator<Item = &'_ ()>) -> Option<()>
3857-
sugg = vec![(span.with_hi(mut_ty.ty.span.lo()), String::new())];
3858-
owned_sugg = true;
3847+
for param in &sig.decl.inputs {
3848+
lt_finder.visit_ty(&param.ty);
3849+
}
3850+
if let ast::FnRetTy::Ty(ret_ty) = &sig.decl.output {
3851+
lt_finder.visit_ty(ret_ty);
3852+
let mut ret_lt_finder =
3853+
LifetimeFinder { lifetime: lt.span, found: None, seen: vec![] };
3854+
ret_lt_finder.visit_ty(ret_ty);
3855+
if let [Ty { span, kind: TyKind::Ref(_, mut_ty), .. }] =
3856+
&ret_lt_finder.seen[..]
3857+
{
3858+
// We might have a situation like
3859+
// fn g(mut x: impl Iterator<Item = &'_ ()>) -> Option<&'_ ()>
3860+
// but `lt.span` only points at `'_`, so to suggest `-> Option<()>`
3861+
// we need to find a more accurate span to end up with
3862+
// fn g<'a>(mut x: impl Iterator<Item = &'_ ()>) -> Option<()>
3863+
sugg = vec![(span.with_hi(mut_ty.ty.span.lo()), String::new())];
3864+
owned_sugg = true;
3865+
}
38593866
}
38603867
if let Some(ty) = lt_finder.found {
38613868
if let TyKind::Path(None, path) = &ty.kind {
@@ -3875,6 +3882,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
38753882
lt.span.with_hi(ty.span.hi()),
38763883
"String".to_string(),
38773884
)];
3885+
sugg_is_str_to_string = true;
38783886
}
38793887
Some(Res::PrimTy(..)) => {}
38803888
Some(Res::Def(
@@ -3901,6 +3909,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
39013909
lt.span.with_hi(ty.span.hi()),
39023910
"String".to_string(),
39033911
)];
3912+
sugg_is_str_to_string = true;
39043913
}
39053914
Res::PrimTy(..) => {}
39063915
Res::Def(
@@ -3935,6 +3944,12 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
39353944
}
39363945
}
39373946
if owned_sugg {
3947+
if let Some(span) =
3948+
self.find_ref_prefix_span_for_owned_suggestion(lt.span)
3949+
&& !sugg_is_str_to_string
3950+
{
3951+
sugg = vec![(span, String::new())];
3952+
}
39383953
err.multipart_suggestion_verbose(
39393954
format!("{pre} to return an owned value"),
39403955
sugg,
@@ -3961,6 +3976,23 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
39613976
}
39623977
}
39633978
}
3979+
3980+
fn find_ref_prefix_span_for_owned_suggestion(&self, lifetime: Span) -> Option<Span> {
3981+
let mut finder = RefPrefixSpanFinder { lifetime, span: None };
3982+
if let Some(item) = self.diag_metadata.current_item {
3983+
finder.visit_item(item);
3984+
} else if let Some((kind, _span)) = self.diag_metadata.current_function
3985+
&& let FnKind::Fn(_, _, ast::Fn { sig, .. }) = kind
3986+
{
3987+
for param in &sig.decl.inputs {
3988+
finder.visit_ty(&param.ty);
3989+
}
3990+
if let ast::FnRetTy::Ty(ret_ty) = &sig.decl.output {
3991+
finder.visit_ty(ret_ty);
3992+
}
3993+
}
3994+
finder.span
3995+
}
39643996
}
39653997

39663998
fn mk_where_bound_predicate(
@@ -4058,6 +4090,26 @@ impl<'ast> Visitor<'ast> for LifetimeFinder<'ast> {
40584090
}
40594091
}
40604092

4093+
struct RefPrefixSpanFinder {
4094+
lifetime: Span,
4095+
span: Option<Span>,
4096+
}
4097+
4098+
impl<'ast> Visitor<'ast> for RefPrefixSpanFinder {
4099+
fn visit_ty(&mut self, t: &'ast Ty) {
4100+
if self.span.is_some() {
4101+
return;
4102+
}
4103+
if let TyKind::Ref(_, mut_ty) | TyKind::PinnedRef(_, mut_ty) = &t.kind
4104+
&& t.span.lo() == self.lifetime.lo()
4105+
{
4106+
self.span = Some(t.span.with_hi(mut_ty.ty.span.lo()));
4107+
return;
4108+
}
4109+
walk_ty(self, t);
4110+
}
4111+
}
4112+
40614113
/// Shadowing involving a label is only a warning for historical reasons.
40624114
//FIXME: make this a proper lint.
40634115
pub(super) fn signal_label_shadowing(sess: &Session, orig: Span, shadower: Ident) {

‎tests/ui/lifetimes/mut-ref-owned-suggestion.rs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//! Regression test for <https://github.com/rust-lang/rust/issues/150077>
2+
//! Tests that `&mut T` suggests `T`, not `mut T`, `&mut str` suggests `String`, not `str`,
3+
//! when recommending an owned value.
4+
fn with_fn(_f: impl Fn() -> &mut ()) {}
5+
//~^ ERROR: missing lifetime specifier
6+
7+
fn with_ref_mut_str(_f: impl Fn() -> &mut str) {}
8+
//~^ ERROR: missing lifetime specifier
9+
10+
fn with_fn_has_return(_f: impl Fn() -> &mut ()) -> i32 {
11+
//~^ ERROR: missing lifetime specifier
12+
2
13+
}
14+
15+
fn with_dyn(_f: Box<dyn Fn() -> &mut i32>) {}
16+
//~^ ERROR: missing lifetime specifier
17+
18+
fn trait_bound<F: Fn() -> &mut i32>(_f: F) {}
19+
//~^ ERROR: missing lifetime specifier
20+
21+
fn nested_result(_f: impl Fn() -> Result<&mut i32, ()>) {}
22+
//~^ ERROR: missing lifetime specifier
23+
24+
struct Holder<F: Fn() -> &mut i32> {
25+
//~^ ERROR: missing lifetime specifier
26+
f: F,
27+
}
28+
29+
fn main() {}

‎tests/ui/lifetimes/mut-ref-owned-suggestion.stderr‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
error[E0106]: missing lifetime specifier
2+
--> $DIR/mut-ref-owned-suggestion.rs:4:29
3+
|
4+
LL | fn with_fn(_f: impl Fn() -> &mut ()) {}
5+
| ^ expected named lifetime parameter
6+
|
7+
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
8+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
9+
|
10+
LL | fn with_fn(_f: impl Fn() -> &'static mut ()) {}
11+
| +++++++
12+
help: instead, you are more likely to want to return an owned value
13+
|
14+
LL - fn with_fn(_f: impl Fn() -> &mut ()) {}
15+
LL + fn with_fn(_f: impl Fn() -> ()) {}
16+
|
17+
18+
error[E0106]: missing lifetime specifier
19+
--> $DIR/mut-ref-owned-suggestion.rs:7:38
20+
|
21+
LL | fn with_ref_mut_str(_f: impl Fn() -> &mut str) {}
22+
| ^ expected named lifetime parameter
23+
|
24+
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
25+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
26+
|
27+
LL | fn with_ref_mut_str(_f: impl Fn() -> &'static mut str) {}
28+
| +++++++
29+
help: instead, you are more likely to want to return an owned value
30+
|
31+
LL - fn with_ref_mut_str(_f: impl Fn() -> &mut str) {}
32+
LL + fn with_ref_mut_str(_f: impl Fn() -> String) {}
33+
|
34+
35+
error[E0106]: missing lifetime specifier
36+
--> $DIR/mut-ref-owned-suggestion.rs:10:40
37+
|
38+
LL | fn with_fn_has_return(_f: impl Fn() -> &mut ()) -> i32 {
39+
| ^ expected named lifetime parameter
40+
|
41+
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
42+
= note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
43+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
44+
|
45+
LL | fn with_fn_has_return(_f: impl Fn() -> &'static mut ()) -> i32 {
46+
| +++++++
47+
help: consider making the bound lifetime-generic with a new `'a` lifetime
48+
|
49+
LL - fn with_fn_has_return(_f: impl Fn() -> &mut ()) -> i32 {
50+
LL + fn with_fn_has_return(_f: impl for<'a> Fn() -> &'a ()) -> i32 {
51+
|
52+
help: consider introducing a named lifetime parameter
53+
|
54+
LL - fn with_fn_has_return(_f: impl Fn() -> &mut ()) -> i32 {
55+
LL + fn with_fn_has_return<'a>(_f: impl Fn() -> &'a ()) -> i32 {
56+
|
57+
help: alternatively, you might want to return an owned value
58+
|
59+
LL - fn with_fn_has_return(_f: impl Fn() -> &mut ()) -> i32 {
60+
LL + fn with_fn_has_return(_f: impl Fn() -> ()) -> i32 {
61+
|
62+
63+
error[E0106]: missing lifetime specifier
64+
--> $DIR/mut-ref-owned-suggestion.rs:15:33
65+
|
66+
LL | fn with_dyn(_f: Box<dyn Fn() -> &mut i32>) {}
67+
| ^ expected named lifetime parameter
68+
|
69+
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
70+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
71+
|
72+
LL | fn with_dyn(_f: Box<dyn Fn() -> &'static mut i32>) {}
73+
| +++++++
74+
help: instead, you are more likely to want to return an owned value
75+
|
76+
LL - fn with_dyn(_f: Box<dyn Fn() -> &mut i32>) {}
77+
LL + fn with_dyn(_f: Box<dyn Fn() -> i32>) {}
78+
|
79+
80+
error[E0106]: missing lifetime specifier
81+
--> $DIR/mut-ref-owned-suggestion.rs:18:27
82+
|
83+
LL | fn trait_bound<F: Fn() -> &mut i32>(_f: F) {}
84+
| ^ expected named lifetime parameter
85+
|
86+
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
87+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
88+
|
89+
LL | fn trait_bound<F: Fn() -> &'static mut i32>(_f: F) {}
90+
| +++++++
91+
help: instead, you are more likely to want to change the argument to be borrowed...
92+
|
93+
LL | fn trait_bound<F: Fn() -> &mut i32>(_f: &F) {}
94+
| +
95+
help: ...or alternatively, you might want to return an owned value
96+
|
97+
LL - fn trait_bound<F: Fn() -> &mut i32>(_f: F) {}
98+
LL + fn trait_bound<F: Fn() -> i32>(_f: F) {}
99+
|
100+
101+
error[E0106]: missing lifetime specifier
102+
--> $DIR/mut-ref-owned-suggestion.rs:21:42
103+
|
104+
LL | fn nested_result(_f: impl Fn() -> Result<&mut i32, ()>) {}
105+
| ^ expected named lifetime parameter
106+
|
107+
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
108+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
109+
|
110+
LL | fn nested_result(_f: impl Fn() -> Result<&'static mut i32, ()>) {}
111+
| +++++++
112+
help: instead, you are more likely to want to return an owned value
113+
|
114+
LL - fn nested_result(_f: impl Fn() -> Result<&mut i32, ()>) {}
115+
LL + fn nested_result(_f: impl Fn() -> Result<i32, ()>) {}
116+
|
117+
118+
error[E0106]: missing lifetime specifier
119+
--> $DIR/mut-ref-owned-suggestion.rs:24:26
120+
|
121+
LL | struct Holder<F: Fn() -> &mut i32> {
122+
| ^ expected named lifetime parameter
123+
|
124+
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
125+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
126+
|
127+
LL | struct Holder<F: Fn() -> &'static mut i32> {
128+
| +++++++
129+
help: instead, you are more likely to want to return an owned value
130+
|
131+
LL - struct Holder<F: Fn() -> &mut i32> {
132+
LL + struct Holder<F: Fn() -> i32> {
133+
|
134+
135+
error: aborting due to 7 previous errors
136+
137+
For more information about this error, try `rustc --explain E0106`.

0 commit comments

Comments
 (0)