Skip to content

Commit 5753b30

Browse files
borsbors
committed
Auto merge of #117967 - adetaylor:fix-lifetime-elision-bug, r=lcnr
Fix ambiguous cases of multiple & in elided self lifetimes This change proposes simpler rules to identify the lifetime on `self` parameters which may be used to elide a return type lifetime. ## The old rules (copied from [this comment](#117967 (comment))) Most of the code can be found in [late.rs](https://doc.rust-lang.org/stable/nightly-rustc/src/rustc_resolve/late.rs.html) and acts on AST types. The function [resolve_fn_params](https://doc.rust-lang.org/stable/nightly-rustc/src/rustc_resolve/late.rs.html#2006), in the success case, returns a single lifetime which can be used to elide the lifetime of return types. Here's how: * If the first parameter is called self then we search that parameter using "`self` search rules", below * If no unique applicable lifetime was found, search all other parameters using "regular parameter search rules", below (In practice the code does extra work to assemble good diagnostic information, so it's not quite laid out like the above.) ### `self` search rules This is primarily handled in [find_lifetime_for_self](https://doc.rust-lang.org/stable/nightly-rustc/src/rustc_resolve/late.rs.html#2118) , and is described slightly [here](#117715 (comment)) already. The code: 1. Recursively walks the type of the `self` parameter (there's some complexity about resolving various special cases, but it's essentially just walking the type as far as I can see) 2. Each time we find a reference anywhere in the type, if the **direct** referent is `Self` (either spelled `Self` or by some alias resolution which I don't fully understand), then we'll add that to a set of candidate lifetimes 3. If there's exactly one such unique lifetime candidate found, we return this lifetime. ### Regular parameter search rules 1. Find all the lifetimes in each parameter, including implicit, explicit etc. 2. If there's exactly one parameter containing lifetimes, and if that parameter contains exactly one (unique) lifetime, *and if we didn't find a `self` lifetime parameter already*, we'll return this lifetime. ## The new rules There are no changes to the "regular parameter search rules" or to the overall flow, only to the `self` search rules which are now: 1. Recursively walks the type of the `self` parameter, searching for lifetimes of reference types whose referent **contains** `Self`.[^1] 2. Keep a record of: * Whether 0, 1 or n unique lifetimes are found on references encountered during the walk 4. If no lifetime was found, we don't return a lifetime. (This means other parameters' lifetimes may be used for return type lifetime elision). 5. If there's one lifetime found, we return the lifetime. 6. If multiple lifetimes were found, we abort elision entirely (other parameters' lifetimes won't be used). [^1]: this prevents us from considering lifetimes from inside of the self-type ## Examples that were accepted before and will now be rejected ```rust fn a(self: &Box<&Self>) -> &u32 fn b(self: &Pin<&mut Self>) -> &String fn c(self: &mut &Self) -> Option<&Self> fn d(self: &mut &Box<Self>, arg: &usize) -> &usize // previously used the lt from arg ``` ### Examples that change the elided lifetime ```rust fn e(self: &mut Box<Self>, arg: &usize) -> &usize // ^ new ^ previous ``` ## Examples that were rejected before and will now be accepted ```rust fn f(self: &Box<Self>) -> &u32 ``` --- *edit: old PR description:* ```rust struct Concrete(u32); impl Concrete { fn m(self: &Box<Self>) -> &u32 { &self.0 } } ``` resulted in a confusing error. ```rust impl Concrete { fn n(self: &Box<&Self>) -> &u32 { &self.0 } } ``` resulted in no error or warning, despite apparent ambiguity over the elided lifetime. Fixes #117715
2 parents b01a977 + a22130e commit 5753b30

19 files changed

+522
-47
lines changed

‎compiler/rustc_resolve/src/late.rs

+56-22
Original file line numberDiff line numberDiff line change
@@ -2175,13 +2175,17 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
21752175
// Handle `self` specially.
21762176
if index == 0 && has_self {
21772177
let self_lifetime = self.find_lifetime_for_self(ty);
2178-
if let Set1::One(lifetime) = self_lifetime {
2178+
elision_lifetime = match self_lifetime {
21792179
// We found `self` elision.
2180-
elision_lifetime = Elision::Self_(lifetime);
2181-
} else {
2180+
Set1::One(lifetime) => Elision::Self_(lifetime),
2181+
// `self` itself had ambiguous lifetimes, e.g.
2182+
// &Box<&Self>. In this case we won't consider
2183+
// taking an alternative parameter lifetime; just avoid elision
2184+
// entirely.
2185+
Set1::Many => Elision::Err,
21822186
// We do not have `self` elision: disregard the `Elision::Param` that we may
21832187
// have found.
2184-
elision_lifetime = Elision::None;
2188+
Set1::Empty => Elision::None,
21852189
}
21862190
}
21872191
debug!("(resolving function / closure) recorded parameter");
@@ -2201,15 +2205,55 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
22012205

22022206
/// List all the lifetimes that appear in the provided type.
22032207
fn find_lifetime_for_self(&self, ty: &'ast Ty) -> Set1<LifetimeRes> {
2204-
struct SelfVisitor<'r, 'a, 'tcx> {
2208+
/// Visits a type to find all the &references, and determines the
2209+
/// set of lifetimes for all of those references where the referent
2210+
/// contains Self.
2211+
struct FindReferenceVisitor<'r, 'a, 'tcx> {
22052212
r: &'r Resolver<'a, 'tcx>,
22062213
impl_self: Option<Res>,
22072214
lifetime: Set1<LifetimeRes>,
22082215
}
22092216

2217+
impl<'a> Visitor<'a> for FindReferenceVisitor<'_, '_, '_> {
2218+
fn visit_ty(&mut self, ty: &'a Ty) {
2219+
trace!("FindReferenceVisitor considering ty={:?}", ty);
2220+
if let TyKind::Ref(lt, _) = ty.kind {
2221+
// See if anything inside the &thing contains Self
2222+
let mut visitor =
2223+
SelfVisitor { r: self.r, impl_self: self.impl_self, self_found: false };
2224+
visitor.visit_ty(ty);
2225+
trace!("FindReferenceVisitor: SelfVisitor self_found={:?}", visitor.self_found);
2226+
if visitor.self_found {
2227+
let lt_id = if let Some(lt) = lt {
2228+
lt.id
2229+
} else {
2230+
let res = self.r.lifetimes_res_map[&ty.id];
2231+
let LifetimeRes::ElidedAnchor { start, .. } = res else { bug!() };
2232+
start
2233+
};
2234+
let lt_res = self.r.lifetimes_res_map[&lt_id];
2235+
trace!("FindReferenceVisitor inserting res={:?}", lt_res);
2236+
self.lifetime.insert(lt_res);
2237+
}
2238+
}
2239+
visit::walk_ty(self, ty)
2240+
}
2241+
2242+
// A type may have an expression as a const generic argument.
2243+
// We do not want to recurse into those.
2244+
fn visit_expr(&mut self, _: &'a Expr) {}
2245+
}
2246+
2247+
/// Visitor which checks the referent of a &Thing to see if the
2248+
/// Thing contains Self
2249+
struct SelfVisitor<'r, 'a, 'tcx> {
2250+
r: &'r Resolver<'a, 'tcx>,
2251+
impl_self: Option<Res>,
2252+
self_found: bool,
2253+
}
2254+
22102255
impl SelfVisitor<'_, '_, '_> {
2211-
// Look for `self: &'a Self` - also desugared from `&'a self`,
2212-
// and if that matches, use it for elision and return early.
2256+
// Look for `self: &'a Self` - also desugared from `&'a self`
22132257
fn is_self_ty(&self, ty: &Ty) -> bool {
22142258
match ty.kind {
22152259
TyKind::ImplicitSelf => true,
@@ -2228,19 +2272,9 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
22282272
impl<'a> Visitor<'a> for SelfVisitor<'_, '_, '_> {
22292273
fn visit_ty(&mut self, ty: &'a Ty) {
22302274
trace!("SelfVisitor considering ty={:?}", ty);
2231-
if let TyKind::Ref(lt, ref mt) = ty.kind
2232-
&& self.is_self_ty(&mt.ty)
2233-
{
2234-
let lt_id = if let Some(lt) = lt {
2235-
lt.id
2236-
} else {
2237-
let res = self.r.lifetimes_res_map[&ty.id];
2238-
let LifetimeRes::ElidedAnchor { start, .. } = res else { bug!() };
2239-
start
2240-
};
2241-
let lt_res = self.r.lifetimes_res_map[&lt_id];
2242-
trace!("SelfVisitor inserting res={:?}", lt_res);
2243-
self.lifetime.insert(lt_res);
2275+
if self.is_self_ty(ty) {
2276+
trace!("SelfVisitor found Self");
2277+
self.self_found = true;
22442278
}
22452279
visit::walk_ty(self, ty)
22462280
}
@@ -2271,9 +2305,9 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
22712305
Res::Def(DefKind::Struct | DefKind::Union | DefKind::Enum, _,) | Res::PrimTy(_)
22722306
)
22732307
});
2274-
let mut visitor = SelfVisitor { r: self.r, impl_self, lifetime: Set1::Empty };
2308+
let mut visitor = FindReferenceVisitor { r: self.r, impl_self, lifetime: Set1::Empty };
22752309
visitor.visit_ty(ty);
2276-
trace!("SelfVisitor found={:?}", visitor.lifetime);
2310+
trace!("FindReferenceVisitor found={:?}", visitor.lifetime);
22772311
visitor.lifetime
22782312
}
22792313

‎tests/crashes/122903-1.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//@ known-bug: #122903
22
impl Struct {
3-
async fn box_box_ref_Struct(
4-
self: Box<Box<Self, impl FnMut(&mut Box<Box<Self, impl FnMut(&mut Self)>>)>>,
3+
fn box_box_ref_Struct(
4+
self: impl FnMut(Box<impl FnMut(&mut Self)>),
55
) -> &u32 {
66
f
77
}

‎tests/crashes/122903-2.rs

-9
This file was deleted.

‎tests/ui/self/elision/ignore-non-reference-lifetimes.rs

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//@ check-pass
2+
3+
struct Foo<'a>(&'a str);
4+
5+
impl<'b> Foo<'b> {
6+
fn a<'a>(self: Self, a: &'a str) -> &str {
7+
a
8+
}
9+
fn b<'a>(self: Foo<'b>, a: &'a str) -> &str {
10+
a
11+
}
12+
}
13+
14+
struct Foo2<'a>(&'a u32);
15+
impl<'a> Foo2<'a> {
16+
fn foo(self: &Self) -> &u32 { self.0 } // ok
17+
fn bar(self: &Foo2<'a>) -> &u32 { self.0 } // ok (do not look into `Foo`)
18+
fn baz2(self: Self, arg: &u32) -> &u32 { arg } // use lt from `arg`
19+
fn baz3(self: Foo2<'a>, arg: &u32) -> &u32 { arg } // use lt from `arg`
20+
}
21+
22+
fn main() {}

‎tests/ui/self/elision/multiple-ref-self-async.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
//@ check-pass
21
//@ edition:2018
32

43
#![feature(arbitrary_self_types)]
@@ -21,22 +20,27 @@ impl Struct {
2120
// Test using multiple `&Self`:
2221

2322
async fn wrap_ref_Self_ref_Self(self: Wrap<&Self, &Self>, f: &u8) -> &u8 {
23+
//~^ ERROR missing lifetime specifier
2424
f
2525
}
2626

2727
async fn box_wrap_ref_Self_ref_Self(self: Box<Wrap<&Self, &Self>>, f: &u32) -> &u32 {
28+
//~^ ERROR missing lifetime specifier
2829
f
2930
}
3031

3132
async fn pin_wrap_ref_Self_ref_Self(self: Pin<Wrap<&Self, &Self>>, f: &u32) -> &u32 {
33+
//~^ ERROR missing lifetime specifier
3234
f
3335
}
3436

3537
async fn box_box_wrap_ref_Self_ref_Self(self: Box<Box<Wrap<&Self, &Self>>>, f: &u32) -> &u32 {
38+
//~^ ERROR missing lifetime specifier
3639
f
3740
}
3841

3942
async fn box_pin_wrap_ref_Self_ref_Self(self: Box<Pin<Wrap<&Self, &Self>>>, f: &u32) -> &u32 {
43+
//~^ ERROR missing lifetime specifier
4044
f
4145
}
4246
}

‎tests/ui/self/elision/multiple-ref-self-async.stderr

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
error[E0106]: missing lifetime specifier
2+
--> $DIR/multiple-ref-self-async.rs:22:74
3+
|
4+
LL | async fn wrap_ref_Self_ref_Self(self: Wrap<&Self, &Self>, f: &u8) -> &u8 {
5+
| ------------------ --- ^ expected named lifetime parameter
6+
|
7+
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `self`'s 2 lifetimes or `f`
8+
help: consider introducing a named lifetime parameter
9+
|
10+
LL | async fn wrap_ref_Self_ref_Self<'a>(self: Wrap<&'a Self, &'a Self>, f: &'a u8) -> &'a u8 {
11+
| ++++ ++ ++ ++ ++
12+
13+
error[E0106]: missing lifetime specifier
14+
--> $DIR/multiple-ref-self-async.rs:27:84
15+
|
16+
LL | async fn box_wrap_ref_Self_ref_Self(self: Box<Wrap<&Self, &Self>>, f: &u32) -> &u32 {
17+
| ----------------------- ---- ^ expected named lifetime parameter
18+
|
19+
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `self`'s 2 lifetimes or `f`
20+
help: consider introducing a named lifetime parameter
21+
|
22+
LL | async fn box_wrap_ref_Self_ref_Self<'a>(self: Box<Wrap<&'a Self, &'a Self>>, f: &'a u32) -> &'a u32 {
23+
| ++++ ++ ++ ++ ++
24+
25+
error[E0106]: missing lifetime specifier
26+
--> $DIR/multiple-ref-self-async.rs:32:84
27+
|
28+
LL | async fn pin_wrap_ref_Self_ref_Self(self: Pin<Wrap<&Self, &Self>>, f: &u32) -> &u32 {
29+
| ----------------------- ---- ^ expected named lifetime parameter
30+
|
31+
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `self`'s 2 lifetimes or `f`
32+
help: consider introducing a named lifetime parameter
33+
|
34+
LL | async fn pin_wrap_ref_Self_ref_Self<'a>(self: Pin<Wrap<&'a Self, &'a Self>>, f: &'a u32) -> &'a u32 {
35+
| ++++ ++ ++ ++ ++
36+
37+
error[E0106]: missing lifetime specifier
38+
--> $DIR/multiple-ref-self-async.rs:37:93
39+
|
40+
LL | async fn box_box_wrap_ref_Self_ref_Self(self: Box<Box<Wrap<&Self, &Self>>>, f: &u32) -> &u32 {
41+
| ---------------------------- ---- ^ expected named lifetime parameter
42+
|
43+
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `self`'s 2 lifetimes or `f`
44+
help: consider introducing a named lifetime parameter
45+
|
46+
LL | async fn box_box_wrap_ref_Self_ref_Self<'a>(self: Box<Box<Wrap<&'a Self, &'a Self>>>, f: &'a u32) -> &'a u32 {
47+
| ++++ ++ ++ ++ ++
48+
49+
error[E0106]: missing lifetime specifier
50+
--> $DIR/multiple-ref-self-async.rs:42:93
51+
|
52+
LL | async fn box_pin_wrap_ref_Self_ref_Self(self: Box<Pin<Wrap<&Self, &Self>>>, f: &u32) -> &u32 {
53+
| ---------------------------- ---- ^ expected named lifetime parameter
54+
|
55+
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `self`'s 2 lifetimes or `f`
56+
help: consider introducing a named lifetime parameter
57+
|
58+
LL | async fn box_pin_wrap_ref_Self_ref_Self<'a>(self: Box<Pin<Wrap<&'a Self, &'a Self>>>, f: &'a u32) -> &'a u32 {
59+
| ++++ ++ ++ ++ ++
60+
61+
error: aborting due to 5 previous errors
62+
63+
For more information about this error, try `rustc --explain E0106`.

‎tests/ui/self/elision/multiple-ref-self.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
//@ check-pass
2-
31
#![feature(arbitrary_self_types)]
42
#![allow(non_snake_case)]
53

@@ -20,22 +18,27 @@ impl Struct {
2018
// Test using multiple `&Self`:
2119

2220
fn wrap_ref_Self_ref_Self(self: Wrap<&Self, &Self>, f: &u8) -> &u8 {
21+
//~^ ERROR missing lifetime specifier
2322
f
2423
}
2524

2625
fn box_wrap_ref_Self_ref_Self(self: Box<Wrap<&Self, &Self>>, f: &u32) -> &u32 {
26+
//~^ ERROR missing lifetime specifier
2727
f
2828
}
2929

3030
fn pin_wrap_ref_Self_ref_Self(self: Pin<Wrap<&Self, &Self>>, f: &u32) -> &u32 {
31+
//~^ ERROR missing lifetime specifier
3132
f
3233
}
3334

3435
fn box_box_wrap_ref_Self_ref_Self(self: Box<Box<Wrap<&Self, &Self>>>, f: &u32) -> &u32 {
36+
//~^ ERROR missing lifetime specifier
3537
f
3638
}
3739

3840
fn box_pin_wrap_ref_Self_ref_Self(self: Box<Pin<Wrap<&Self, &Self>>>, f: &u32) -> &u32 {
41+
//~^ ERROR missing lifetime specifier
3942
f
4043
}
4144
}

‎tests/ui/self/elision/multiple-ref-self.stderr

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
error[E0106]: missing lifetime specifier
2+
--> $DIR/multiple-ref-self.rs:20:68
3+
|
4+
LL | fn wrap_ref_Self_ref_Self(self: Wrap<&Self, &Self>, f: &u8) -> &u8 {
5+
| ------------------ --- ^ expected named lifetime parameter
6+
|
7+
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `self`'s 2 lifetimes or `f`
8+
help: consider introducing a named lifetime parameter
9+
|
10+
LL | fn wrap_ref_Self_ref_Self<'a>(self: Wrap<&'a Self, &'a Self>, f: &'a u8) -> &'a u8 {
11+
| ++++ ++ ++ ++ ++
12+
13+
error[E0106]: missing lifetime specifier
14+
--> $DIR/multiple-ref-self.rs:25:78
15+
|
16+
LL | fn box_wrap_ref_Self_ref_Self(self: Box<Wrap<&Self, &Self>>, f: &u32) -> &u32 {
17+
| ----------------------- ---- ^ expected named lifetime parameter
18+
|
19+
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `self`'s 2 lifetimes or `f`
20+
help: consider introducing a named lifetime parameter
21+
|
22+
LL | fn box_wrap_ref_Self_ref_Self<'a>(self: Box<Wrap<&'a Self, &'a Self>>, f: &'a u32) -> &'a u32 {
23+
| ++++ ++ ++ ++ ++
24+
25+
error[E0106]: missing lifetime specifier
26+
--> $DIR/multiple-ref-self.rs:30:78
27+
|
28+
LL | fn pin_wrap_ref_Self_ref_Self(self: Pin<Wrap<&Self, &Self>>, f: &u32) -> &u32 {
29+
| ----------------------- ---- ^ expected named lifetime parameter
30+
|
31+
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `self`'s 2 lifetimes or `f`
32+
help: consider introducing a named lifetime parameter
33+
|
34+
LL | fn pin_wrap_ref_Self_ref_Self<'a>(self: Pin<Wrap<&'a Self, &'a Self>>, f: &'a u32) -> &'a u32 {
35+
| ++++ ++ ++ ++ ++
36+
37+
error[E0106]: missing lifetime specifier
38+
--> $DIR/multiple-ref-self.rs:35:87
39+
|
40+
LL | fn box_box_wrap_ref_Self_ref_Self(self: Box<Box<Wrap<&Self, &Self>>>, f: &u32) -> &u32 {
41+
| ---------------------------- ---- ^ expected named lifetime parameter
42+
|
43+
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `self`'s 2 lifetimes or `f`
44+
help: consider introducing a named lifetime parameter
45+
|
46+
LL | fn box_box_wrap_ref_Self_ref_Self<'a>(self: Box<Box<Wrap<&'a Self, &'a Self>>>, f: &'a u32) -> &'a u32 {
47+
| ++++ ++ ++ ++ ++
48+
49+
error[E0106]: missing lifetime specifier
50+
--> $DIR/multiple-ref-self.rs:40:87
51+
|
52+
LL | fn box_pin_wrap_ref_Self_ref_Self(self: Box<Pin<Wrap<&Self, &Self>>>, f: &u32) -> &u32 {
53+
| ---------------------------- ---- ^ expected named lifetime parameter
54+
|
55+
= help: this function's return type contains a borrowed value, but the signature does not say whether it is borrowed from one of `self`'s 2 lifetimes or `f`
56+
help: consider introducing a named lifetime parameter
57+
|
58+
LL | fn box_pin_wrap_ref_Self_ref_Self<'a>(self: Box<Pin<Wrap<&'a Self, &'a Self>>>, f: &'a u32) -> &'a u32 {
59+
| ++++ ++ ++ ++ ++
60+
61+
error: aborting due to 5 previous errors
62+
63+
For more information about this error, try `rustc --explain E0106`.

‎tests/ui/self/elision/no-shadow-pin-self.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
use std::pin::Pin;
2+
trait Trait {
3+
fn method<'a>(self: Pin<&Self>, f: &'a u32) -> &'a u32 {
4+
f
5+
}
6+
}
7+
8+
impl<P> Trait for Pin<P> {
9+
// This should not hide `&Self`, which would cause this to compile.
10+
fn method(self: Pin<&Self>, f: &u32) -> &u32 {
11+
//~^ ERROR `impl` item signature doesn't match `trait`
12+
f
13+
//~^ ERROR lifetime may not live long enough
14+
}
15+
}
16+
17+
fn main() {}

0 commit comments

Comments
 (0)