Skip to content

Commit bc34aa4

Browse files
estebankestebank
committed
On borrow return type, suggest borrowing from arg or owned return type
When we encounter a function with a return type that has an anonymous lifetime with no argument to borrow from, besides suggesting the `'static` lifetime we now also suggest changing the arguments to be borrows or changing the return type to be an owned type. ``` error[E0106]: missing lifetime specifier --> $DIR/variadic-ffi-6.rs:7:6 | LL | ) -> &usize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | ) -> &'static usize { | +++++++ help: instead, you are more likely to want to change one of the arguments to be borrowed... | LL | x: &usize, | + help: ...or alternatively, to want to return an owned value | LL - ) -> &usize { LL + ) -> usize { | ``` Fix #85843.
1 parent d97bb19 commit bc34aa4

18 files changed

+304
-56
lines changed

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

+126-12
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::{Module, ModuleKind, ModuleOrUniformRoot};
66
use crate::{PathResult, PathSource, Segment};
77
use rustc_hir::def::Namespace::{self, *};
88

9-
use rustc_ast::visit::{FnCtxt, FnKind, LifetimeCtxt};
9+
use rustc_ast::visit::{walk_ty, FnCtxt, FnKind, LifetimeCtxt, Visitor};
1010
use rustc_ast::{
1111
self as ast, AssocItemKind, Expr, ExprKind, GenericParam, GenericParamKind, Item, ItemKind,
1212
MethodCall, NodeId, Path, Ty, TyKind, DUMMY_NODE_ID,
@@ -2612,6 +2612,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
26122612
.collect();
26132613
debug!(?in_scope_lifetimes);
26142614

2615+
let mut maybe_static = false;
26152616
debug!(?function_param_lifetimes);
26162617
if let Some((param_lifetimes, params)) = &function_param_lifetimes {
26172618
let elided_len = param_lifetimes.len();
@@ -2650,9 +2651,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
26502651

26512652
if num_params == 0 {
26522653
err.help(
2653-
"this function's return type contains a borrowed value, \
2654-
but there is no value for it to be borrowed from",
2654+
"this function's return type contains a borrowed value, but there is no value \
2655+
for it to be borrowed from",
26552656
);
2657+
maybe_static = true;
26562658
if in_scope_lifetimes.is_empty() {
26572659
in_scope_lifetimes = vec![(
26582660
Ident::with_dummy_span(kw::StaticLifetime),
@@ -2661,10 +2663,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
26612663
}
26622664
} else if elided_len == 0 {
26632665
err.help(
2664-
"this function's return type contains a borrowed value with \
2665-
an elided lifetime, but the lifetime cannot be derived from \
2666-
the arguments",
2666+
"this function's return type contains a borrowed value with an elided \
2667+
lifetime, but the lifetime cannot be derived from the arguments",
26672668
);
2669+
maybe_static = true;
26682670
if in_scope_lifetimes.is_empty() {
26692671
in_scope_lifetimes = vec![(
26702672
Ident::with_dummy_span(kw::StaticLifetime),
@@ -2673,13 +2675,13 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
26732675
}
26742676
} else if num_params == 1 {
26752677
err.help(format!(
2676-
"this function's return type contains a borrowed value, \
2677-
but the signature does not say which {m} it is borrowed from"
2678+
"this function's return type contains a borrowed value, but the signature does \
2679+
not say which {m} it is borrowed from",
26782680
));
26792681
} else {
26802682
err.help(format!(
2681-
"this function's return type contains a borrowed value, \
2682-
but the signature does not say whether it is borrowed from {m}"
2683+
"this function's return type contains a borrowed value, but the signature does \
2684+
not say whether it is borrowed from {m}",
26832685
));
26842686
}
26852687
}
@@ -2744,11 +2746,107 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
27442746
);
27452747
}
27462748
1 => {
2749+
let post = if maybe_static {
2750+
let owned = if let [lt] = &lifetime_refs[..]
2751+
&& lt.kind != MissingLifetimeKind::Ampersand
2752+
{
2753+
", or if you will only have owned values"
2754+
} else {
2755+
""
2756+
};
2757+
format!(
2758+
", but this is uncommon unless you're returning a borrowed value from a \
2759+
`const` or a `static`{owned}",
2760+
)
2761+
} else {
2762+
String::new()
2763+
};
27472764
err.multipart_suggestion_verbose(
2748-
format!("consider using the `{existing_name}` lifetime"),
2765+
format!("consider using the `{existing_name}` lifetime{post}"),
27492766
spans_suggs,
27502767
Applicability::MaybeIncorrect,
27512768
);
2769+
if maybe_static {
2770+
// FIXME: what follows are general suggestions, but we'd want to perform some
2771+
// minimal flow analysis to provide more accurate suggestions. For example, if
2772+
// we identified that the return expression references only one argument, we
2773+
// would suggest borrowing only that argument, and we'd skip the prior
2774+
// "use `'static`" suggestion entirely.
2775+
if let [lt] = &lifetime_refs[..] && lt.kind == MissingLifetimeKind::Ampersand {
2776+
let pre = if let Some((kind, _span)) =
2777+
self.diagnostic_metadata.current_function
2778+
&& let FnKind::Fn(_, _, sig, _, _, _) = kind
2779+
&& !sig.decl.inputs.is_empty()
2780+
&& let sugg = sig
2781+
.decl
2782+
.inputs
2783+
.iter()
2784+
.filter_map(|param| {
2785+
if param.ty.span.contains(lt.span) {
2786+
// We don't want to suggest `fn elision(_: &fn() -> &i32)`
2787+
// when we have `fn elision(_: fn() -> &i32)`
2788+
None
2789+
} else if let TyKind::CVarArgs = param.ty.kind {
2790+
// Don't suggest `&...` for ffi fn with varargs
2791+
None
2792+
} else {
2793+
Some((param.ty.span.shrink_to_lo(), "&".to_string()))
2794+
}
2795+
})
2796+
.collect::<Vec<_>>()
2797+
&& !sugg.is_empty()
2798+
{
2799+
2800+
let (the, s) = if sig.decl.inputs.len() == 1 {
2801+
("the", "")
2802+
} else {
2803+
("one of the", "s")
2804+
};
2805+
err.multipart_suggestion_verbose(
2806+
format!(
2807+
"instead, you are more likely to want to change {the} \
2808+
argument{s} to be borrowed...",
2809+
),
2810+
sugg,
2811+
Applicability::MaybeIncorrect,
2812+
);
2813+
"...or alternatively,"
2814+
} else {
2815+
"instead, you are more likely"
2816+
};
2817+
let mut sugg = vec![(lt.span, String::new())];
2818+
if let Some((kind, _span)) =
2819+
self.diagnostic_metadata.current_function
2820+
&& let FnKind::Fn(_, _, sig, _, _, _) = kind
2821+
&& let ast::FnRetTy::Ty(ty) = &sig.decl.output
2822+
{
2823+
let mut lt_finder = LifetimeFinder { lifetime: lt.span, found: None };
2824+
lt_finder.visit_ty(&ty);
2825+
2826+
if let Some(ty) = lt_finder.found {
2827+
if let TyKind::Path(None, Path { segments, .. }) = &ty.kind
2828+
&& segments.len() == 1
2829+
&& segments[0].ident.name == sym::str
2830+
{
2831+
// Don't suggest `-> str`, suggest `-> String`.
2832+
sugg = vec![(lt.span.with_hi(ty.span.hi()), "String".to_string())];
2833+
}
2834+
if let TyKind::Slice(inner_ty) = &ty.kind {
2835+
// Don't suggest `-> [T]`, suggest `-> Vec<T>`.
2836+
sugg = vec![
2837+
(lt.span.with_hi(inner_ty.span.lo()), "Vec<".to_string()),
2838+
(ty.span.with_lo(inner_ty.span.hi()), ">".to_string()),
2839+
];
2840+
}
2841+
}
2842+
};
2843+
err.multipart_suggestion_verbose(
2844+
format!("{pre} to want to return an owned value"),
2845+
sugg,
2846+
Applicability::MaybeIncorrect,
2847+
);
2848+
}
2849+
}
27522850

27532851
// Record as using the suggested resolution.
27542852
let (_, (_, res)) = in_scope_lifetimes[0];
@@ -2778,7 +2876,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
27782876
fn mk_where_bound_predicate(
27792877
path: &Path,
27802878
poly_trait_ref: &ast::PolyTraitRef,
2781-
ty: &ast::Ty,
2879+
ty: &Ty,
27822880
) -> Option<ast::WhereBoundPredicate> {
27832881
use rustc_span::DUMMY_SP;
27842882
let modified_segments = {
@@ -2855,6 +2953,22 @@ pub(super) fn signal_lifetime_shadowing(sess: &Session, orig: Ident, shadower: I
28552953
err.emit();
28562954
}
28572955

2956+
struct LifetimeFinder<'ast> {
2957+
lifetime: Span,
2958+
found: Option<&'ast Ty>,
2959+
}
2960+
2961+
impl<'ast> Visitor<'ast> for LifetimeFinder<'ast> {
2962+
fn visit_ty(&mut self, t: &'ast Ty) {
2963+
if t.span.lo() == self.lifetime.lo()
2964+
&& let TyKind::Ref(_, mut_ty) = &t.kind
2965+
{
2966+
self.found = Some(&mut_ty.ty);
2967+
}
2968+
walk_ty(self, t)
2969+
}
2970+
}
2971+
28582972
/// Shadowing involving a label is only a warning for historical reasons.
28592973
//FIXME: make this a proper lint.
28602974
pub(super) fn signal_label_shadowing(sess: &Session, orig: Span, shadower: Ident) {

‎tests/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr

+6-1
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,15 @@ LL | fn elision<T: Fn() -> &i32>() {
55
| ^ expected named lifetime parameter
66
|
77
= 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
8+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
99
|
1010
LL | fn elision<T: Fn() -> &'static i32>() {
1111
| +++++++
12+
help: instead, you are more likely to want to return an owned value
13+
|
14+
LL - fn elision<T: Fn() -> &i32>() {
15+
LL + fn elision<T: Fn() -> i32>() {
16+
|
1217

1318
error: aborting due to previous error
1419

‎tests/ui/associated-types/bound-lifetime-in-return-only.elision.stderr

+6-1
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,15 @@ LL | fn elision(_: fn() -> &i32) {
55
| ^ expected named lifetime parameter
66
|
77
= 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
8+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
99
|
1010
LL | fn elision(_: fn() -> &'static i32) {
1111
| +++++++
12+
help: instead, you are more likely to want to return an owned value
13+
|
14+
LL - fn elision(_: fn() -> &i32) {
15+
LL + fn elision(_: fn() -> i32) {
16+
|
1217

1318
error: aborting due to previous error
1419

‎tests/ui/c-variadic/variadic-ffi-6.stderr

+10-1
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,19 @@ LL | ) -> &usize {
55
| ^ expected named lifetime parameter
66
|
77
= 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
8+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
99
|
1010
LL | ) -> &'static usize {
1111
| +++++++
12+
help: instead, you are more likely to want to change one of the arguments to be borrowed...
13+
|
14+
LL | x: &usize,
15+
| +
16+
help: ...or alternatively, to want to return an owned value
17+
|
18+
LL - ) -> &usize {
19+
LL + ) -> usize {
20+
|
1221

1322
error: aborting due to previous error
1423

‎tests/ui/foreign-fn-return-lifetime.fixed

-8
This file was deleted.

‎tests/ui/foreign-fn-return-lifetime.rs

-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
// run-rustfix
2-
31
extern "C" {
42
pub fn g(_: &u8) -> &u8; // OK
53
pub fn f() -> &u8; //~ ERROR missing lifetime specifier

‎tests/ui/foreign-fn-return-lifetime.stderr

+7-2
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
error[E0106]: missing lifetime specifier
2-
--> $DIR/foreign-fn-return-lifetime.rs:5:19
2+
--> $DIR/foreign-fn-return-lifetime.rs:3:19
33
|
44
LL | pub fn f() -> &u8;
55
| ^ expected named lifetime parameter
66
|
77
= 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
8+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
99
|
1010
LL | pub fn f() -> &'static u8;
1111
| +++++++
12+
help: instead, you are more likely to want to return an owned value
13+
|
14+
LL - pub fn f() -> &u8;
15+
LL + pub fn f() -> u8;
16+
|
1217

1318
error: aborting due to previous error
1419

‎tests/ui/generic-associated-types/issue-70304.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ LL | fn create_doc() -> impl Document<Cursor<'_> = DocCursorImpl<'_>> {
1111
| ^^ expected named lifetime parameter
1212
|
1313
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
14-
help: consider using the `'static` lifetime
14+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values
1515
|
1616
LL | fn create_doc() -> impl Document<Cursor<'_> = DocCursorImpl<'static>> {
1717
| ~~~~~~~

‎tests/ui/impl-trait/impl-fn-hrtb-bounds.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ LL | fn d() -> impl Fn() -> (impl Debug + '_) {
55
| ^^ expected named lifetime parameter
66
|
77
= 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
8+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values
99
|
1010
LL | fn d() -> impl Fn() -> (impl Debug + 'static) {
1111
| ~~~~~~~

‎tests/ui/issues/issue-13497.stderr

+5-1
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,14 @@ LL | &str
55
| ^ expected named lifetime parameter
66
|
77
= 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
8+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
99
|
1010
LL | &'static str
1111
| +++++++
12+
help: instead, you are more likely to want to return an owned value
13+
|
14+
LL | String
15+
| ~~~~~~
1216

1317
error: aborting due to previous error
1418

‎tests/ui/lifetimes/issue-26638.stderr

+14-2
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,18 @@ LL | fn parse_type_2(iter: fn(&u8)->&u8) -> &str { iter() }
1717
| ^ expected named lifetime parameter
1818
|
1919
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
20-
help: consider using the `'static` lifetime
20+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
2121
|
2222
LL | fn parse_type_2(iter: fn(&u8)->&u8) -> &'static str { iter() }
2323
| +++++++
24+
help: instead, you are more likely to want to change the argument to be borrowed...
25+
|
26+
LL | fn parse_type_2(iter: &fn(&u8)->&u8) -> &str { iter() }
27+
| +
28+
help: ...or alternatively, to want to return an owned value
29+
|
30+
LL | fn parse_type_2(iter: fn(&u8)->&u8) -> String { iter() }
31+
| ~~~~~~
2432

2533
error[E0106]: missing lifetime specifier
2634
--> $DIR/issue-26638.rs:10:22
@@ -29,10 +37,14 @@ LL | fn parse_type_3() -> &str { unimplemented!() }
2937
| ^ expected named lifetime parameter
3038
|
3139
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
32-
help: consider using the `'static` lifetime
40+
help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`
3341
|
3442
LL | fn parse_type_3() -> &'static str { unimplemented!() }
3543
| +++++++
44+
help: instead, you are more likely to want to return an owned value
45+
|
46+
LL | fn parse_type_3() -> String { unimplemented!() }
47+
| ~~~~~~
3648

3749
error[E0308]: mismatched types
3850
--> $DIR/issue-26638.rs:1:69

0 commit comments

Comments
 (0)