From dfd7b919f215513693badb58367f72647548b033 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 25 Aug 2019 16:46:02 -0400 Subject: [PATCH] Allow using #[pin_project] type with private field types Previously, given code such as: ```rust struct Private; pub struct Public { #[pin] private: Private } ``` we would generate an Unpin impl like this: ```rust impl Unpin for Public where Private: Unpin {} ``` Unfortunately, since Private is not a public type, this would cause an E0446 ('private type `Private` in public interface) When RFC 2145 is implemented (https://github.com/rust-lang/rust/issues/48054), this will become a lint, rather then a hard error. In the time being, we need a solution that will work with the current type privacy rules. The solution is to generate code like this: ```rust fn __private_scope() { pub struct __UnpinPublic { __field0: Private } impl Unpin for Public where __UnpinPublic: Unpin {} } ``` That is, we generate a new struct, containing all of the pinned fields from our #[pin_project] type. This struct is delcared within a function, which makes it impossible to be named by user code. This guarnatees that it will use the default auto-trait impl for Unpin - that is, it will implement Unpin iff all of its fields implement Unpin. This type can be safely declared as 'public', satisfiying the privacy checker without actually allowing user code to access it. This allows users to apply the #[pin_project] attribute to types regardless of the privacy of the types of their fields. --- pin-project-internal/Cargo.toml | 3 + pin-project-internal/build.rs | 15 ++ pin-project-internal/src/lib.rs | 1 + pin-project-internal/src/pin_project/enums.rs | 4 +- pin-project-internal/src/pin_project/mod.rs | 139 +++++++++++++++++- .../src/pin_project/structs.rs | 4 +- src/lib.rs | 6 + tests/pin_project.rs | 24 +++ tests/ui/pin_project/proper_unpin.rs | 25 ++++ tests/ui/pin_project/proper_unpin.stderr | 19 +++ tests/ui/pin_project/unpin_sneaky.rs | 11 ++ tests/ui/pin_project/unpin_sneaky.stderr | 20 +++ 12 files changed, 260 insertions(+), 11 deletions(-) create mode 100644 pin-project-internal/build.rs create mode 100644 tests/ui/pin_project/proper_unpin.rs create mode 100644 tests/ui/pin_project/proper_unpin.stderr create mode 100644 tests/ui/pin_project/unpin_sneaky.rs create mode 100644 tests/ui/pin_project/unpin_sneaky.stderr diff --git a/pin-project-internal/Cargo.toml b/pin-project-internal/Cargo.toml index 0f06d6b9..36f32898 100644 --- a/pin-project-internal/Cargo.toml +++ b/pin-project-internal/Cargo.toml @@ -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" diff --git a/pin-project-internal/build.rs b/pin-project-internal/build.rs new file mode 100644 index 00000000..0ed72856 --- /dev/null +++ b/pin-project-internal/build.rs @@ -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\""); + } + _ => {} + } +} diff --git a/pin-project-internal/src/lib.rs b/pin-project-internal/src/lib.rs index a40d230a..5c514807 100644 --- a/pin-project-internal/src/lib.rs +++ b/pin-project-internal/src/lib.rs @@ -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; diff --git a/pin-project-internal/src/pin_project/enums.rs b/pin-project-internal/src/pin_project/enums.rs index 03f753af..648e2be6 100644 --- a/pin-project-internal/src/pin_project/enums.rs +++ b/pin-project-internal/src/pin_project/enums.rs @@ -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>)); @@ -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>)); diff --git a/pin-project-internal/src/pin_project/mod.rs b/pin-project-internal/src/pin_project/mod.rs index 5d35a8ea..61fab439 100644 --- a/pin-project-internal/src/pin_project/mod.rs +++ b/pin-project-internal/src/pin_project/mod.rs @@ -64,6 +64,8 @@ struct Context { /// Where-clause for conditional Unpin implementation. impl_unpin: WhereClause, + pinned_fields: Vec, + unsafe_unpin: bool, pinned_drop: Option, } @@ -97,6 +99,7 @@ impl Context { generics, lifetime, impl_unpin, + pinned_fields: vec![], unsafe_unpin: unsafe_unpin.is_some(), pinned_drop, }) @@ -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(PhantomData) {} + // impl Unpin for AlwaysUnpin {} + // + // ... + // _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 { + val: ::core::marker::PhantomData + } + + impl ::core::marker::Unpin for #always_unpin_ident {} + + // 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 { + __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 + } + } + } } } diff --git a/pin-project-internal/src/pin_project/structs.rs b/pin-project-internal/src/pin_project/structs.rs index b60a3dd8..af805d30 100644 --- a/pin-project-internal/src/pin_project/structs.rs +++ b/pin-project-internal/src/pin_project/structs.rs @@ -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))); @@ -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))); diff --git a/src/lib.rs b/src/lib.rs index dd415a5d..fba95916 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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. diff --git a/tests/pin_project.rs b/tests/pin_project.rs index 716a80de..b20afa13 100644 --- a/tests/pin_project.rs +++ b/tests/pin_project.rs @@ -238,3 +238,27 @@ fn combine() { #[allow(unsafe_code)] unsafe impl UnsafeUnpin for Foo {} } + +#[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 { + #[pin] + inner: PrivateStruct, + } + + struct PrivateStruct(T); + + #[pin_project] + pub enum PublicEnum { + Variant(#[pin] PrivateEnum), + } + + enum PrivateEnum { + OtherVariant(u8), + } +} diff --git a/tests/ui/pin_project/proper_unpin.rs b/tests/ui/pin_project/proper_unpin.rs new file mode 100644 index 00000000..1eac4780 --- /dev/null +++ b/tests/ui/pin_project/proper_unpin.rs @@ -0,0 +1,25 @@ +// compile-fail + +#![deny(warnings, unsafe_code)] + +use pin_project::{pin_project, pinned_drop}; +use std::pin::Pin; + +struct Inner { + val: T +} + +#[pin_project] +struct Foo { + #[pin] + inner: Inner, + other: U +} + +fn is_unpin() {} + +fn bar() { + is_unpin::>(); //~ ERROR E0277 +} + +fn main() {} diff --git a/tests/ui/pin_project/proper_unpin.stderr b/tests/ui/pin_project/proper_unpin.stderr new file mode 100644 index 00000000..086ea674 --- /dev/null +++ b/tests/ui/pin_project/proper_unpin.stderr @@ -0,0 +1,19 @@ +error[E0277]: the trait bound `T: std::marker::Unpin` is not satisfied in `__UnpinStructFoo` + --> $DIR/proper_unpin.rs:22:5 + | +22 | is_unpin::>(); //~ ERROR E0277 + | ^^^^^^^^^^^^^^^^^^^^^ within `__UnpinStructFoo`, 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` + = note: required because it appears within the type `__UnpinStructFoo` + = note: required because of the requirements on the impl of `std::marker::Unpin` for `Foo` +note: required by `is_unpin` + --> $DIR/proper_unpin.rs:19:1 + | +19 | fn is_unpin() {} + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/pin_project/unpin_sneaky.rs b/tests/ui/pin_project/unpin_sneaky.rs new file mode 100644 index 00000000..732e6fb3 --- /dev/null +++ b/tests/ui/pin_project/unpin_sneaky.rs @@ -0,0 +1,11 @@ +use pin_project::pin_project; + +#[pin_project] +struct Foo { + #[pin] + inner: u8 +} + +impl Unpin for __UnpinStructFoo {} + +fn main() {} diff --git a/tests/ui/pin_project/unpin_sneaky.stderr b/tests/ui/pin_project/unpin_sneaky.stderr new file mode 100644 index 00000000..3507ada8 --- /dev/null +++ b/tests/ui/pin_project/unpin_sneaky.stderr @@ -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`.