From 8d1958f0d27f2a3393f2d6fde9560356c78b5f60 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Wed, 15 Nov 2023 19:46:42 +0000 Subject: [PATCH 1/7] Ambiguous Self lifetimes: don't elide. struct Concrete(u32); impl Concrete { fn m(self: &Box) -> &u32 { &self.0 } } resulted in a confusing error. impl Concrete { fn n(self: &Box<&Self>) -> &u32 { &self.0 } } resulted in no error or warning, despite apparent ambiguity over the elided lifetime. This commit changes two aspects of the behavior. Previously, when examining the self type, we considered lifetimes only if they were immediately adjacent to Self. We now consider lifetimes anywhere in the self type. Secondly, if more than one lifetime is discovered in the self type, we disregard it as a possible lifetime elision candidate. This is a compatibility break, and in fact has required some changes to tests which assumed the earlier behavior. Fixes https://github.com/rust-lang/rust/issues/117715 --- compiler/rustc_resolve/src/late.rs | 26 ++++--- tests/ui/self/elision/ref-assoc-async.rs | 6 +- tests/ui/self/elision/ref-assoc-async.stderr | 77 ++++++++++++++++++++ tests/ui/self/elision/ref-assoc.rs | 7 +- tests/ui/self/elision/ref-assoc.stderr | 77 ++++++++++++++++++++ tests/ui/self/elision/ref-self-multi.rs | 29 ++++++++ tests/ui/self/elision/ref-self.fixed | 7 ++ tests/ui/self/elision/ref-self.rs | 7 ++ tests/ui/self/elision/ref-self.stderr | 31 ++++++-- 9 files changed, 246 insertions(+), 21 deletions(-) create mode 100644 tests/ui/self/elision/ref-assoc-async.stderr create mode 100644 tests/ui/self/elision/ref-assoc.stderr create mode 100644 tests/ui/self/elision/ref-self-multi.rs diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 0f585aafdd5cf..7c971e1f1bb97 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -2132,13 +2132,15 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { // Handle `self` specially. if index == 0 && has_self { let self_lifetime = self.find_lifetime_for_self(ty); - if let Set1::One(lifetime) = self_lifetime { + elision_lifetime = match self_lifetime { // We found `self` elision. - elision_lifetime = Elision::Self_(lifetime); - } else { + Set1::One(lifetime) => Elision::Self_(lifetime), + // `self` itself had ambiguous lifetimes, e.g. + // &Box<&Self> + Set1::Many => Elision::None, // We do not have `self` elision: disregard the `Elision::Param` that we may // have found. - elision_lifetime = Elision::None; + Set1::Empty => Elision::None, } } debug!("(resolving function / closure) recorded parameter"); @@ -2162,6 +2164,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { r: &'r Resolver<'a, 'tcx>, impl_self: Option, lifetime: Set1, + self_found: bool, } impl SelfVisitor<'_, '_, '_> { @@ -2185,9 +2188,11 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { impl<'a> Visitor<'a> for SelfVisitor<'_, '_, '_> { fn visit_ty(&mut self, ty: &'a Ty) { trace!("SelfVisitor considering ty={:?}", ty); - if let TyKind::Ref(lt, ref mt) = ty.kind - && self.is_self_ty(&mt.ty) - { + if self.is_self_ty(ty) { + trace!("SelfVisitor found Self"); + self.self_found = true; + } + if let TyKind::Ref(lt, _) = ty.kind { let lt_id = if let Some(lt) = lt { lt.id } else { @@ -2228,10 +2233,11 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { Res::Def(DefKind::Struct | DefKind::Union | DefKind::Enum, _,) | Res::PrimTy(_) ) }); - let mut visitor = SelfVisitor { r: self.r, impl_self, lifetime: Set1::Empty }; + let mut visitor = + SelfVisitor { r: self.r, impl_self, lifetime: Set1::Empty, self_found: false }; visitor.visit_ty(ty); - trace!("SelfVisitor found={:?}", visitor.lifetime); - visitor.lifetime + trace!("SelfVisitor found={:?}, self_found={:?}", visitor.lifetime, visitor.self_found); + if visitor.self_found { visitor.lifetime } else { Set1::Empty } } /// Searches the current set of local scopes for labels. Returns the `NodeId` of the resolved diff --git a/tests/ui/self/elision/ref-assoc-async.rs b/tests/ui/self/elision/ref-assoc-async.rs index 2af4f13a41b34..25deb25253d9b 100644 --- a/tests/ui/self/elision/ref-assoc-async.rs +++ b/tests/ui/self/elision/ref-assoc-async.rs @@ -1,5 +1,4 @@ //@ edition:2018 -//@ check-pass #![allow(non_snake_case)] @@ -18,22 +17,27 @@ impl Trait for Struct { impl Struct { async fn ref_AssocType(self: &::AssocType, f: &u32) -> &u32 { f + //~^ ERROR lifetime may not live long enough } async fn box_ref_AssocType(self: Box<&::AssocType>, f: &u32) -> &u32 { f + //~^ ERROR lifetime may not live long enough } async fn pin_ref_AssocType(self: Pin<&::AssocType>, f: &u32) -> &u32 { f + //~^ ERROR lifetime may not live long enough } async fn box_box_ref_AssocType(self: Box::AssocType>>, f: &u32) -> &u32 { f + //~^ ERROR lifetime may not live long enough } async fn box_pin_ref_AssocType(self: Box::AssocType>>, f: &u32) -> &u32 { f + //~^ ERROR lifetime may not live long enough } } diff --git a/tests/ui/self/elision/ref-assoc-async.stderr b/tests/ui/self/elision/ref-assoc-async.stderr new file mode 100644 index 0000000000000..cf54a86b45f06 --- /dev/null +++ b/tests/ui/self/elision/ref-assoc-async.stderr @@ -0,0 +1,77 @@ +error: lifetime may not live long enough + --> $DIR/ref-assoc-async.rs:19:9 + | +LL | async fn ref_AssocType(self: &::AssocType, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | async fn ref_AssocType<'a>(self: &'a ::AssocType, f: &'a u32) -> &u32 { + | ++++ ++ ++ + +error: lifetime may not live long enough + --> $DIR/ref-assoc-async.rs:24:9 + | +LL | async fn box_ref_AssocType(self: Box<&::AssocType>, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | async fn box_ref_AssocType<'a>(self: Box<&'a ::AssocType>, f: &'a u32) -> &u32 { + | ++++ ++ ++ + +error: lifetime may not live long enough + --> $DIR/ref-assoc-async.rs:29:9 + | +LL | async fn pin_ref_AssocType(self: Pin<&::AssocType>, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | async fn pin_ref_AssocType<'a>(self: Pin<&'a ::AssocType>, f: &'a u32) -> &u32 { + | ++++ ++ ++ + +error: lifetime may not live long enough + --> $DIR/ref-assoc-async.rs:34:9 + | +LL | async fn box_box_ref_AssocType(self: Box::AssocType>>, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | async fn box_box_ref_AssocType<'a>(self: Box::AssocType>>, f: &'a u32) -> &u32 { + | ++++ ++ ++ + +error: lifetime may not live long enough + --> $DIR/ref-assoc-async.rs:39:9 + | +LL | async fn box_pin_ref_AssocType(self: Box::AssocType>>, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | async fn box_pin_ref_AssocType<'a>(self: Box::AssocType>>, f: &'a u32) -> &u32 { + | ++++ ++ ++ + +error: aborting due to 5 previous errors + diff --git a/tests/ui/self/elision/ref-assoc.rs b/tests/ui/self/elision/ref-assoc.rs index 8dc78d31d39e9..01d2556df62d5 100644 --- a/tests/ui/self/elision/ref-assoc.rs +++ b/tests/ui/self/elision/ref-assoc.rs @@ -1,5 +1,3 @@ -//@ check-pass - #![allow(non_snake_case)] use std::pin::Pin; @@ -17,22 +15,27 @@ impl Trait for Struct { impl Struct { fn ref_AssocType(self: &::AssocType, f: &u32) -> &u32 { f + //~^ ERROR lifetime may not live long enough } fn box_ref_AssocType(self: Box<&::AssocType>, f: &u32) -> &u32 { f + //~^ ERROR lifetime may not live long enough } fn pin_ref_AssocType(self: Pin<&::AssocType>, f: &u32) -> &u32 { f + //~^ ERROR lifetime may not live long enough } fn box_box_ref_AssocType(self: Box::AssocType>>, f: &u32) -> &u32 { f + //~^ ERROR lifetime may not live long enough } fn box_pin_ref_AssocType(self: Box::AssocType>>, f: &u32) -> &u32 { f + //~^ ERROR lifetime may not live long enough } } diff --git a/tests/ui/self/elision/ref-assoc.stderr b/tests/ui/self/elision/ref-assoc.stderr new file mode 100644 index 0000000000000..7c8a1de95aecd --- /dev/null +++ b/tests/ui/self/elision/ref-assoc.stderr @@ -0,0 +1,77 @@ +error: lifetime may not live long enough + --> $DIR/ref-assoc.rs:17:9 + | +LL | fn ref_AssocType(self: &::AssocType, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn ref_AssocType<'a>(self: &::AssocType, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ + +error: lifetime may not live long enough + --> $DIR/ref-assoc.rs:22:9 + | +LL | fn box_ref_AssocType(self: Box<&::AssocType>, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn box_ref_AssocType<'a>(self: Box<&::AssocType>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ + +error: lifetime may not live long enough + --> $DIR/ref-assoc.rs:27:9 + | +LL | fn pin_ref_AssocType(self: Pin<&::AssocType>, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn pin_ref_AssocType<'a>(self: Pin<&::AssocType>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ + +error: lifetime may not live long enough + --> $DIR/ref-assoc.rs:32:9 + | +LL | fn box_box_ref_AssocType(self: Box::AssocType>>, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn box_box_ref_AssocType<'a>(self: Box::AssocType>>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ + +error: lifetime may not live long enough + --> $DIR/ref-assoc.rs:37:9 + | +LL | fn box_pin_ref_AssocType(self: Box::AssocType>>, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn box_pin_ref_AssocType<'a>(self: Box::AssocType>>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ + +error: aborting due to 5 previous errors + diff --git a/tests/ui/self/elision/ref-self-multi.rs b/tests/ui/self/elision/ref-self-multi.rs new file mode 100644 index 0000000000000..438e08fb1af39 --- /dev/null +++ b/tests/ui/self/elision/ref-self-multi.rs @@ -0,0 +1,29 @@ +//@ run-pass + +#![feature(arbitrary_self_types)] +#![allow(non_snake_case)] +#![allow(unused)] + +use std::marker::PhantomData; +use std::ops::Deref; + +struct Struct { } + +struct Wrap(T, PhantomData

); + +impl Deref for Wrap { + type Target = T; + fn deref(&self) -> &T { &self.0 } +} + +impl Struct { + fn ref_box_ref_Self(self: &Box<&Self>, f: &u32) -> &u32 { + f + } + + fn ref_wrap_ref_Self(self: &Wrap<&Self, u32>, f: &u32) -> &u32 { + f + } +} + +fn main() { } diff --git a/tests/ui/self/elision/ref-self.fixed b/tests/ui/self/elision/ref-self.fixed index 8bf5a0bb22322..784ccb9efe2f6 100644 --- a/tests/ui/self/elision/ref-self.fixed +++ b/tests/ui/self/elision/ref-self.fixed @@ -1,4 +1,6 @@ //@ run-rustfix +//@ edition:2018 + #![feature(arbitrary_self_types)] #![allow(non_snake_case, dead_code)] @@ -56,6 +58,11 @@ impl Struct { f //~^ ERROR lifetime may not live long enough } + + fn ref_box_Self<'a>(self: &Box, f: &'a u32) -> &'a u32 { + f + //~^ ERROR lifetime may not live long enough + } } fn main() {} diff --git a/tests/ui/self/elision/ref-self.rs b/tests/ui/self/elision/ref-self.rs index 4b4b8aa5b511d..dbe441879cc5d 100644 --- a/tests/ui/self/elision/ref-self.rs +++ b/tests/ui/self/elision/ref-self.rs @@ -1,4 +1,6 @@ //@ run-rustfix +//@ edition:2018 + #![feature(arbitrary_self_types)] #![allow(non_snake_case, dead_code)] @@ -56,6 +58,11 @@ impl Struct { f //~^ ERROR lifetime may not live long enough } + + fn ref_box_Self(self: &Box, f: &u32) -> &u32 { + f + //~^ ERROR lifetime may not live long enough + } } fn main() {} diff --git a/tests/ui/self/elision/ref-self.stderr b/tests/ui/self/elision/ref-self.stderr index c4ec8c55a003f..64e7bfc1bb06c 100644 --- a/tests/ui/self/elision/ref-self.stderr +++ b/tests/ui/self/elision/ref-self.stderr @@ -1,5 +1,5 @@ error: lifetime may not live long enough - --> $DIR/ref-self.rs:24:9 + --> $DIR/ref-self.rs:26:9 | LL | fn ref_self(&self, f: &u32) -> &u32 { | - - let's call the lifetime of this reference `'1` @@ -14,7 +14,7 @@ LL | fn ref_self<'a>(&self, f: &'a u32) -> &'a u32 { | ++++ ++ ++ error: lifetime may not live long enough - --> $DIR/ref-self.rs:31:9 + --> $DIR/ref-self.rs:33:9 | LL | fn ref_Self(self: &Self, f: &u32) -> &u32 { | - - let's call the lifetime of this reference `'1` @@ -29,7 +29,7 @@ LL | fn ref_Self<'a>(self: &Self, f: &'a u32) -> &'a u32 { | ++++ ++ ++ error: lifetime may not live long enough - --> $DIR/ref-self.rs:36:9 + --> $DIR/ref-self.rs:38:9 | LL | fn box_ref_Self(self: Box<&Self>, f: &u32) -> &u32 { | - - let's call the lifetime of this reference `'1` @@ -44,7 +44,7 @@ LL | fn box_ref_Self<'a>(self: Box<&Self>, f: &'a u32) -> &'a u32 { | ++++ ++ ++ error: lifetime may not live long enough - --> $DIR/ref-self.rs:41:9 + --> $DIR/ref-self.rs:43:9 | LL | fn pin_ref_Self(self: Pin<&Self>, f: &u32) -> &u32 { | - - let's call the lifetime of this reference `'1` @@ -59,7 +59,7 @@ LL | fn pin_ref_Self<'a>(self: Pin<&Self>, f: &'a u32) -> &'a u32 { | ++++ ++ ++ error: lifetime may not live long enough - --> $DIR/ref-self.rs:46:9 + --> $DIR/ref-self.rs:48:9 | LL | fn box_box_ref_Self(self: Box>, f: &u32) -> &u32 { | - - let's call the lifetime of this reference `'1` @@ -74,7 +74,7 @@ LL | fn box_box_ref_Self<'a>(self: Box>, f: &'a u32) -> &'a u32 { | ++++ ++ ++ error: lifetime may not live long enough - --> $DIR/ref-self.rs:51:9 + --> $DIR/ref-self.rs:53:9 | LL | fn box_pin_ref_Self(self: Box>, f: &u32) -> &u32 { | - - let's call the lifetime of this reference `'1` @@ -89,7 +89,7 @@ LL | fn box_pin_ref_Self<'a>(self: Box>, f: &'a u32) -> &'a u32 { | ++++ ++ ++ error: lifetime may not live long enough - --> $DIR/ref-self.rs:56:9 + --> $DIR/ref-self.rs:58:9 | LL | fn wrap_ref_Self_Self(self: Wrap<&Self, Self>, f: &u8) -> &u8 { | - - let's call the lifetime of this reference `'1` @@ -103,5 +103,20 @@ help: consider introducing a named lifetime parameter and update trait if needed LL | fn wrap_ref_Self_Self<'a>(self: Wrap<&Self, Self>, f: &'a u8) -> &'a u8 { | ++++ ++ ++ -error: aborting due to 7 previous errors +error: lifetime may not live long enough + --> $DIR/ref-self.rs:63:9 + | +LL | fn ref_box_Self(self: &Box, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn ref_box_Self<'a>(self: &Box, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ + +error: aborting due to 8 previous errors From e62599f8569509e5ebbea407d81f77fd0a9ca01b Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Tue, 9 Jan 2024 17:32:32 +0000 Subject: [PATCH 2/7] Do not elide if there's ambiguity in self lifetime. This makes a small change as requested in code review, such that if there's ambiguity in the self lifetime, we avoid lifetime elision entirely instead of considering using lifetimes from any of the other parameters. For example, impl Something { fn method(self: &Box<&Self>, something_else: &u32) -> &u32 { ... } } in standard Rust would have assumed the return lifetime was that of &Self; with this PR prior to this commit would have chosen the lifetime of 'something_else', and after this commit would give an error message explaining that the lifetime is ambiguous. --- compiler/rustc_resolve/src/late.rs | 6 +- .../self/elision/multiple-ref-self-async.rs | 6 +- .../elision/multiple-ref-self-async.stderr | 63 +++++++++++++++++++ tests/ui/self/elision/multiple-ref-self.rs | 7 ++- .../ui/self/elision/multiple-ref-self.stderr | 63 +++++++++++++++++++ tests/ui/self/elision/ref-self-multi.rs | 4 +- tests/ui/self/elision/ref-self-multi.stderr | 27 ++++++++ 7 files changed, 169 insertions(+), 7 deletions(-) create mode 100644 tests/ui/self/elision/multiple-ref-self-async.stderr create mode 100644 tests/ui/self/elision/multiple-ref-self.stderr create mode 100644 tests/ui/self/elision/ref-self-multi.stderr diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 7c971e1f1bb97..eb6827c85ec9b 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -2136,8 +2136,10 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { // We found `self` elision. Set1::One(lifetime) => Elision::Self_(lifetime), // `self` itself had ambiguous lifetimes, e.g. - // &Box<&Self> - Set1::Many => Elision::None, + // &Box<&Self>. In this case we won't consider + // taking an alternative parameter lifetime; just avoid elision + // entirely. + Set1::Many => Elision::Err, // We do not have `self` elision: disregard the `Elision::Param` that we may // have found. Set1::Empty => Elision::None, diff --git a/tests/ui/self/elision/multiple-ref-self-async.rs b/tests/ui/self/elision/multiple-ref-self-async.rs index fb77f91396caa..f63b455901ee0 100644 --- a/tests/ui/self/elision/multiple-ref-self-async.rs +++ b/tests/ui/self/elision/multiple-ref-self-async.rs @@ -1,4 +1,3 @@ -//@ check-pass //@ edition:2018 #![feature(arbitrary_self_types)] @@ -21,22 +20,27 @@ impl Struct { // Test using multiple `&Self`: async fn wrap_ref_Self_ref_Self(self: Wrap<&Self, &Self>, f: &u8) -> &u8 { + //~^ ERROR missing lifetime specifier f } async fn box_wrap_ref_Self_ref_Self(self: Box>, f: &u32) -> &u32 { + //~^ ERROR missing lifetime specifier f } async fn pin_wrap_ref_Self_ref_Self(self: Pin>, f: &u32) -> &u32 { + //~^ ERROR missing lifetime specifier f } async fn box_box_wrap_ref_Self_ref_Self(self: Box>>, f: &u32) -> &u32 { + //~^ ERROR missing lifetime specifier f } async fn box_pin_wrap_ref_Self_ref_Self(self: Box>>, f: &u32) -> &u32 { + //~^ ERROR missing lifetime specifier f } } diff --git a/tests/ui/self/elision/multiple-ref-self-async.stderr b/tests/ui/self/elision/multiple-ref-self-async.stderr new file mode 100644 index 0000000000000..e2abc7c1e7852 --- /dev/null +++ b/tests/ui/self/elision/multiple-ref-self-async.stderr @@ -0,0 +1,63 @@ +error[E0106]: missing lifetime specifier + --> $DIR/multiple-ref-self-async.rs:22:74 + | +LL | async fn wrap_ref_Self_ref_Self(self: Wrap<&Self, &Self>, f: &u8) -> &u8 { + | ------------------ --- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | async fn wrap_ref_Self_ref_Self<'a>(self: Wrap<&'a Self, &'a Self>, f: &'a u8) -> &'a u8 { + | ++++ ++ ++ ++ ++ + +error[E0106]: missing lifetime specifier + --> $DIR/multiple-ref-self-async.rs:27:84 + | +LL | async fn box_wrap_ref_Self_ref_Self(self: Box>, f: &u32) -> &u32 { + | ----------------------- ---- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | async fn box_wrap_ref_Self_ref_Self<'a>(self: Box>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ ++ ++ + +error[E0106]: missing lifetime specifier + --> $DIR/multiple-ref-self-async.rs:32:84 + | +LL | async fn pin_wrap_ref_Self_ref_Self(self: Pin>, f: &u32) -> &u32 { + | ----------------------- ---- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | async fn pin_wrap_ref_Self_ref_Self<'a>(self: Pin>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ ++ ++ + +error[E0106]: missing lifetime specifier + --> $DIR/multiple-ref-self-async.rs:37:93 + | +LL | async fn box_box_wrap_ref_Self_ref_Self(self: Box>>, f: &u32) -> &u32 { + | ---------------------------- ---- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | async fn box_box_wrap_ref_Self_ref_Self<'a>(self: Box>>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ ++ ++ + +error[E0106]: missing lifetime specifier + --> $DIR/multiple-ref-self-async.rs:42:93 + | +LL | async fn box_pin_wrap_ref_Self_ref_Self(self: Box>>, f: &u32) -> &u32 { + | ---------------------------- ---- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | async fn box_pin_wrap_ref_Self_ref_Self<'a>(self: Box>>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ ++ ++ + +error: aborting due to 5 previous errors + +For more information about this error, try `rustc --explain E0106`. diff --git a/tests/ui/self/elision/multiple-ref-self.rs b/tests/ui/self/elision/multiple-ref-self.rs index 01d6bb47c045c..dd9b138051d0b 100644 --- a/tests/ui/self/elision/multiple-ref-self.rs +++ b/tests/ui/self/elision/multiple-ref-self.rs @@ -1,5 +1,3 @@ -//@ check-pass - #![feature(arbitrary_self_types)] #![allow(non_snake_case)] @@ -20,22 +18,27 @@ impl Struct { // Test using multiple `&Self`: fn wrap_ref_Self_ref_Self(self: Wrap<&Self, &Self>, f: &u8) -> &u8 { + //~^ ERROR missing lifetime specifier f } fn box_wrap_ref_Self_ref_Self(self: Box>, f: &u32) -> &u32 { + //~^ ERROR missing lifetime specifier f } fn pin_wrap_ref_Self_ref_Self(self: Pin>, f: &u32) -> &u32 { + //~^ ERROR missing lifetime specifier f } fn box_box_wrap_ref_Self_ref_Self(self: Box>>, f: &u32) -> &u32 { + //~^ ERROR missing lifetime specifier f } fn box_pin_wrap_ref_Self_ref_Self(self: Box>>, f: &u32) -> &u32 { + //~^ ERROR missing lifetime specifier f } } diff --git a/tests/ui/self/elision/multiple-ref-self.stderr b/tests/ui/self/elision/multiple-ref-self.stderr new file mode 100644 index 0000000000000..24d74d352e482 --- /dev/null +++ b/tests/ui/self/elision/multiple-ref-self.stderr @@ -0,0 +1,63 @@ +error[E0106]: missing lifetime specifier + --> $DIR/multiple-ref-self.rs:20:68 + | +LL | fn wrap_ref_Self_ref_Self(self: Wrap<&Self, &Self>, f: &u8) -> &u8 { + | ------------------ --- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | fn wrap_ref_Self_ref_Self<'a>(self: Wrap<&'a Self, &'a Self>, f: &'a u8) -> &'a u8 { + | ++++ ++ ++ ++ ++ + +error[E0106]: missing lifetime specifier + --> $DIR/multiple-ref-self.rs:25:78 + | +LL | fn box_wrap_ref_Self_ref_Self(self: Box>, f: &u32) -> &u32 { + | ----------------------- ---- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | fn box_wrap_ref_Self_ref_Self<'a>(self: Box>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ ++ ++ + +error[E0106]: missing lifetime specifier + --> $DIR/multiple-ref-self.rs:30:78 + | +LL | fn pin_wrap_ref_Self_ref_Self(self: Pin>, f: &u32) -> &u32 { + | ----------------------- ---- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | fn pin_wrap_ref_Self_ref_Self<'a>(self: Pin>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ ++ ++ + +error[E0106]: missing lifetime specifier + --> $DIR/multiple-ref-self.rs:35:87 + | +LL | fn box_box_wrap_ref_Self_ref_Self(self: Box>>, f: &u32) -> &u32 { + | ---------------------------- ---- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | fn box_box_wrap_ref_Self_ref_Self<'a>(self: Box>>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ ++ ++ + +error[E0106]: missing lifetime specifier + --> $DIR/multiple-ref-self.rs:40:87 + | +LL | fn box_pin_wrap_ref_Self_ref_Self(self: Box>>, f: &u32) -> &u32 { + | ---------------------------- ---- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | fn box_pin_wrap_ref_Self_ref_Self<'a>(self: Box>>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ ++ ++ + +error: aborting due to 5 previous errors + +For more information about this error, try `rustc --explain E0106`. diff --git a/tests/ui/self/elision/ref-self-multi.rs b/tests/ui/self/elision/ref-self-multi.rs index 438e08fb1af39..ed431a9c852dd 100644 --- a/tests/ui/self/elision/ref-self-multi.rs +++ b/tests/ui/self/elision/ref-self-multi.rs @@ -1,5 +1,3 @@ -//@ run-pass - #![feature(arbitrary_self_types)] #![allow(non_snake_case)] #![allow(unused)] @@ -18,10 +16,12 @@ impl Deref for Wrap { impl Struct { fn ref_box_ref_Self(self: &Box<&Self>, f: &u32) -> &u32 { + //~^ ERROR missing lifetime specifier f } fn ref_wrap_ref_Self(self: &Wrap<&Self, u32>, f: &u32) -> &u32 { + //~^ ERROR missing lifetime specifier f } } diff --git a/tests/ui/self/elision/ref-self-multi.stderr b/tests/ui/self/elision/ref-self-multi.stderr new file mode 100644 index 0000000000000..7e0451aa0d5c3 --- /dev/null +++ b/tests/ui/self/elision/ref-self-multi.stderr @@ -0,0 +1,27 @@ +error[E0106]: missing lifetime specifier + --> $DIR/ref-self-multi.rs:18:56 + | +LL | fn ref_box_ref_Self(self: &Box<&Self>, f: &u32) -> &u32 { + | ----------- ---- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | fn ref_box_ref_Self<'a>(self: &'a Box<&'a Self>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ ++ ++ + +error[E0106]: missing lifetime specifier + --> $DIR/ref-self-multi.rs:23:63 + | +LL | fn ref_wrap_ref_Self(self: &Wrap<&Self, u32>, f: &u32) -> &u32 { + | ----------------- ---- ^ expected named lifetime parameter + | + = 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` +help: consider introducing a named lifetime parameter + | +LL | fn ref_wrap_ref_Self<'a>(self: &'a Wrap<&'a Self, u32>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ ++ ++ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0106`. From 6287c94fa131208111a26886d1aca81183e9eba0 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Wed, 22 May 2024 15:58:00 +0000 Subject: [PATCH 3/7] Adjust crash bug to still reproduce. This test reproduces a rustc ICE. Unfortunately, the changes to lifetime elision mask the original ICE bug by making this function signature illegal. However, by simplifying the signature we can regain the original ICE. --- tests/crashes/122903-1.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/crashes/122903-1.rs b/tests/crashes/122903-1.rs index 9323c435851e6..0953962ed9ef2 100644 --- a/tests/crashes/122903-1.rs +++ b/tests/crashes/122903-1.rs @@ -1,7 +1,7 @@ //@ known-bug: #122903 impl Struct { - async fn box_box_ref_Struct( - self: Box>)>>, + fn box_box_ref_Struct( + self: impl FnMut(Box), ) -> &u32 { f } From 8c1777750bce6329a3ce1268c0f6f49b7842f7d2 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Fri, 31 May 2024 11:03:37 +0000 Subject: [PATCH 4/7] Remove duplicative test. I think that this test is supposed to be a treereduced version of 122903-1.rs, but is actually identical. --- tests/crashes/122903-2.rs | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 tests/crashes/122903-2.rs diff --git a/tests/crashes/122903-2.rs b/tests/crashes/122903-2.rs deleted file mode 100644 index 0d5d93014c187..0000000000000 --- a/tests/crashes/122903-2.rs +++ /dev/null @@ -1,9 +0,0 @@ -//@ known-bug: #122903 - -impl Struct { - async fn box_box_ref_Struct( - self: Box>)>> - ) -> &u32 { - f - } -} From c20a90f2b87bf935d7e62124cf0660839813f1a9 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Tue, 4 Jun 2024 15:06:25 +0000 Subject: [PATCH 5/7] Add additional tests. --- .../elision/ignore-non-reference-lifetimes.rs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 tests/ui/self/elision/ignore-non-reference-lifetimes.rs diff --git a/tests/ui/self/elision/ignore-non-reference-lifetimes.rs b/tests/ui/self/elision/ignore-non-reference-lifetimes.rs new file mode 100644 index 0000000000000..f7f61b8c81063 --- /dev/null +++ b/tests/ui/self/elision/ignore-non-reference-lifetimes.rs @@ -0,0 +1,22 @@ +//@ check-pass + +struct Foo<'a>(&'a str); + +impl<'b> Foo<'b> { + fn a<'a>(self: Self, a: &'a str) -> &str { + a + } + fn b<'a>(self: Foo<'b>, a: &'a str) -> &str { + a + } +} + +struct Foo2<'a>(&'a u32); +impl<'a> Foo2<'a> { + fn foo(self: &Self) -> &u32 { self.0 } // ok + fn bar(self: &Foo2<'a>) -> &u32 { self.0 } // ok (do not look into `Foo`) + fn baz2(self: Self, arg: &u32) -> &u32 { arg } // use lt from `arg` + fn baz3(self: Foo2<'a>, arg: &u32) -> &u32 { arg } // use lt from `arg` +} + +fn main() {} From 386838d69fc404da44ce11537a2cc640683e4742 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Wed, 5 Jun 2024 14:19:20 +0000 Subject: [PATCH 6/7] Additional test due to Pin<&Self> discovery --- tests/ui/self/elision/no-shadow-pin-self.rs | 17 ++++++++++ .../ui/self/elision/no-shadow-pin-self.stderr | 32 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 tests/ui/self/elision/no-shadow-pin-self.rs create mode 100644 tests/ui/self/elision/no-shadow-pin-self.stderr diff --git a/tests/ui/self/elision/no-shadow-pin-self.rs b/tests/ui/self/elision/no-shadow-pin-self.rs new file mode 100644 index 0000000000000..e390b869ca7b7 --- /dev/null +++ b/tests/ui/self/elision/no-shadow-pin-self.rs @@ -0,0 +1,17 @@ +use std::pin::Pin; +trait Trait { + fn method<'a>(self: Pin<&Self>, f: &'a u32) -> &'a u32 { + f + } +} + +impl

Trait for Pin

{ + // This should not hide `&Self`, which would cause this to compile. + fn method(self: Pin<&Self>, f: &u32) -> &u32 { + //~^ ERROR `impl` item signature doesn't match `trait` + f + //~^ ERROR lifetime may not live long enough + } +} + +fn main() {} diff --git a/tests/ui/self/elision/no-shadow-pin-self.stderr b/tests/ui/self/elision/no-shadow-pin-self.stderr new file mode 100644 index 0000000000000..23485e9de79b7 --- /dev/null +++ b/tests/ui/self/elision/no-shadow-pin-self.stderr @@ -0,0 +1,32 @@ +error: `impl` item signature doesn't match `trait` item signature + --> $DIR/no-shadow-pin-self.rs:10:5 + | +LL | fn method<'a>(self: Pin<&Self>, f: &'a u32) -> &'a u32 { + | ------------------------------------------------------ expected `fn(Pin<&'1 Pin

>, &'a u32) -> &'a u32` +... +LL | fn method(self: Pin<&Self>, f: &u32) -> &u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(Pin<&'1 Pin

>, &'2 u32) -> &'1 u32` + | + = note: expected signature `fn(Pin<&'1 Pin

>, &'a u32) -> &'a u32` + found signature `fn(Pin<&'1 Pin

>, &'2 u32) -> &'1 u32` + = help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` + = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output + +error: lifetime may not live long enough + --> $DIR/no-shadow-pin-self.rs:12:9 + | +LL | fn method(self: Pin<&Self>, f: &u32) -> &u32 { + | - - let's call the lifetime of this reference `'1` + | | + | let's call the lifetime of this reference `'2` +LL | +LL | f + | ^ method was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1` + | +help: consider introducing a named lifetime parameter and update trait if needed + | +LL | fn method<'a>(self: Pin<&Self>, f: &'a u32) -> &'a u32 { + | ++++ ++ ++ + +error: aborting due to 2 previous errors + From a22130e1e0ed619b53bfe15c50c9c7f7400c3a5a Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Mon, 10 Jun 2024 08:44:44 +0000 Subject: [PATCH 7/7] Elision: consider lifetimes from &T iff T has Self Change the algorithm which determines whether a self lifetime can be used for return type lifetime elision, such that we consider lifetimes attached to any reference in the self type, so long as Self can be found anywhere inside the type of that reference. --- compiler/rustc_resolve/src/late.rs | 64 +++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index eb6827c85ec9b..c3243f68e1b24 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -2162,16 +2162,55 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { /// List all the lifetimes that appear in the provided type. fn find_lifetime_for_self(&self, ty: &'ast Ty) -> Set1 { - struct SelfVisitor<'r, 'a, 'tcx> { + /// Visits a type to find all the &references, and determines the + /// set of lifetimes for all of those references where the referent + /// contains Self. + struct FindReferenceVisitor<'r, 'a, 'tcx> { r: &'r Resolver<'a, 'tcx>, impl_self: Option, lifetime: Set1, + } + + impl<'a> Visitor<'a> for FindReferenceVisitor<'_, '_, '_> { + fn visit_ty(&mut self, ty: &'a Ty) { + trace!("FindReferenceVisitor considering ty={:?}", ty); + if let TyKind::Ref(lt, _) = ty.kind { + // See if anything inside the &thing contains Self + let mut visitor = + SelfVisitor { r: self.r, impl_self: self.impl_self, self_found: false }; + visitor.visit_ty(ty); + trace!("FindReferenceVisitor: SelfVisitor self_found={:?}", visitor.self_found); + if visitor.self_found { + let lt_id = if let Some(lt) = lt { + lt.id + } else { + let res = self.r.lifetimes_res_map[&ty.id]; + let LifetimeRes::ElidedAnchor { start, .. } = res else { bug!() }; + start + }; + let lt_res = self.r.lifetimes_res_map[<_id]; + trace!("FindReferenceVisitor inserting res={:?}", lt_res); + self.lifetime.insert(lt_res); + } + } + visit::walk_ty(self, ty) + } + + // A type may have an expression as a const generic argument. + // We do not want to recurse into those. + fn visit_expr(&mut self, _: &'a Expr) {} + } + + /// Visitor which checks the referent of a &Thing to see if the + /// Thing contains Self + struct SelfVisitor<'r, 'a, 'tcx> { + r: &'r Resolver<'a, 'tcx>, + impl_self: Option, self_found: bool, } impl SelfVisitor<'_, '_, '_> { - // Look for `self: &'a Self` - also desugared from `&'a self`, - // and if that matches, use it for elision and return early. + // Look for `self: &'a Self` - also desugared from `&'a self` fn is_self_ty(&self, ty: &Ty) -> bool { match ty.kind { TyKind::ImplicitSelf => true, @@ -2194,18 +2233,6 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { trace!("SelfVisitor found Self"); self.self_found = true; } - if let TyKind::Ref(lt, _) = ty.kind { - let lt_id = if let Some(lt) = lt { - lt.id - } else { - let res = self.r.lifetimes_res_map[&ty.id]; - let LifetimeRes::ElidedAnchor { start, .. } = res else { bug!() }; - start - }; - let lt_res = self.r.lifetimes_res_map[<_id]; - trace!("SelfVisitor inserting res={:?}", lt_res); - self.lifetime.insert(lt_res); - } visit::walk_ty(self, ty) } @@ -2235,11 +2262,10 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { Res::Def(DefKind::Struct | DefKind::Union | DefKind::Enum, _,) | Res::PrimTy(_) ) }); - let mut visitor = - SelfVisitor { r: self.r, impl_self, lifetime: Set1::Empty, self_found: false }; + let mut visitor = FindReferenceVisitor { r: self.r, impl_self, lifetime: Set1::Empty }; visitor.visit_ty(ty); - trace!("SelfVisitor found={:?}, self_found={:?}", visitor.lifetime, visitor.self_found); - if visitor.self_found { visitor.lifetime } else { Set1::Empty } + trace!("FindReferenceVisitor found={:?}", visitor.lifetime); + visitor.lifetime } /// Searches the current set of local scopes for labels. Returns the `NodeId` of the resolved