Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Jul 28, 2019
95 changes: 62 additions & 33 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions src/test/ui/self/arbitrary_self_types_pin_lifetime.rs
Original file line number Diff line number Diff line change
@@ -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() };
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: lifetime may not live long enough
--> $DIR/arbitrary_self_types_pin_lifetime_impl_trait.rs:8:31
|
LL | fn f(self: Pin<&Self>) -> impl Clone { self }
| - ^^^^^^^^^^ opaque type requires that `'1` must outlive `'static`
| |
| let's call the lifetime of this reference `'1`
help: to allow this `impl Trait` to capture borrowed data with lifetime `'1`, add `'_` as a constraint
|
LL | fn f(self: Pin<&Self>) -> impl Clone + '_ { self }
| ^^^^^^^^^^^^^^^

error: aborting due to previous error

13 changes: 13 additions & 0 deletions src/test/ui/self/arbitrary_self_types_pin_lifetime_impl_trait.rs
Original file line number Diff line number Diff line change
@@ -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() };
}
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
error: lifetime may not live long enough
--> $DIR/arbitrary_self_types_pin_lifetime_mismatch.rs:8:46
|
LL | fn a(self: Pin<&Foo>, f: &Foo) -> &Foo { f }
| - - ^ function was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1`
| | |
| | let's call the lifetime of this reference `'1`
| let's call the lifetime of this reference `'2`

error: lifetime may not live long enough
--> $DIR/arbitrary_self_types_pin_lifetime_mismatch.rs:10:69
|
LL | fn c(self: Pin<&Self>, f: &Foo, g: &Foo) -> (Pin<&Foo>, &Foo) { (self, f) }
| - - ^^^^^^^^^ function was supposed to return data with lifetime `'2` but it is returning data with lifetime `'1`
| | |
| | let's call the lifetime of this reference `'1`
| let's call the lifetime of this reference `'2`

error: lifetime may not live long enough
--> $DIR/arbitrary_self_types_pin_lifetime_mismatch.rs:15:58
|
LL | fn bar<'a>(self: Alias<&Self>, arg: &'a ()) -> &() { arg }
| -- ---- has type `std::pin::Pin<&'1 Foo>` ^^^ function was supposed to return data with lifetime `'1` but it is returning data with lifetime `'a`
| |
| lifetime `'a` defined here

error: aborting due to 3 previous errors

18 changes: 18 additions & 0 deletions src/test/ui/self/arbitrary_self_types_pin_lifetime_mismatch.rs
Original file line number Diff line number Diff line change
@@ -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() {}
26 changes: 26 additions & 0 deletions src/test/ui/self/arbitrary_self_types_pin_lifetime_mismatch.stderr
Original file line number Diff line number Diff line change
@@ -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

44 changes: 44 additions & 0 deletions src/test/ui/self/elision/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
Test cases intended 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% |
Centril marked this conversation as resolved.
Show resolved Hide resolved
| `ref-mut-struct.rs` | `Struct` | `&mut Struct` | take lifetime from `&mut Self` | 50% |
| `ref-alias.rs` | `Struct` | `&Alias` | ignore `Alias` | 0% |
Centril marked this conversation as resolved.
Show resolved Hide resolved
| `ref-mut-alias.rs` | `Struct` | `&mut Alias` | ignore `Alias` | 0% |
| `lt-self.rs` | `Struct<'a>` | `Self` | ignore `Self` (and hence `'a`) | 25% |
Centril marked this conversation as resolved.
Show resolved Hide resolved
| `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`.
36 changes: 36 additions & 0 deletions src/test/ui/self/elision/alias.rs
Original file line number Diff line number Diff line change
@@ -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
}
}
taiki-e marked this conversation as resolved.
Show resolved Hide resolved

fn main() { }
Loading