From 15ce755ca861bdb7637303f48cd757ec79975160 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Wed, 15 Nov 2023 19:46:42 +0000 Subject: [PATCH] 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.rs | 5 ++ tests/ui/self/elision/ref-self.stderr | 17 ++++- 8 files changed, 230 insertions(+), 14 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 3be962dab90b9..d7008b05ce239 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -2090,13 +2090,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"); @@ -2120,6 +2122,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<'_, '_, '_> { @@ -2143,9 +2146,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 { @@ -2186,10 +2191,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 ad10d8ba4f4e2..de2c4f011340d 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 2f02cb5f3c8af..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..19bd544b96aca --- /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: &'a ::AssocType, f: &'a u32) -> &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<&'a ::AssocType>, f: &'a u32) -> &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<&'a ::AssocType>, f: &'a u32) -> &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) -> &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) -> &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..5f3eceeb01fdd --- /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.rs b/tests/ui/self/elision/ref-self.rs index dd07fe1b00be3..52f4f1586c26e 100644 --- a/tests/ui/self/elision/ref-self.rs +++ b/tests/ui/self/elision/ref-self.rs @@ -53,6 +53,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 35693257c9919..38f8b92d6ed1a 100644 --- a/tests/ui/self/elision/ref-self.stderr +++ b/tests/ui/self/elision/ref-self.stderr @@ -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<&'a Self, Self>, f: &'a u8) -> &u8 { | ++++ ++ ++ -error: aborting due to 7 previous errors +error: lifetime may not live long enough + --> $DIR/ref-self.rs:58: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: &'a Box, f: &'a u32) -> &u32 { + | ++++ ++ ++ + +error: aborting due to 8 previous errors