Skip to content

Commit

Permalink
Change #[pinned_drop] to trait implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
taiki-e committed Sep 10, 2019
1 parent 4cd166a commit a4837ec
Show file tree
Hide file tree
Showing 17 changed files with 437 additions and 155 deletions.
17 changes: 4 additions & 13 deletions examples/pinned_drop-expanded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,27 +72,18 @@ impl<'a, T> ::core::ops::Drop for Foo<'a, T> {
// Safety - we're in 'drop', so we know that 'self' will
// never move again.
let pinned_self = unsafe { ::core::pin::Pin::new_unchecked(self) };
// We call `pinned_drop` only once. Since `UnsafePinnedDrop::pinned_drop`
// We call `pinned_drop` only once. Since `UnsafePinnedDrop::drop`
// is an unsafe function and a private API, it is never called again in safe
// code *unless the user uses a maliciously crafted macro*.
unsafe {
::pin_project::__private::UnsafePinnedDrop::pinned_drop(pinned_self);
::pin_project::__private::UnsafePinnedDrop::drop(pinned_self);
}
}
}

unsafe impl<T> ::pin_project::__private::UnsafePinnedDrop for Foo<'_, T> {
unsafe fn pinned_drop(self: ::core::pin::Pin<&mut Self>) {
// Declare the #[pinned_drop] function *inside* our pinned_drop function
// This guarantees that it's impossible for any other user code
// to call it.
fn drop_foo<T>(mut foo: Pin<&mut Foo<'_, T>>) {
**foo.project().was_dropped = true;
}

// #[pinned_drop] function is a free function - if it were part of a trait impl,
// it would be possible for user code to call it by directly invoking the trait.
drop_foo(self)
unsafe fn drop(mut self: ::core::pin::Pin<&mut Self>) {
**self.project().was_dropped = true;
}
}

Expand Down
6 changes: 4 additions & 2 deletions examples/pinned_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ pub struct Foo<'a, T> {
}

#[pinned_drop]
fn drop_foo<T>(mut this: Pin<&mut Foo<'_, T>>) {
**this.project().was_dropped = true;
impl<T> PinnedDrop for Foo<'_, T> {
fn drop(mut self: Pin<&mut Self>) {
**self.project().was_dropped = true;
}
}

fn main() {}
29 changes: 16 additions & 13 deletions pin-project-internal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,10 @@ use syn::parse::Nothing;
/// takes `&mut Self`, not `Pin<&mut Self>`.
///
/// To ensure that this requirement is upheld, the `pin_project` attribute will
/// provide a `Drop` impl for you. This `Drop` impl will delegate to a function
/// provide a `Drop` impl for you. This `Drop` impl will delegate to an impl block
/// annotated with `#[pinned_drop]` if you use the `PinnedDrop` argument to
/// `#[pin_project]`. This function acts just like a normal [`drop`] impl, except
/// for the fact that it takes `Pin<&mut Self>`. In particular, it will never be
/// `#[pin_project]`. This impl block acts just like a normal [`drop`] impl, except
/// for the fact that `drop` takes `Pin<&mut Self>`. In particular, it will never be
/// called more than once, just like [`Drop::drop`].
///
/// For example:
Expand All @@ -217,10 +217,11 @@ use syn::parse::Nothing;
/// }
///
/// #[pinned_drop]
/// fn my_drop_fn<T: Debug, U: Debug>(mut foo: Pin<&mut Foo<T, U>>) {
/// let foo = foo.project();
/// println!("Dropping pinned field: {:?}", foo.pinned_field);
/// println!("Dropping unpin field: {:?}", foo.unpin_field);
/// impl<T: Debug, U: Debug> PinnedDrop for Foo<T, U> {
/// fn drop(self: Pin<&mut Self>) {
/// println!("Dropping pinned field: {:?}", self.pinned_field);
/// println!("Dropping unpin field: {:?}", self.unpin_field);
/// }
/// }
///
/// fn main() {
Expand Down Expand Up @@ -338,11 +339,11 @@ pub fn pin_project(args: TokenStream, input: TokenStream) -> TokenStream {
}

// TODO: Move this doc into pin-project crate when https://github.com/rust-lang/rust/pull/62855 merged.
/// An attribute for annotating a function that implements [`Drop`].
/// An attribute for annotating an impl block that implements [`Drop`].
///
/// This attribute is only needed when you wish to provide a [`Drop`]
/// impl for your type. The function annotated with `#[pinned_drop]` acts just
/// like a normal [`drop`](Drop::drop) impl, except for the fact that it takes
/// impl for your type. The impl block annotated with `#[pinned_drop]` acts just
/// like a normal [`Drop`] impl, except for the fact that `drop` method takes
/// `Pin<&mut Self>`. In particular, it will never be called more than once,
/// just like [`Drop::drop`].
///
Expand All @@ -358,8 +359,10 @@ pub fn pin_project(args: TokenStream, input: TokenStream) -> TokenStream {
/// }
///
/// #[pinned_drop]
/// fn my_drop(foo: Pin<&mut Foo>) {
/// println!("Dropping: {}", foo.field);
/// impl PinnedDrop for Foo {
/// fn drop(self: Pin<&mut Self>) {
/// println!("Dropping: {}", self.field);
/// }
/// }
///
/// fn main() {
Expand All @@ -374,7 +377,7 @@ pub fn pin_project(args: TokenStream, input: TokenStream) -> TokenStream {
pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
let _: Nothing = syn::parse_macro_input!(args);
let input = syn::parse_macro_input!(input);
pinned_drop::attribute(&input).into()
pinned_drop::attribute(input).into()
}

// TODO: Move this doc into pin-project crate when https://github.com/rust-lang/rust/pull/62855 merged.
Expand Down
4 changes: 2 additions & 2 deletions pin-project-internal/src/pin_project/attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl Context {
let crate_path = &self.crate_path;

let call = quote_spanned! { pinned_drop =>
::#crate_path::__private::UnsafePinnedDrop::pinned_drop(pinned_self)
::#crate_path::__private::UnsafePinnedDrop::drop(pinned_self)
};

quote! {
Expand All @@ -237,7 +237,7 @@ impl Context {
// Safety - we're in 'drop', so we know that 'self' will
// never move again.
let pinned_self = unsafe { ::core::pin::Pin::new_unchecked(self) };
// We call `pinned_drop` only once. Since `UnsafePinnedDrop::pinned_drop`
// We call `pinned_drop` only once. Since `UnsafePinnedDrop::drop`
// is an unsafe function and a private API, it is never called again in safe
// code *unless the user uses a maliciously crafted macro*.
unsafe {
Expand Down
141 changes: 104 additions & 37 deletions pin-project-internal/src/pinned_drop.rs
Original file line number Diff line number Diff line change
@@ -1,69 +1,136 @@
use proc_macro2::TokenStream;
use quote::quote;
use syn::{
FnArg, GenericArgument, ItemFn, PatType, PathArguments, Result, ReturnType, Type, TypePath,
TypeReference, TypeTuple,
};
use quote::{quote_spanned, ToTokens};
use syn::{parse::Nothing, spanned::Spanned, *};

use crate::utils::crate_path;

pub(crate) fn attribute(input: &ItemFn) -> TokenStream {
pub(crate) fn attribute(input: ItemImpl) -> TokenStream {
parse(input).unwrap_or_else(|e| e.to_compile_error())
}

fn parse_arg(arg: &FnArg) -> Result<&Type> {
if let FnArg::Typed(PatType { ty, .. }) = arg {
if let Type::Path(TypePath { qself: None, path }) = &**ty {
let ty = &path.segments[path.segments.len() - 1];
fn parse_method(method: &ImplItemMethod) -> Result<()> {
fn get_ty_path(ty: &Type) -> Option<&Path> {
if let Type::Path(TypePath { qself: None, path }) = ty { Some(path) } else { None }
}

const INVALID_ARGUMENT: &str = "method `drop` must take an argument `self: Pin<&mut Self>`";

if method.sig.ident != "drop" {
return Err(error!(
method.sig.ident,
"method `{}` is not a member of trait `PinnedDrop", method.sig.ident,
));
}

if let ReturnType::Type(_, ty) = &method.sig.output {
match &**ty {
Type::Tuple(TypeTuple { elems, .. }) if elems.is_empty() => {}
_ => return Err(error!(ty, "method `drop` must return the unit type")),
}
}

if method.sig.inputs.len() != 1 {
if method.sig.inputs.is_empty() {
return Err(syn::Error::new(method.sig.paren_token.span, INVALID_ARGUMENT));
} else {
return Err(error!(&method.sig.inputs, INVALID_ARGUMENT));
}
}

if let FnArg::Typed(PatType { pat, ty, .. }) = &method.sig.inputs[0] {
// !by_ref (mutability) ident !subpat: path
if let (Pat::Ident(PatIdent { by_ref: None, ident, subpat: None, .. }), Some(path)) =
(&**pat, get_ty_path(ty))
{
let ty = &path.segments.last().unwrap();
if let PathArguments::AngleBracketed(args) = &ty.arguments {
if args.args.len() == 1 && ty.ident == "Pin" {
// (mut) self: (path::)Pin<args>
if ident == "self" && args.args.len() == 1 && ty.ident == "Pin" {
// &mut <elem>
if let GenericArgument::Type(Type::Reference(TypeReference {
mutability: Some(_),
elem,
..
})) = &args.args[0]
{
return Ok(&**elem);
if get_ty_path(elem).map_or(false, |path| path.is_ident("Self")) {
if method.sig.unsafety.is_some() {
return Err(error!(
method.sig.unsafety,
"implementing the method `drop` is not unsafe"
));
}
return Ok(());
}
}
}
}
}
}

Err(error!(arg, "#[pinned_drop] function must take a argument `Pin<&mut Type>`"))
Err(error!(method.sig.inputs[0], INVALID_ARGUMENT))
}

fn parse(item: &ItemFn) -> Result<TokenStream> {
if let ReturnType::Type(_, ty) = &item.sig.output {
match &**ty {
Type::Tuple(TypeTuple { elems, .. }) if elems.is_empty() => {}
_ => return Err(error!(ty, "#[pinned_drop] function must return the unit type")),
fn parse(mut item: ItemImpl) -> Result<TokenStream> {
if let Some((_, path, _)) = &mut item.trait_ {
if path.is_ident("PinnedDrop") {
let crate_path = crate_path();

*path = syn::parse2(quote_spanned! { path.span() =>
::#crate_path::__private::UnsafePinnedDrop
})
.unwrap();
} else {
return Err(error!(
path,
"#[pinned_drop] may only be used on implementation for the `PinnedDrop` trait"
));
}
}
if item.sig.inputs.len() != 1 {
} else {
return Err(error!(
item.sig.inputs,
"#[pinned_drop] function must take exactly one argument"
item.self_ty,
"#[pinned_drop] may only be used on implementation for the `PinnedDrop` trait"
));
}

let crate_path = crate_path();
let type_ = parse_arg(&item.sig.inputs[0])?;
let fn_name = &item.sig.ident;
let (impl_generics, _, where_clause) = item.sig.generics.split_for_impl();
if item.unsafety.is_some() {
return Err(error!(item.unsafety, "implementing the trait `PinnedDrop` is not unsafe"));
}
item.unsafety = Some(token::Unsafe::default());

Ok(quote! {
unsafe impl #impl_generics ::#crate_path::__private::UnsafePinnedDrop for #type_ #where_clause {
unsafe fn pinned_drop(self: ::core::pin::Pin<&mut Self>) {
// Declare the #[pinned_drop] function *inside* our pinned_drop function
// This guarantees that it's impossible for any other user code
// to call it.
#item
// #[pinned_drop] function is a free function - if it were part of a trait impl,
// it would be possible for user code to call it by directly invoking the trait.
#fn_name(self)
if item.items.is_empty() {
return Err(error!(item, "not all trait items implemented, missing: `drop`"));
} else {
for (i, item) in item.items.iter().enumerate() {
match item {
ImplItem::Const(item) => {
return Err(error!(
item,
"const `{}` is not a member of trait `PinnedDrop`", item.ident
));
}
ImplItem::Type(item) => {
return Err(error!(
item,
"type `{}` is not a member of trait `PinnedDrop`", item.ident
));
}
ImplItem::Method(method) => {
parse_method(method)?;
if i != 0 {
return Err(error!(method, "duplicate definitions with name `drop`"));
}
}
_ => {
syn::parse2::<Nothing>(item.to_token_stream())?;
}
}
}
})
}

if let ImplItem::Method(method) = &mut item.items[0] {
method.sig.unsafety = Some(token::Unsafe::default());
}

Ok(item.into_token_stream())
}
2 changes: 1 addition & 1 deletion pin-project-internal/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use quote::format_ident;
use syn::{
punctuated::Punctuated,
token::{self, Comma},
Attribute, GenericParam, Generics, Ident, Lifetime, LifetimeDef,
*,
};

pub(crate) const DEFAULT_LIFETIME_NAME: &str = "'_pin";
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ pub mod __private {
// Since calling it twice on the same object would be UB,
// this method is unsafe.
#[doc(hidden)]
unsafe fn pinned_drop(self: Pin<&mut Self>);
unsafe fn drop(self: Pin<&mut Self>);
}

// This is an internal helper struct used by `pin-project-internal`.
Expand Down
4 changes: 3 additions & 1 deletion tests/pin_project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,9 @@ fn combine() {
}

#[pinned_drop]
fn do_drop<T>(_: Pin<&mut Foo<T>>) {}
impl<T> PinnedDrop for Foo<T> {
fn drop(self: Pin<&mut Self>) {}
}

#[allow(unsafe_code)]
unsafe impl<T: Unpin> UnsafeUnpin for Foo<T> {}
Expand Down
27 changes: 4 additions & 23 deletions tests/pinned_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,13 @@ fn safe_project() {
}

#[pinned_drop]
fn do_drop(mut foo: Pin<&mut Foo<'_>>) {
**foo.project().was_dropped = true;
impl PinnedDrop for Foo<'_> {
fn drop(mut self: Pin<&mut Self>) {
**self.project().was_dropped = true;
}
}

let mut was_dropped = false;
drop(Foo { was_dropped: &mut was_dropped, field: 42 });
assert!(was_dropped);
}

#[test]
fn overlapping_drop_fn_names() {
#[pin_project(PinnedDrop)]
pub struct Foo {
#[pin]
field: u8,
}

#[pinned_drop]
fn do_drop(_: Pin<&mut Foo>) {}

#[pin_project(PinnedDrop)]
pub struct Bar {
#[pin]
field: u8,
}

#[pinned_drop]
fn do_drop(_: Pin<&mut Bar>) {}
}
4 changes: 3 additions & 1 deletion tests/ui/pin_project/drop_conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ struct Bar<T, U> {
}

#[pinned_drop]
fn do_drop<T, U>(this: Pin<&mut Bar<T, U>>) {}
impl<T, U> PinnedDrop for Bar<T, U> {
fn drop(self: Pin<&mut Self>) {}
}

impl<T, U> Drop for Bar<T, U> {
fn drop(&mut self) {}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/pin_project/drop_conflict.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ error[E0119]: conflicting implementations of trait `std::ops::Drop` for type `Ba
17 | #[pin_project(PinnedDrop)] //~ ERROR E0119
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Bar<_, _>`
...
27 | impl<T, U> Drop for Bar<T, U> {
29 | impl<T, U> Drop for Bar<T, U> {
| ----------------------------- first implementation here

error: aborting due to 2 previous errors
Expand Down
4 changes: 3 additions & 1 deletion tests/ui/pin_project/packed_sneaky-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ struct D {
}

#[pinned_drop]
fn drop_d(_: Pin<&mut D>) {}
impl PinnedDrop for D {
fn drop(self: Pin<&mut Self>) {}
}

fn main() {}
Loading

0 comments on commit a4837ec

Please sign in to comment.