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 using #[pin_project] type with private field types #53

Merged
merged 1 commit into from
Aug 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pin-project-internal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,6 @@ lazy_static = { version = "1.3", optional = true }

[dev-dependencies]
pin-project = { version = "0.4.0-alpha", path = ".." }

[build-dependencies]
rustc_version = "0.2.3"
15 changes: 15 additions & 0 deletions pin-project-internal/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Based on https://stackoverflow.com/a/49250753/1290530

use rustc_version::{version_meta, Channel};

fn main() {
// Set cfg flags depending on release channel
match version_meta().unwrap().channel {
// Enable our feature on nightly, or when using a
// locally build rustc
Channel::Nightly | Channel::Dev => {
println!("cargo:rustc-cfg=feature=\"RUSTC_IS_NIGHTLY\"");
}
_ => {}
}
}
1 change: 1 addition & 0 deletions pin-project-internal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#![warn(single_use_lifetimes)]
#![warn(clippy::all, clippy::pedantic)]
#![allow(clippy::use_self)]
#![cfg_attr(feature = "RUSTC_IS_NIGHTLY", feature(proc_macro_def_site))]

extern crate proc_macro;

Expand Down
4 changes: 2 additions & 2 deletions pin-project-internal/src/pin_project/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn named(
for Field { attrs, ident, ty, .. } in fields {
if let Some(attr) = attrs.find_remove(PIN) {
let _: Nothing = syn::parse2(attr.tokens)?;
cx.push_unpin_bounds(ty);
cx.push_unpin_bounds(ty.clone());
let lifetime = &cx.lifetime;
proj_body.push(quote!(#ident: ::core::pin::Pin::new_unchecked(#ident)));
proj_field.push(quote!(#ident: ::core::pin::Pin<&#lifetime mut #ty>));
Expand Down Expand Up @@ -108,7 +108,7 @@ fn unnamed(
let x = format_ident!("_x{}", i);
if let Some(attr) = attrs.find_remove(PIN) {
let _: Nothing = syn::parse2(attr.tokens)?;
cx.push_unpin_bounds(ty);
cx.push_unpin_bounds(ty.clone());
let lifetime = &cx.lifetime;
proj_body.push(quote!(::core::pin::Pin::new_unchecked(#x)));
proj_field.push(quote!(::core::pin::Pin<&#lifetime mut #ty>));
Expand Down
139 changes: 132 additions & 7 deletions pin-project-internal/src/pin_project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ struct Context {
/// Where-clause for conditional Unpin implementation.
impl_unpin: WhereClause,

pinned_fields: Vec<Type>,

unsafe_unpin: bool,
pinned_drop: Option<Span>,
}
Expand Down Expand Up @@ -97,6 +99,7 @@ impl Context {
generics,
lifetime,
impl_unpin,
pinned_fields: vec![],
unsafe_unpin: unsafe_unpin.is_some(),
pinned_drop,
})
Expand All @@ -109,21 +112,143 @@ impl Context {
generics
}

fn push_unpin_bounds(&mut self, ty: &Type) {
// We only add bounds for automatically generated impls
if !self.unsafe_unpin {
self.impl_unpin.predicates.push(syn::parse_quote!(#ty: ::core::marker::Unpin));
}
fn push_unpin_bounds(&mut self, ty: Type) {
self.pinned_fields.push(ty);
}

/// Makes conditional `Unpin` implementation for original type.
fn make_unpin_impl(&self) -> TokenStream {
let orig_ident = &self.orig_ident;
let (impl_generics, ty_generics, _) = self.generics.split_for_impl();
let type_params: Vec<_> = self.generics.type_params().map(|t| t.ident.clone()).collect();
let where_clause = &self.impl_unpin;

quote! {
impl #impl_generics ::core::marker::Unpin for #orig_ident #ty_generics #where_clause {}
if self.unsafe_unpin {
quote! {
impl #impl_generics ::core::marker::Unpin for #orig_ident #ty_generics #where_clause {}
}
} else {
let make_span = || {
#[cfg(feature = "RUSTC_IS_NIGHTLY")]
{
proc_macro::Span::def_site().into()
}

#[cfg(not(feature = "RUSTC_IS_NIGHTLY"))]
{
Span::call_site()
}
};

let struct_ident = Ident::new(&format!("__UnpinStruct{}", orig_ident), make_span());
let always_unpin_ident = Ident::new("AlwaysUnpin", make_span());

// Generate a field in our new struct for every
// pinned field in the original type
let fields: Vec<_> = self
.pinned_fields
.iter()
.enumerate()
.map(|(i, ty)| {
let field_ident = format_ident!("__field{}", i);
quote! {
#field_ident: #ty
}
})
.collect();

// We could try to determine the subset of type parameters
// and lifetimes that are actually used by the pinned fields
// (as opposed to those only used by unpinned fields).
// However, this would be tricky and error-prone, since
// it's possible for users to create types that would alias
// with generic parameters (e.g. 'struct T').
//
// Instead, we generate a use of every single type parameter
// and lifetime used in the original struct. For type parameters,
// we generate code like this:
//
// ```rust
// struct AlwaysUnpin<T: ?Sized>(PhantomData<T>) {}
// impl<T: ?Sized> Unpin for AlwaysUnpin<T> {}
//
// ...
// _field: AlwaysUnpin<(A, B, C)>
// ```
//
// This ensures that any unused type paramters
// don't end up with Unpin bounds
let lifetime_fields: Vec<_> = self
.generics
.lifetimes()
.enumerate()
.map(|(i, l)| {
let field_ident = format_ident!("__lifetime{}", i);
quote! {
#field_ident: &#l ()
}
})
.collect();

let scope_ident = format_ident!("__unpin_scope_{}", orig_ident);

let full_generics = &self.generics;
let mut full_where_clause = where_clause.clone();

let unpin_clause: WherePredicate = syn::parse_quote! {
#struct_ident #ty_generics: ::core::marker::Unpin
};

full_where_clause.predicates.push(unpin_clause);

let inner_data = quote! {

struct #always_unpin_ident <T: ?Sized> {
val: ::core::marker::PhantomData<T>
}

impl<T: ?Sized> ::core::marker::Unpin for #always_unpin_ident <T> {}

// This needs to be public, due to the limitations of the
// 'public in private' error.
//
// Out goal is to implement the public trait Unpin for
// a potentially public user type. Because of this, rust
// requires that any types mentioned in the where clause of
// our Unpin impl also be public. This means that our generated
// '__UnpinStruct' type must also be public. However, we take
// steps to ensure that the user can never actually reference
// this 'public' type. These steps are described below
pub struct #struct_ident #full_generics #where_clause {
taiki-e marked this conversation as resolved.
Show resolved Hide resolved
__pin_project_use_generics: #always_unpin_ident <(#(#type_params),*)>,

#(#fields,)*
#(#lifetime_fields,)*
}

impl #impl_generics ::core::marker::Unpin for #orig_ident #ty_generics #full_where_clause {}
};

if cfg!(feature = "RUSTC_IS_NIGHTLY") {
// On nightly, we use def-site hygiene to make it impossible
// for user code to refer to any of the types we define.
// This allows us to omit wrapping the generated types
// in an fn() scope, allowing rustdoc to properly document
// them.
inner_data
} else {
// When we're not on nightly, we need to create an enclosing fn() scope
// for all of our generated items. This makes it impossible for
// user code to refer to any of our generated types, but has
// the advantage of preventing Rustdoc from displaying
// docs for any of our types. In particular, users cannot see
// the automatically generated Unpin impl for the '__UnpinStruct$Name' types
quote! {
fn #scope_ident() {
inner_data
}
}
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions pin-project-internal/src/pin_project/structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ fn named(
for Field { attrs, ident, ty, .. } in fields {
if let Some(attr) = attrs.find_remove(PIN) {
let _: Nothing = syn::parse2(attr.tokens)?;
cx.push_unpin_bounds(ty);
cx.push_unpin_bounds(ty.clone());
let lifetime = &cx.lifetime;
proj_fields.push(quote!(#ident: ::core::pin::Pin<&#lifetime mut #ty>));
proj_init.push(quote!(#ident: ::core::pin::Pin::new_unchecked(&mut this.#ident)));
Expand All @@ -79,7 +79,7 @@ fn unnamed(
let i = Index::from(i);
if let Some(attr) = attrs.find_remove(PIN) {
let _: Nothing = syn::parse2(attr.tokens)?;
cx.push_unpin_bounds(ty);
cx.push_unpin_bounds(ty.clone());
let lifetime = &cx.lifetime;
proj_fields.push(quote!(::core::pin::Pin<&#lifetime mut #ty>));
proj_init.push(quote!(::core::pin::Pin::new_unchecked(&mut this.#i)));
Expand Down
6 changes: 6 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@
//! * [`pinned_drop`] - An attribute for annotating a function that implements `Drop`.
//! * [`project`] - An attribute to support pattern matching.
//!
//! NOTE: While this crate supports stable Rust, it currently requires
//! nightly Rust in order for rustdoc to correctly document auto-generated
//! `Unpin` impls. This does not affect the runtime functionality of this crate,
//! nor does it affect the safety of the api provided by this crate.
//!
//!
//! ## Examples
//!
//! [`pin_project`] attribute creates a projection struct covering all the fields.
Expand Down
24 changes: 24 additions & 0 deletions tests/pin_project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,3 +238,27 @@ fn combine() {
#[allow(unsafe_code)]
unsafe impl<T: Unpin> UnsafeUnpin for Foo<T> {}
}

#[test]
// This 'allow' is unrelated to the code
// generated by pin-project - it's just to
// allow us to put a private enum in a public enum
#[allow(private_in_public)]
fn test_private_type_in_public_type() {
#[pin_project]
pub struct PublicStruct<T> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is that this generates the following documents (due to rust-lang/rust#63281):

impl<T> Unpin for PublicStruct<T>
where
    __UnpinStructPublicStruct<T>: Unpin, 

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if UnsafeUnpin is used, a similar document is generated, but UnsafeUnpin impl is also documented at the same time, so we can know which type needs to be Unpin.

impl<T> Unpin for Foo<T>
where
    Wrapper<Self>: UnsafeUnpin,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be fine if __UnpinStructPublicStruct had documentation generated - that would allow users to use --document-private-items to seewhen it implemented Unpin. However, the fact that __UnpinStructPublicStruct is defined within a function seems to be preventing rustdoc from documenting it.

I'll investigate further.

#[pin]
inner: PrivateStruct<T>,
}

struct PrivateStruct<T>(T);

#[pin_project]
pub enum PublicEnum {
Variant(#[pin] PrivateEnum),
}

enum PrivateEnum {
OtherVariant(u8),
}
}
25 changes: 25 additions & 0 deletions tests/ui/pin_project/proper_unpin.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// compile-fail

#![deny(warnings, unsafe_code)]

use pin_project::{pin_project, pinned_drop};
use std::pin::Pin;

struct Inner<T> {
val: T
}

#[pin_project]
struct Foo<T, U> {
#[pin]
inner: Inner<T>,
other: U
}

fn is_unpin<T: Unpin>() {}

fn bar<T, U>() {
is_unpin::<Foo<T, U>>(); //~ ERROR E0277
}

fn main() {}
19 changes: 19 additions & 0 deletions tests/ui/pin_project/proper_unpin.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error[E0277]: the trait bound `T: std::marker::Unpin` is not satisfied in `__UnpinStructFoo<T, U>`
--> $DIR/proper_unpin.rs:22:5
|
22 | is_unpin::<Foo<T, U>>(); //~ ERROR E0277
| ^^^^^^^^^^^^^^^^^^^^^ within `__UnpinStructFoo<T, U>`, the trait `std::marker::Unpin` is not implemented for `T`
|
= help: consider adding a `where T: std::marker::Unpin` bound
= note: required because it appears within the type `Inner<T>`
= note: required because it appears within the type `__UnpinStructFoo<T, U>`
= note: required because of the requirements on the impl of `std::marker::Unpin` for `Foo<T, U>`
note: required by `is_unpin`
--> $DIR/proper_unpin.rs:19:1
|
19 | fn is_unpin<T: Unpin>() {}
| ^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
11 changes: 11 additions & 0 deletions tests/ui/pin_project/unpin_sneaky.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use pin_project::pin_project;

#[pin_project]
struct Foo {
#[pin]
inner: u8
}

impl Unpin for __UnpinStructFoo {}

fn main() {}
20 changes: 20 additions & 0 deletions tests/ui/pin_project/unpin_sneaky.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0412]: cannot find type `__UnpinStructFoo` in this scope
--> $DIR/unpin_sneaky.rs:9:16
|
9 | impl Unpin for __UnpinStructFoo {}
| ^^^^^^^^^^^^^^^^ not found in this scope
help: possible candidate is found in another module, you can import it into scope
|
1 | use crate::__UnpinStructFoo;
|

error[E0321]: cross-crate traits with a default impl, like `std::marker::Unpin`, can only be implemented for a struct/enum type, not `[type error]`
--> $DIR/unpin_sneaky.rs:9:1
|
9 | impl Unpin for __UnpinStructFoo {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't implement cross-crate trait with a default impl for non-struct/enum type

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0321, E0412.
For more information about an error, try `rustc --explain E0321`.