Rollup merge of rust-lang#61207 - taiki-e:arbitrary_self_types-lifetime-elision-2, r=Centril

Allow lifetime elision in `Pin<&(mut) Self>`

This PR changes `self: &(mut) S` elision rules to instead visit the type of `self` and look for `&(mut) S` (where `is_self_ty(S)`) within it

Replaces rust-lang#60944

Closes rust-lang#52675

r? @eddyb
cc @cramertj @Centril @withoutboats @scottmcm

Author: Centril

src/librustc/middle/resolve_lifetime.rs

@@ -2146,48 +2146,77 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
         // First (determined here), if `self` is by-reference, then the
         // implied output region is the region of the self parameter.
         if has_self {
-            // Look for `self: &'a Self` - also desugared from `&'a self`,
-            // and if that matches, use it for elision and return early.
-            let is_self_ty = |res: Res| {
-                if let Res::SelfTy(..) = res {
-                    return true;
-                }
-
-                // Can't always rely on literal (or implied) `Self` due
-                // to the way elision rules were originally specified.
-                let impl_self = impl_self.map(|ty| &ty.node);
-                if let Some(&hir::TyKind::Path(hir::QPath::Resolved(None, ref path))) = impl_self {
-                    match path.res {
-                        // Whitelist the types that unambiguously always
-                        // result in the same type constructor being used
-                        // (it can't differ between `Self` and `self`).
-                        Res::Def(DefKind::Struct, _)
-                        | Res::Def(DefKind::Union, _)
-                        | Res::Def(DefKind::Enum, _)
-                        | Res::PrimTy(_) => {
-                            return res == path.res
+            struct SelfVisitor<'a> {
+                map: &'a NamedRegionMap,
+                impl_self: Option<&'a hir::TyKind>,
+                lifetime: Set1<Region>,
+            }
+
+            impl SelfVisitor<'_> {
+                // Look for `self: &'a Self` - also desugared from `&'a self`,
+                // and if that matches, use it for elision and return early.
+                fn is_self_ty(&self, res: Res) -> bool {
+                    if let Res::SelfTy(..) = res {
+                        return true;
+                    }
+
+                    // Can't always rely on literal (or implied) `Self` due
+                    // to the way elision rules were originally specified.
+                    if let Some(&hir::TyKind::Path(hir::QPath::Resolved(None, ref path))) =
+                        self.impl_self
+                    {
+                        match path.res {
+                            // Whitelist the types that unambiguously always
+                            // result in the same type constructor being used
+                            // (it can't differ between `Self` and `self`).
+                            Res::Def(DefKind::Struct, _)
+                            | Res::Def(DefKind::Union, _)
+                            | Res::Def(DefKind::Enum, _)
+                            | Res::PrimTy(_) => {
+                                return res == path.res
+                            }
+                            _ => {}
                         }
-                        _ => {}
                     }
+
+                    false
                 }
+            }
 
-                false
-            };
+            impl<'a> Visitor<'a> for SelfVisitor<'a> {
+                fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'a> {
+                    NestedVisitorMap::None
+                }
 
-            if let hir::TyKind::Rptr(lifetime_ref, ref mt) = inputs[0].node {
-                if let hir::TyKind::Path(hir::QPath::Resolved(None, ref path)) = mt.ty.node {
-                    if is_self_ty(path.res) {
-                        if let Some(&lifetime) = self.map.defs.get(&lifetime_ref.hir_id) {
-                            let scope = Scope::Elision {
-                                elide: Elide::Exact(lifetime),
-                                s: self.scope,
-                            };
-                            self.with(scope, |_, this| this.visit_ty(output));
-                            return;
+                fn visit_ty(&mut self, ty: &'a hir::Ty) {
+                    if let hir::TyKind::Rptr(lifetime_ref, ref mt) = ty.node {
+                        if let hir::TyKind::Path(hir::QPath::Resolved(None, ref path)) = mt.ty.node
+                        {
+                            if self.is_self_ty(path.res) {
+                                if let Some(lifetime) = self.map.defs.get(&lifetime_ref.hir_id) {
+                                    self.lifetime.insert(*lifetime);
+                                }
+                            }
                         }
                     }
+                    intravisit::walk_ty(self, ty)
                 }
             }
+
+            let mut visitor = SelfVisitor {
+                map: self.map,
+                impl_self: impl_self.map(|ty| &ty.node),
+                lifetime: Set1::Empty,
+            };
+            visitor.visit_ty(&inputs[0]);
+            if let Set1::One(lifetime) = visitor.lifetime {
+                let scope = Scope::Elision {
+                    elide: Elide::Exact(lifetime),
+                    s: self.scope,
+                };
+                self.with(scope, |_, this| this.visit_ty(output));
+                return;
+            }
         }
 
         // Second, if there was exactly one lifetime (either a substitution or a

src/test/ui/self/arbitrary_self_types_pin_lifetime.rs

@@ -0,0 +1,60 @@
+// check-pass
+
+use std::pin::Pin;
+use std::task::{Context, Poll};
+
+struct Foo;
+
+impl Foo {
+    fn pin_ref(self: Pin<&Self>) -> Pin<&Self> { self }
+
+    fn pin_mut(self: Pin<&mut Self>) -> Pin<&mut Self> { self }
+
+    fn pin_pin_pin_ref(self: Pin<Pin<Pin<&Self>>>) -> Pin<Pin<Pin<&Self>>> { self }
+
+    fn pin_ref_impl_trait(self: Pin<&Self>) -> impl Clone + '_ { self }
+
+    fn b(self: Pin<&Foo>, f: &Foo) -> Pin<&Foo> { self }
+}
+
+type Alias<T> = Pin<T>;
+impl Foo {
+    fn bar<'a>(self: Alias<&Self>, arg: &'a ()) -> Alias<&Self> { self }
+}
+
+struct Bar<T: Unpin, U: Unpin> {
+    field1: T,
+    field2: U,
+}
+
+impl<T: Unpin, U: Unpin> Bar<T, U> {
+    fn fields(self: Pin<&mut Self>) -> (Pin<&mut T>, Pin<&mut U>) {
+        let this = self.get_mut();
+        (Pin::new(&mut this.field1), Pin::new(&mut this.field2))
+    }
+}
+
+trait AsyncBufRead {
+    fn poll_fill_buf(self: Pin<&mut Self>, cx: &mut Context<'_>)
+        -> Poll<std::io::Result<&[u8]>>;
+}
+
+struct Baz(Vec<u8>);
+
+impl AsyncBufRead for Baz {
+    fn poll_fill_buf(self: Pin<&mut Self>, cx: &mut Context<'_>)
+        -> Poll<std::io::Result<&[u8]>>
+    {
+        Poll::Ready(Ok(&self.get_mut().0))
+    }
+}
+
+fn main() {
+    let mut foo = Foo;
+    { Pin::new(&foo).pin_ref() };
+    { Pin::new(&mut foo).pin_mut() };
+    { Pin::new(Pin::new(Pin::new(&foo))).pin_pin_pin_ref() };
+    { Pin::new(&foo).pin_ref_impl_trait() };
+    let mut bar = Bar { field1: 0u8, field2: 1u8 };
+    { Pin::new(&mut bar).fields() };
+}

src/test/ui/self/arbitrary_self_types_pin_lifetime_impl_trait.rs

@@ -0,0 +1,13 @@
+// compile-fail
+
+use std::pin::Pin;
+
+struct Foo;
+
+impl Foo {
+    fn f(self: Pin<&Self>) -> impl Clone { self } //~ ERROR cannot infer an appropriate lifetime
+}
+
+fn main() {
+    { Pin::new(&Foo).f() };
+}

src/test/ui/self/arbitrary_self_types_pin_lifetime_impl_trait.stderr

@@ -0,0 +1,20 @@
+error: cannot infer an appropriate lifetime
+  --> $DIR/arbitrary_self_types_pin_lifetime_impl_trait.rs:8:44
+   |
+LL |     fn f(self: Pin<&Self>) -> impl Clone { self }
+   |                               ----------   ^^^^ ...but this borrow...
+   |                               |
+   |                               this return type evaluates to the `'static` lifetime...
+   |
+note: ...can't outlive the anonymous lifetime #1 defined on the method body at 8:5
+  --> $DIR/arbitrary_self_types_pin_lifetime_impl_trait.rs:8:5
+   |
+LL |     fn f(self: Pin<&Self>) -> impl Clone { self }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: you can add a constraint to the return type to make it last less than `'static` and match the anonymous lifetime #1 defined on the method body at 8:5
+   |
+LL |     fn f(self: Pin<&Self>) -> impl Clone + '_ { self }
+   |                               ^^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+

src/test/ui/self/arbitrary_self_types_pin_lifetime_mismatch.rs

@@ -0,0 +1,18 @@
+// compile-fail
+
+use std::pin::Pin;
+
+struct Foo;
+
+impl Foo {
+    fn a(self: Pin<&Foo>, f: &Foo) -> &Foo { f } //~ ERROR E0623
+
+    fn c(self: Pin<&Self>, f: &Foo, g: &Foo) -> (Pin<&Foo>, &Foo) { (self, f) } //~ ERROR E0623
+}
+
+type Alias<T> = Pin<T>;
+impl Foo {
+    fn bar<'a>(self: Alias<&Self>, arg: &'a ()) -> &() { arg } //~ ERROR E0623
+}
+
+fn main() {}

src/test/ui/self/arbitrary_self_types_pin_lifetime_mismatch.stderr

@@ -0,0 +1,26 @@
+error[E0623]: lifetime mismatch
+  --> $DIR/arbitrary_self_types_pin_lifetime_mismatch.rs:8:46
+   |
+LL |     fn a(self: Pin<&Foo>, f: &Foo) -> &Foo { f }
+   |                              ----     ----   ^ ...but data from `f` is returned here
+   |                              |
+   |                              this parameter and the return type are declared with different lifetimes...
+
+error[E0623]: lifetime mismatch
+  --> $DIR/arbitrary_self_types_pin_lifetime_mismatch.rs:10:76
+   |
+LL |     fn c(self: Pin<&Self>, f: &Foo, g: &Foo) -> (Pin<&Foo>, &Foo) { (self, f) }
+   |                               ----              -----------------          ^ ...but data from `f` is returned here
+   |                               |
+   |                               this parameter and the return type are declared with different lifetimes...
+
+error[E0623]: lifetime mismatch
+  --> $DIR/arbitrary_self_types_pin_lifetime_mismatch.rs:15:58
+   |
+LL |     fn bar<'a>(self: Alias<&Self>, arg: &'a ()) -> &() { arg }
+   |                                         ------     ---   ^^^ ...but data from `arg` is returned here
+   |                                         |
+   |                                         this parameter and the return type are declared with different lifetimes...
+
+error: aborting due to 3 previous errors
+

src/test/ui/self/elision/README.md

@@ -0,0 +1,44 @@
+Test cases intended to to document behavior and try to exhaustively
+explore the combinations.
+
+## Confidence
+
+These tests are not yet considered 100% normative, in that some
+aspects of the current behavior are not desirable. This is expressed
+in the "confidence" field in the following table. Values:
+
+| Confidence | Interpretation |
+| --- | --- |
+| 100% | this will remain recommended behavior |
+| 75% | unclear whether we will continue to accept this |
+| 50% | this will likely be deprecated but remain valid |
+| 25% | this could change in the future |
+| 0% | this is definitely bogus and will likely change in the future in *some* way |
+
+## Tests
+
+| Test file | `Self` type | Pattern | Current elision behavior | Confidence |
+| --- | --- | --- | --- | --- |
+| `self.rs` | `Struct` | `Self` | ignore `self` parameter | 100% |
+| `struct.rs` | `Struct` | `Struct` | ignore `self` parameter | 100% |
+| `alias.rs` | `Struct` | `Alias` | ignore `self` parameter | 100% |
+| `ref-self.rs` | `Struct` | `&Self` | take lifetime from `&Self` | 100% |
+| `ref-mut-self.rs` | `Struct` | `&mut Self` | take lifetime from `&mut Self` | 100% |
+| `ref-struct.rs` | `Struct` | `&Struct` | take lifetime from `&Self` | 50% |
+| `ref-mut-struct.rs` | `Struct` | `&mut Struct` | take lifetime from `&mut Self` | 50% |
+| `ref-alias.rs` | `Struct` | `&Alias` | ignore `Alias` | 0% |
+| `ref-mut-alias.rs` | `Struct` | `&mut Alias` | ignore `Alias` | 0% |
+| `lt-self.rs` | `Struct<'a>` | `Self` | ignore `Self` (and hence `'a`) | 25% |
+| `lt-struct.rs` | `Struct<'a>` | `Self` | ignore `Self` (and hence `'a`) | 0% |
+| `lt-alias.rs`   | `Alias<'a>` | `Self` | ignore `Self` (and hence `'a`) | 0% |
+| `lt-ref-self.rs` | `Struct<'a>` | `&Self` | take lifetime from `&Self` | 75% |
+
+In each case, we test the following patterns:
+
+- `self: XXX`
+- `self: Box<XXX>`
+- `self: Pin<XXX>`
+- `self: Box<Box<XXX>>`
+- `self: Box<Pin<XXX>>`
+
+In the non-reference cases, `Pin` causes errors so we substitute `Rc`.

src/test/ui/self/elision/alias.rs

@@ -0,0 +1,36 @@
+// check-pass
+
+#![feature(arbitrary_self_types)]
+#![allow(non_snake_case)]
+
+use std::rc::Rc;
+
+struct Struct { }
+
+type Alias = Struct;
+
+impl Struct {
+    // Test using an alias for `Struct`:
+
+    fn alias(self: Alias, f: &u32) -> &u32 {
+        f
+    }
+
+    fn box_Alias(self: Box<Alias>, f: &u32) -> &u32 {
+        f
+    }
+
+    fn rc_Alias(self: Rc<Alias>, f: &u32) -> &u32 {
+        f
+    }
+
+    fn box_box_Alias(self: Box<Box<Alias>>, f: &u32) -> &u32 {
+        f
+    }
+
+    fn box_rc_Alias(self: Box<Rc<Alias>>, f: &u32) -> &u32 {
+        f
+    }
+}
+
+fn main() { }

src/test/ui/self/elision/assoc.rs

@@ -0,0 +1,40 @@
+// check-pass
+
+#![feature(arbitrary_self_types)]
+#![allow(non_snake_case)]
+
+use std::rc::Rc;
+
+trait Trait {
+    type AssocType;
+}
+
+struct Struct { }
+
+impl Trait for Struct {
+    type AssocType = Self;
+}
+
+impl Struct {
+    fn assoc(self: <Struct as Trait>::AssocType, f: &u32) -> &u32 {
+        f
+    }
+
+    fn box_AssocType(self: Box<<Struct as Trait>::AssocType>, f: &u32) -> &u32 {
+        f
+    }
+
+    fn rc_AssocType(self: Rc<<Struct as Trait>::AssocType>, f: &u32) -> &u32 {
+        f
+    }
+
+    fn box_box_AssocType(self: Box<Box<<Struct as Trait>::AssocType>>, f: &u32) -> &u32 {
+        f
+    }
+
+    fn box_rc_AssocType(self: Box<Rc<<Struct as Trait>::AssocType>>, f: &u32) -> &u32 {
+        f
+    }
+}
+
+fn main() { }