Change syntax of method family attribute

Also put it after `#[method_id(...)]` in generated output, and add a few
tests.

Author: madsmtm

crates/header-translator/src/method.rs

@@ -767,6 +767,18 @@ impl fmt::Display for Method {
             writeln!(f, "        #[optional]")?;
         }
 
+        let method_kind = if self.memory_management == MemoryManagement::Normal {
+            "method"
+        } else {
+            "method_id"
+        };
+        let error_trailing = if self.is_error { "_" } else { "" };
+        writeln!(
+            f,
+            "        #[{}({}{})]",
+            method_kind, self.selector, error_trailing
+        )?;
+
         let id_mm_name = match &self.memory_management {
             // MemoryManagement::IdAlloc => Some("alloc"), // Unsupported
             MemoryManagement::IdCopy => Some("copy"),
@@ -779,21 +791,9 @@ impl fmt::Display for Method {
         if let Some(id_mm_name) = id_mm_name {
             // TODO: Be explicit about when we emit this for better
             // compile-times, and when we do it for soundness.
-            writeln!(f, "        #[unsafe(method_family({id_mm_name}))]")?;
+            writeln!(f, "        #[unsafe(method_family = {id_mm_name})]")?;
         };
 
-        let method_kind = if self.memory_management == MemoryManagement::Normal {
-            "method"
-        } else {
-            "method_id"
-        };
-        let error_trailing = if self.is_error { "_" } else { "" };
-        writeln!(
-            f,
-            "        #[{}({}{})]",
-            method_kind, self.selector, error_trailing
-        )?;
-
         //
         // Signature
         //

crates/objc2/CHANGELOG.md

@@ -34,6 +34,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * Added 16-fold `Encode` and `RefEncode` impls for function pointers
   (previously only implemented for up to 12 arguments, which turned out to be
   insufficient).
+* Added `#[unsafe(method_family = ...)]` attribute in `extern_methods!` and
+  `extern_protocol!`, to allow overriding the inferred method family.
 
 ### Changed
 * **BREAKING**: Renamed `declare_class!` to `define_class!`, and changed the

crates/objc2/src/__macro_helpers/method_family.rs

@@ -34,8 +34,8 @@ pub type MutableCopy = MethodFamily<5>;
 /// No family.
 pub type Other = MethodFamily<6>;
 
-/// Helper module where `#[method_family($family:ident)]` will import its
-/// value from.
+/// Helper module where `#[unsafe(method_family = $family:ident)]` will import
+/// its value from.
 #[allow(non_camel_case_types)]
 pub mod method_family_import {
     // Rename to match Clang's `__attribute__((objc_method_family(family)))`.

crates/objc2/src/__macro_helpers/msg_send_retained.rs

@@ -595,7 +595,7 @@ mod tests {
 
     use crate::rc::{autoreleasepool, RcTestObject, ThreadTestData};
     use crate::runtime::{NSObject, NSZone};
-    use crate::{class, msg_send_id, AllocAnyThread};
+    use crate::{class, extern_methods, msg_send_id, AllocAnyThread};
 
     mod test_trait_disambugated {
         use super::*;
@@ -1136,4 +1136,98 @@ mod tests {
         let retained = retained.unwrap();
         assert_eq!(&*retained, cls);
     }
+
+    extern_methods!(
+        unsafe impl RcTestObject {
+            #[method_id(copy)]
+            #[unsafe(method_family = new)]
+            fn copy_new(&self) -> Retained<Self>;
+
+            #[method_id(copy)]
+            #[unsafe(method_family = init)]
+            fn copy_init(this: Allocated<Self>) -> Retained<Self>;
+
+            #[method_id(copy)]
+            #[unsafe(method_family = copy)]
+            fn copy_copy(&self) -> Retained<Self>;
+
+            #[method_id(copy)]
+            #[unsafe(method_family = mutableCopy)]
+            fn copy_mutable_copy(&self) -> Retained<Self>;
+
+            #[method_id(copy)]
+            #[unsafe(method_family = none)]
+            fn copy_none(&self) -> Retained<Self>;
+        }
+    );
+
+    #[test]
+    fn test_method_family() {
+        // Test a few combinations of (incorrect) method families.
+        let obj = RcTestObject::new();
+        let mut expected = ThreadTestData::current();
+
+        let copy = obj.copy_new();
+        expected.copy += 1;
+        expected.alloc += 1;
+        expected.init += 1;
+        expected.assert_current();
+        drop(copy);
+        expected.release += 1;
+        expected.drop += 1;
+        expected.assert_current();
+
+        let alloc = RcTestObject::alloc();
+        let ptr = Allocated::as_ptr(&alloc);
+        expected.alloc += 1;
+        expected.assert_current();
+        let copy = RcTestObject::copy_init(alloc);
+        expected.copy += 1;
+        expected.alloc += 1;
+        expected.init += 1;
+        expected.assert_current();
+        drop(copy);
+        expected.release += 1;
+        expected.drop += 1;
+        expected.assert_current();
+        drop(unsafe { Allocated::new(ptr.cast_mut()) });
+        expected.release += 1;
+        expected.assert_current();
+
+        let copy = obj.copy_copy();
+        expected.copy += 1;
+        expected.alloc += 1;
+        expected.init += 1;
+        expected.assert_current();
+        drop(copy);
+        expected.release += 1;
+        expected.drop += 1;
+        expected.assert_current();
+
+        let copy = obj.copy_mutable_copy();
+        expected.copy += 1;
+        expected.alloc += 1;
+        expected.init += 1;
+        expected.assert_current();
+        drop(copy);
+        expected.release += 1;
+        expected.drop += 1;
+        expected.assert_current();
+
+        let copy = obj.copy_none();
+        expected.copy += 1;
+        expected.alloc += 1;
+        expected.init += 1;
+        expected.retain += 1;
+        expected.assert_current();
+        // SAFETY: Wrong method family specified, so we have +1 retain count
+        // in excess.
+        drop(unsafe { Retained::from_raw(Retained::as_ptr(&copy).cast_mut()) });
+        expected.release += 1;
+        expected.assert_current();
+        drop(copy);
+        expected.release += 1;
+        expected.drop += 1;
+        expected.assert_current();
+    }
 }

crates/objc2/src/macros/__attribute_helpers.rs

@@ -234,7 +234,7 @@ macro_rules! __extract_method_attributes_inner {
     // `method_family` attribute
     {
         (
-            #[unsafe(method_family($($parsed:tt)+))]
+            #[unsafe(method_family = $($parsed:tt)+)]
             $($rest:tt)*
         )
         ($($method:tt)*)

crates/objc2/src/macros/define_class.rs

@@ -144,8 +144,8 @@
 ///
 /// The desired selector can be specified using the `#[method(my:selector:)]`
 /// or `#[method_id(my:selector:)]` attribute, similar to the
-/// [`extern_methods!`] macro. The `#[method_family(family)]` attribute is not
-/// yet supported.
+/// [`extern_methods!`] macro. The `#[unsafe(method_family = family)]`
+/// attribute is not yet supported.
 ///
 /// If the `#[method_id(...)]` attribute is used, the return type must be
 /// `Option<Retained<T>>` or `Retained<T>`. Additionally, if the selector is
@@ -1448,7 +1448,7 @@ macro_rules! __define_class_no_method_family {
     () => {};
     ($($t:tt)+) => {
         $crate::__macro_helpers::compile_error!(
-            "`#[method_family(...)]` is not yet supported in `define_class!`"
+            "`#[method_family = ...]` is not yet supported in `define_class!`"
         )
     };
 }

crates/objc2/src/macros/extern_methods.rs

@@ -54,7 +54,7 @@
 /// for details.
 ///
 ///
-/// ### `#[unsafe(method_family(...))]`
+/// ### `#[unsafe(method_family = ...)]` (optional)
 ///
 /// The Cocoa memory management convention is figured out automatically based
 /// on the name of the selector, but it can be overwritten with this `unsafe`
@@ -413,7 +413,7 @@ macro_rules! __extern_methods_no_method_family {
     () => {};
     ($($t:tt)+) => {
         $crate::__macro_helpers::compile_error!(
-            "`#[method_family(...)]` is only supported together with `#[method_id(...)]`"
+            "`#[method_family = ...]` is only supported together with `#[method_id(...)]`"
         )
     };
 }

crates/objc2/src/macros/extern_protocol.rs

@@ -33,8 +33,9 @@
 /// This macro creates an `unsafe` trait with the specified methods. A default
 /// implementation of the method is generated based on the selector specified
 /// with `#[method(a:selector:)]` or `#[method_id(a:selector:)]`.
-/// Similar to [`extern_methods!`], you can use the `#[method_family(...)]`
-/// attribute to override the inferred method family.
+/// Similar to [`extern_methods!`], you can use the
+/// `#[unsafe(method_family = ...)]` attribute to override the inferred method
+/// family.
 ///
 /// Other protocols that this protocol conforms to / inherits can be specified
 /// as supertraits.
@@ -489,7 +490,7 @@ macro_rules! __extern_protocol_no_method_family {
     () => {};
     ($($t:tt)+) => {
         $crate::__macro_helpers::compile_error!(
-            "`#[method_family(...)]` is only supported together with `#[method_id(...)]`"
+            "`#[method_family = ...]` is only supported together with `#[method_id(...)]`"
         )
     };
 }

crates/test-ui/ui/extern_methods_invalid_method_family.rs

@@ -0,0 +1,35 @@
+//! Test various usage of the method_family attribute.
+use objc2::rc::Retained;
+use objc2::runtime::NSObject;
+use objc2::{extern_class, extern_methods};
+
+extern_class!(
+    #[unsafe(super(NSObject))]
+    pub struct MyObject;
+);
+
+extern_methods!(
+    unsafe impl MyObject {
+        #[method_id(noUnsafe)]
+        #[method_family = none]
+        fn no_unsafe(&self) -> Retained<Self>;
+    }
+);
+
+extern_methods!(
+    unsafe impl MyObject {
+        #[method_id(unknownFamily)]
+        #[unsafe(method_family = unknown)]
+        fn unknown_family(&self) -> Retained<Self>;
+    }
+);
+
+extern_methods!(
+    unsafe impl MyObject {
+        #[method(notOnMethodId)]
+        #[unsafe(method_family = none)]
+        fn not_on_method_id(&self);
+    }
+);
+
+fn main() {}