Fix drop order and ref mut handling in proc-macro

Author: oxalica

macros/Cargo.toml

@@ -20,3 +20,4 @@ syn = { version = "2.0", features = ["full"] }
 
 [dev-dependencies]
 async-ffi = { version = "0.4", path = ".." }
+futures-task = "0.3"

macros/src/lib.rs

@@ -234,6 +234,8 @@ fn expand(
             }
             FnArg::Typed(pat_ty) => pat_ty,
         };
+
+        let attributes = &pat_ty.attrs;
         let param_ident = match &*pat_ty.pat {
             Pat::Ident(pat_ident) => {
                 if pat_ident.ident == "self" {
@@ -244,6 +246,12 @@ fn expand(
             }
             _ => Ident::new(&format!("__param{}", i), pat_ty.span()),
         };
+
+        // If this is a declaration, only check but not transform.
+        if body.is_none() {
+            continue;
+        }
+
         let old_pat = mem::replace(
             &mut *pat_ty.pat,
             Pat::Ident(PatIdent {
@@ -254,10 +262,18 @@ fn expand(
                 subpat: None,
             }),
         );
-        let attributes = &pat_ty.attrs;
-        // NB. Re-bindings use external (macro) spans, so they won't trigger lints.
+
+        // NB.
+        // - Rebind the parameter once, to ensure their drop order not broken
+        //   by non-moving patterns containing `_`.
+        // - `mut` is required when the old pattern has `ref mut` inside.
+        // - Use external (macro) spans, so they won't trigger lints.
         param_bindings.extend(quote! {
             #(#attributes)*
+            #[allow(clippy::used_underscore_binding)]
+            let mut #param_ident = #param_ident;
+            #(#attributes)*
+            #[allow(clippy::used_underscore_binding)]
             let #old_pat = #param_ident;
         });
     }
@@ -306,4 +322,12 @@ mod tests {
     /// }
     /// ```
     fn typed_receiver() {}
+
+    /// ```compile_fail
+    /// extern "C" {
+    ///     #[async_ffi_macros::async_ffi]
+    ///     async fn foo(ref mut x: i32);
+    /// }
+    /// ```
+    fn declaration_pattern() {}
 }

macros/tests/test.rs

@@ -49,3 +49,56 @@ pub async fn pat_param(_: i32, (a, _, _b): (i32, i32, i32), x: i32, mut y: i32)
     y += 1;
     a + x + y
 }
+
+#[test]
+#[allow(clippy::toplevel_ref_arg, clippy::unused_async)]
+fn drop_order() {
+    use std::cell::RefCell;
+    use std::future::Future;
+    use std::pin::Pin;
+    use std::rc::Rc;
+    use std::task::{Context, Poll};
+
+    struct Dropper(&'static str, Rc<RefCell<Vec<&'static str>>>);
+
+    impl Drop for Dropper {
+        fn drop(&mut self) {
+            self.1.borrow_mut().push(self.0);
+        }
+    }
+
+    fn check_order<F>(
+        f: fn(Dropper, Dropper, Dropper, (Dropper, Dropper)) -> F,
+    ) -> Vec<&'static str>
+    where
+        F: Future<Output = ()>,
+    {
+        let order = Rc::new(RefCell::new(Vec::new()));
+        let mut fut = f(
+            Dropper("a", order.clone()),
+            Dropper("b", order.clone()),
+            Dropper("_arg", order.clone()),
+            (Dropper("c", order.clone()), Dropper("_pat", order.clone())),
+        );
+        Dropper("ret", order.clone());
+        let fut_pinned = unsafe { Pin::new_unchecked(&mut fut) };
+        let mut cx = Context::from_waker(futures_task::noop_waker_ref());
+        assert_eq!(fut_pinned.poll(&mut cx), Poll::Ready(()));
+        Dropper("done", order.clone());
+        drop(fut);
+        let ret = order.borrow().to_vec();
+        ret
+    }
+
+    #[async_ffi(?Send)]
+    async fn func1(a: Dropper, ref mut _b: Dropper, _: Dropper, (_c, _): (Dropper, Dropper)) {
+        a.1.borrow_mut().push("run");
+    }
+    async fn func2(a: Dropper, ref mut _b: Dropper, _: Dropper, (_c, _): (Dropper, Dropper)) {
+        a.1.borrow_mut().push("run");
+    }
+
+    let ord1 = check_order(func1);
+    let ord2 = check_order(func2);
+    assert_eq!(ord1, ord2);
+}