Change #[pinned_drop] to trait implementation

examples/pinned_drop-expanded.rs

@@ -72,27 +72,22 @@ 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);
         }
     }
 }
 
+// Users can implement `Drop` safely using `#[pinned_drop]`.
+// **Do not call or implement this trait directly.**
 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)
+    // Since calling it twice on the same object would be UB,
+    // this method is unsafe.
+    unsafe fn drop(mut self: ::core::pin::Pin<&mut Self>) {
+        **self.project().was_dropped = true;
     }
 }
 

examples/pinned_drop.rs

@@ -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() {}

pin-project-internal/src/lib.rs

@@ -190,18 +190,28 @@ use syn::parse::Nothing;
 /// times requires using [`.as_mut()`][`Pin::as_mut`] to avoid
 /// consuming the `Pin`.
 ///
+/// See also [`UnsafeUnpin`] trait.
+///
 /// ### `#[pinned_drop]`
 ///
 /// In order to correctly implement pin projections, a type's `Drop` impl must
 /// not move out of any stucturally pinned fields. Unfortunately, [`Drop::drop`]
 /// 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
-/// 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
-/// called more than once, just like [`Drop::drop`].
+/// To ensure that this requirement is upheld, the `#[pin_project]` attribute will
+/// 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 impl block acts just like a normal [`Drop`] impl,
+/// except for the following two:
+///
+/// * `drop` method takes `Pin<&mut Self>`
+/// * Name of the trait is `PinnedDrop`.
+///
+/// `#[pin_project]` implements the actual [`Drop`] trait via `PinnedDrop` you
+/// implemented. To drop a type that implements `PinnedDrop`, use the [`drop`]
+/// function just like dropping a type that directly implements [`Drop`].
+///
+/// In particular, it will never be called more than once, just like [`Drop::drop`].
 ///
 /// For example:
 ///
@@ -217,10 +227,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() {
@@ -338,11 +349,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`].
 ///
@@ -358,8 +369,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() {
@@ -374,7 +387,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.

pin-project-internal/src/pin_project/attribute.rs

@@ -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! {
@@ -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 {

pin-project-internal/src/pinned_drop.rs

@@ -1,69 +1,160 @@
 use proc_macro2::TokenStream;
-use quote::quote;
-use syn::{
-    FnArg, GenericArgument, ItemFn, PatType, PathArguments, Result, ReturnType, Type, TypePath,
-    TypeReference, TypeTuple,
-};
+use quote::{quote, quote_spanned, ToTokens};
+use syn::{parse::Nothing, spanned::Spanned, *};
 
 use crate::utils::crate_path;
 
-pub(crate) fn attribute(input: &ItemFn) -> TokenStream {
-    parse(input).unwrap_or_else(|e| e.to_compile_error())
+pub(crate) fn attribute(mut input: ItemImpl) -> TokenStream {
+    if let Err(e) = parse(&mut input) {
+        let crate_path = crate_path();
+        let self_ty = &input.self_ty;
+        let (impl_generics, _, where_clause) = input.generics.split_for_impl();
+
+        let mut tokens = e.to_compile_error();
+        // Generate a dummy `UnsafePinnedDrop` implementation.
+        // In many cases, `#[pinned_drop] impl` is declared after `#[pin_project]`.
+        // Therefore, if `pinned_drop` compile fails, you will also get an error
+        // about `UnsafePinnedDrop` not being implemented.
+        // This can be prevented to some extent by generating a dummy
+        // `UnsafePinnedDrop` implementation.
+        // We already know that we will get a compile error, so this won't
+        // accidentally compile successfully.
+        tokens.extend(quote! {
+            unsafe impl #impl_generics ::#crate_path::__private::UnsafePinnedDrop
+                for #self_ty #where_clause
+            {
+                unsafe fn drop(self: ::core::pin::Pin<&mut Self>) {}
+            }
+        });
+        tokens
+    } else {
+        input.into_token_stream()
+    }
 }
 
-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(item: &mut ItemImpl) -> Result<()> {
+    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();
-
-    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.unsafety.is_some() {
+        return Err(error!(item.unsafety, "implementing the trait `PinnedDrop` is not unsafe"));
+    }
+    item.unsafety = Some(token::Unsafe::default());
+
+    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`"));
+                    }
+                }
+                _ => {
+                    let _: Nothing = syn::parse2(item.to_token_stream())?;
+                }
             }
         }
-    })
+    }
+
+    if let ImplItem::Method(method) = &mut item.items[0] {
+        method.sig.unsafety = Some(token::Unsafe::default());
+    }
+
+    Ok(())
 }

pin-project-internal/src/utils.rs

@@ -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";

src/lib.rs

@@ -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`.

tests/pin_project.rs

@@ -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> {}