Skip to content

Commit

Permalink
Add OriginalType<A, B>: 'pin bound to projected type
Browse files Browse the repository at this point in the history
Fixes #185

Usually, Rust will automatically add lifetime bounds of the form
`T: 'lifetime` when required (e.g. when a struct contains `&'a T`).

However, the compiler will not normalize associated types before
performing this analysis, which can require users to manually add
`T: 'lifetime` bounds themselves. When this happens with a generated
projection struct, the user has no ability to add such a bound, since
they cannot name the 'pin lifetime.

This PR adds a bound of the form `OriginalType<A, B>: 'pin` to the
`where` clause of the generated projection types. This ensures that
any type parameters which need to outlive 'pin will do so, without
unnecessarily constraining other type parameters.

Implementing this requires making two small breaking changes:

1. Writing `#[project] use path::MyType` will now import both the
   original type and the projected type. not just the projected type.
   This ensures that the original type can be resolved if it is
   referenced in a generated `where` clause predicate.

2. Anonymous lifetimes ('_) are no longer allowed in #[project]
   impl blocks. That is, the following code no longer compiler:
   `#[project] impl MyType<'_> {}`.
   This allows us to directly substitute the user-provided lifetimes
   into the generated `where` clause predicate, since anonymous
   lifetimes cannot be used in a `where` clause.

   This restriction could be lifted by re-writing all anonymous
   lifetimes to unique named lifetimes (e.g. '_a0, '_a1, ..), but
   I chose to leave this out of this PR. If this restriction proves
   cumbersome for users, it could be lifted in a separate PR.
  • Loading branch information
Aaron1011 committed Apr 11, 2020
1 parent 389ce9d commit 7fb8b7f
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 27 deletions.
33 changes: 21 additions & 12 deletions pin-project-internal/src/pin_project/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,9 @@ struct ProjectedType {
lifetime: Lifetime,
/// Generics of the projected type.
generics: Generics,
/// `where` clause of the projected type. This has an additional
/// bound generated by `insert_lifetime_and_bound`
where_clause: WhereClause,
}

struct Context {
Expand Down Expand Up @@ -230,7 +233,16 @@ impl Context {
let lifetime = Lifetime::new(&lifetime_name, Span::call_site());

let mut proj_generics = generics.clone();
insert_lifetime(&mut proj_generics, lifetime.clone());
let ty_generics = generics.split_for_impl().1;
let ty_generics_as_generics = syn::parse_quote!(#ty_generics);
let pred = insert_lifetime_and_bound(
&mut proj_generics,
lifetime.clone(),
&ty_generics_as_generics,
ident.clone(),
);
let mut where_clause = generics.clone().make_where_clause().clone();
where_clause.predicates.push(pred);

Ok(Self {
proj: ProjectedType {
Expand All @@ -239,6 +251,7 @@ impl Context {
ref_ident: proj_ident(&ident, Immutable),
lifetime,
generics: proj_generics,
where_clause,
},
orig: OriginalType { attrs, vis, ident, generics },
pinned_drop,
Expand All @@ -250,7 +263,7 @@ impl Context {
fn parse_struct(&mut self, fields: &Fields) -> Result<TokenStream> {
let (proj_pat, proj_init, proj_fields, proj_ref_fields) = match fields {
Fields::Named(fields) => self.visit_named(fields)?,
Fields::Unnamed(fields) => self.visit_unnamed(fields, true)?,
Fields::Unnamed(fields) => self.visit_unnamed(fields)?,
Fields::Unit => unreachable!(),
};

Expand All @@ -259,7 +272,7 @@ impl Context {
let proj_ref_ident = &self.proj.ref_ident;
let vis = &self.proj.vis;
let proj_generics = &self.proj.generics;
let where_clause = self.orig.generics.split_for_impl().2;
let where_clause = &self.proj.where_clause;

// For tuple structs, we need to generate `(T1, T2) where Foo: Bar`
// For non-tuple structs, we need to generate `where Foo: Bar { field1: T }`
Expand All @@ -268,7 +281,7 @@ impl Context {
(quote!(#where_clause #proj_fields), quote!(#where_clause #proj_ref_fields))
}
Fields::Unnamed(_) => {
(quote!(#proj_fields #where_clause), quote!(#proj_ref_fields #where_clause))
(quote!(#proj_fields #where_clause;), quote!(#proj_ref_fields #where_clause;))
}
Fields::Unit => unreachable!(),
};
Expand Down Expand Up @@ -303,7 +316,7 @@ impl Context {
let proj_ref_ident = &self.proj.ref_ident;
let vis = &self.proj.vis;
let proj_generics = &self.proj.generics;
let where_clause = self.orig.generics.split_for_impl().2;
let where_clause = &self.proj.where_clause;

let mut proj_items = quote! {
#[allow(clippy::mut_mut)] // This lint warns `&mut &mut <ty>`.
Expand Down Expand Up @@ -344,7 +357,7 @@ impl Context {
for Variant { ident, fields, .. } in variants {
let (proj_pat, proj_body, proj_fields, proj_ref_fields) = match fields {
Fields::Named(fields) => self.visit_named(fields)?,
Fields::Unnamed(fields) => self.visit_unnamed(fields, false)?,
Fields::Unnamed(fields) => self.visit_unnamed(fields)?,
Fields::Unit => {
(TokenStream::new(), TokenStream::new(), TokenStream::new(), TokenStream::new())
}
Expand Down Expand Up @@ -422,7 +435,6 @@ impl Context {
fn visit_unnamed(
&mut self,
FieldsUnnamed { unnamed: fields, .. }: &FieldsUnnamed,
is_struct: bool,
) -> Result<(TokenStream, TokenStream, TokenStream, TokenStream)> {
let mut proj_pat = Vec::with_capacity(fields.len());
let mut proj_body = Vec::with_capacity(fields.len());
Expand Down Expand Up @@ -460,11 +472,8 @@ impl Context {

let proj_pat = quote!((#(#proj_pat),*));
let proj_body = quote!((#(#proj_body),*));
let (proj_fields, proj_ref_fields) = if is_struct {
(quote!((#(#proj_fields),*);), quote!((#(#proj_ref_fields),*);))
} else {
(quote!((#(#proj_fields),*)), quote!((#(#proj_ref_fields),*)))
};
let (proj_fields, proj_ref_fields) =
(quote!((#(#proj_fields),*)), quote!((#(#proj_ref_fields),*)));

Ok((proj_pat, proj_body, proj_fields, proj_ref_fields))
}
Expand Down
35 changes: 29 additions & 6 deletions pin-project-internal/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn parse(mut stmt: Stmt, mutability: Mutability) -> Result<TokenStream> {
replace_stmt(&mut stmt, mutability)?;
match &mut stmt {
Stmt::Item(Item::Fn(item)) => replace_item_fn(item, mutability)?,
Stmt::Item(Item::Impl(item)) => replace_item_impl(item, mutability),
Stmt::Item(Item::Impl(item)) => replace_item_impl(item, mutability)?,
Stmt::Item(Item::Use(item)) => replace_item_use(item, mutability)?,
_ => {}
}
Expand Down Expand Up @@ -155,12 +155,13 @@ fn is_replaceable(pat: &Pat, allow_pat_path: bool) -> bool {
}
}

fn replace_item_impl(item: &mut ItemImpl, mutability: Mutability) {
fn replace_item_impl(item: &mut ItemImpl, mutability: Mutability) -> Result<()> {
let PathSegment { ident, arguments } = match &mut *item.self_ty {
Type::Path(TypePath { qself: None, path }) => path.segments.last_mut().unwrap(),
_ => return,
_ => return Ok(()),
};

let orig_ident = ident.clone();
replace_ident(ident, mutability);

let mut lifetime_name = String::from(DEFAULT_LIFETIME_NAME);
Expand All @@ -171,17 +172,31 @@ fn replace_item_impl(item: &mut ItemImpl, mutability: Mutability) {
.for_each(|item| determine_lifetime_name(&mut lifetime_name, &item.sig.generics.params));
let lifetime = Lifetime::new(&lifetime_name, Span::call_site());

insert_lifetime(&mut item.generics, lifetime.clone());
let generics: Generics = syn::parse_quote!(#arguments);

match arguments {
PathArguments::None => {
*arguments = PathArguments::AngleBracketed(syn::parse_quote!(<#lifetime>));
}
PathArguments::AngleBracketed(args) => {
for arg in &args.args {
if let GenericArgument::Lifetime(life) = arg {
if life.ident == "_" {
return Err(error!(
life,
"#[project] attribute may not be used with '_ lifetimes"
));
}
}
}
args.args.insert(0, syn::parse_quote!(#lifetime));
}
PathArguments::Parenthesized(_) => unreachable!(),
}

let pred = insert_lifetime_and_bound(&mut item.generics, lifetime, &generics, orig_ident);
item.generics.make_where_clause().predicates.push(pred);
Ok(())
}

fn replace_item_fn(item: &mut ItemFn, mutability: Mutability) -> Result<()> {
Expand Down Expand Up @@ -267,8 +282,16 @@ impl VisitMut for UseTreeVisitor {
}

match node {
// Desugar `use tree::<name>` into `tree::__<name>Projection`.
UseTree::Name(name) => replace_ident(&mut name.ident, self.mutability),
// Desugar `use tree::<name>` into `{tree::<name>, tree::__<name>Projection}`.
// We need to import both the original and projected name so that
// `insert_lifetime_and_bound` can generate a bound that referes
// to the original type
UseTree::Name(name) => {
let old_ident = &name.ident;
let mut new_ident = name.ident.clone();
replace_ident(&mut new_ident, self.mutability);
*node = syn::parse_quote!({#old_ident, #new_ident});
}
UseTree::Glob(glob) => {
self.res =
Err(error!(glob, "#[project] attribute may not be used on glob imports"));
Expand Down
20 changes: 18 additions & 2 deletions pin-project-internal/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ pub(crate) fn determine_lifetime_name(
}

/// Inserts a `lifetime` at position `0` of `generics.params`.
pub(crate) fn insert_lifetime(generics: &mut Generics, lifetime: Lifetime) {
pub(crate) fn insert_lifetime_and_bound(
generics: &mut Generics,
lifetime: Lifetime,
orig_generics: &Generics,
orig_ident: Ident,
) -> WherePredicate {
if generics.lt_token.is_none() {
generics.lt_token = Some(token::Lt::default())
}
Expand All @@ -74,11 +79,22 @@ pub(crate) fn insert_lifetime(generics: &mut Generics, lifetime: Lifetime) {
0,
GenericParam::Lifetime(LifetimeDef {
attrs: Vec::new(),
lifetime,
lifetime: lifetime.clone(),
colon_token: None,
bounds: Punctuated::new(),
}),
);

let orig_type: syn::Type = syn::parse_quote!(#orig_ident #orig_generics);
let mut punct = Punctuated::new();
punct.push(TypeParamBound::Lifetime(lifetime));

WherePredicate::Type(PredicateType {
lifetimes: None,
bounded_ty: orig_type,
colon_token: syn::token::Colon::default(),
bounds: punct,
})
}

/// Determines the visibility of the projected type and projection method.
Expand Down
19 changes: 19 additions & 0 deletions tests/no_infer_outlives.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use pin_project::{pin_project, project};

trait Bar<X> {
type Y;
}

struct Example<A>(A);

impl<X, T> Bar<X> for Example<T> {
type Y = Option<T>;
}

#[pin_project]
struct Foo<A, B> {
_x: <Example<A> as Bar<B>>::Y,
}

#[project]
impl<A, B> Foo<A, B> {}
4 changes: 0 additions & 4 deletions tests/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,6 @@ fn project_impl() {
field2: U,
}

#[project]
impl<T, U> HasLifetimes<'_, T, U> {}

#[pin_project]
struct HasOverlappingLifetimes<'pin, T, U> {
#[pin]
Expand Down Expand Up @@ -167,7 +164,6 @@ struct A {
}

mod project_use_1 {
use crate::A;
use core::pin::Pin;
use pin_project::project;

Expand Down
3 changes: 0 additions & 3 deletions tests/project_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,6 @@ fn project_impl() {
field2: U,
}

#[project_ref]
impl<T, U> HasLifetimes<'_, T, U> {}

#[pin_project]
struct HasOverlappingLifetimes<'pin, T, U> {
#[pin]
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/pin_project/anon_lifetime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use pin_project::{pin_project, project};

#[pin_project]
struct Foo<'a>(&'a u8);

#[project]
impl Foo<'_> {} //~ ERROR #[project] attribute may not

fn main() {}
5 changes: 5 additions & 0 deletions tests/ui/pin_project/anon_lifetime.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
error: #[project] attribute may not be used with '_ lifetimes
--> $DIR/anon_lifetime.rs:7:10
|
7 | impl Foo<'_> {} //~ ERROR #[project] attribute may not
| ^^

0 comments on commit 7fb8b7f

Please sign in to comment.