Refactor how NSCopying return types are determined

Previously, we used `CounterpartOrSelf`, which was a nice hack to work
around Rust not having associated type defaults.

Now, we use the user-facing `objc2_foundation::[Mutable]CopyingHelper`,
which are intended to be implemented by the user alongside `NSCopying`.

This is more verbose for the common case where the user just wants to
say that something implements `NSCopying`, but is also better separation
of concerns, and allows us to simplify `ClassType` further.

We could consider a `#[derive(NSCopying)]` that does this, see:
#267

Part of #563

crates/header-translator/src/config.rs

@@ -8,7 +8,7 @@ use heck::ToTrainCase;
 use semver::Version;
 use serde::{de, Deserialize, Deserializer};
 
-use crate::stmt::{Derives, Mutability};
+use crate::stmt::{Counterpart, Derives};
 use crate::ItemIdentifier;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -182,7 +182,10 @@ pub struct ClassData {
     #[serde(default)]
     pub derives: Derives,
     #[serde(default)]
-    pub mutability: Mutability,
+    pub counterpart: Counterpart,
+    #[serde(default)]
+    #[serde(rename = "main-thread-only")]
+    pub main_thread_only: bool,
     #[serde(rename = "skipped-protocols")]
     #[serde(default)]
     pub skipped_protocols: HashSet<String>,
@@ -318,7 +321,7 @@ impl LibraryConfig {
     }
 }
 
-impl<'de> Deserialize<'de> for Mutability {
+impl<'de> Deserialize<'de> for Counterpart {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,
@@ -335,7 +338,7 @@ impl<'de> Deserialize<'de> for Mutability {
         struct MutabilityVisitor;
 
         impl<'de> de::Visitor<'de> for MutabilityVisitor {
-            type Value = Mutability;
+            type Value = Counterpart;
 
             fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
                 formatter.write_str("mutability")
@@ -345,29 +348,25 @@ impl<'de> Deserialize<'de> for Mutability {
             where
                 E: de::Error,
             {
-                if let Some(value) = value.strip_prefix("InteriorMutableWithSubclass(") {
+                if let Some(value) = value.strip_prefix("ImmutableSuperclass(") {
                     let value = value
                         .strip_suffix(')')
                         .ok_or(de::Error::custom("end parenthesis"))?;
                     let item =
                         parse_itemidentifier(value).ok_or(de::Error::custom("requires ::"))?;
-                    return Ok(Mutability::InteriorMutableWithSubclass(item));
+                    return Ok(Counterpart::ImmutableSuperclass(item));
                 }
 
-                if let Some(value) = value.strip_prefix("InteriorMutableWithSuperclass(") {
+                if let Some(value) = value.strip_prefix("MutableSubclass(") {
                     let value = value
                         .strip_suffix(')')
                         .ok_or(de::Error::custom("end parenthesis"))?;
                     let item =
                         parse_itemidentifier(value).ok_or(de::Error::custom("requires ::"))?;
-                    return Ok(Mutability::InteriorMutableWithSuperclass(item));
+                    return Ok(Counterpart::MutableSubclass(item));
                 }
 
-                match value {
-                    "InteriorMutable" => Ok(Mutability::InteriorMutable),
-                    "MainThreadOnly" => Ok(Mutability::MainThreadOnly),
-                    value => Err(de::Error::custom(format!("unknown variant {value:?}"))),
-                }
+                Err(de::Error::custom(format!("unknown variant {value:?}")))
             }
         }
 

crates/header-translator/src/lib.rs

@@ -33,7 +33,7 @@ pub use self::global_analysis::global_analysis;
 pub use self::id::{ItemIdentifier, Location};
 pub use self::library::Library;
 pub use self::module::Module;
-pub use self::stmt::{Mutability, Stmt};
+pub use self::stmt::{Counterpart, Stmt};
 
 pub fn run_cargo_fmt(packages: impl IntoIterator<Item = impl Display>) {
     let status = Command::new("cargo")

crates/header-translator/src/stmt.rs

@@ -3,7 +3,6 @@ use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::collections::HashSet;
 use std::fmt;
-use std::fmt::Display;
 use std::iter;
 
 use clang::{Entity, EntityKind, EntityVisitResult};
@@ -295,8 +294,7 @@ pub(crate) fn items_required_by_decl(
             for (superclass, _, _) in parse_superclasses(entity, context) {
                 items.push(superclass);
             }
-            if let Some(Mutability::InteriorMutableWithSubclass(subclass)) =
-                data.map(|data| &data.mutability)
+            if let Some(Counterpart::MutableSubclass(subclass)) = data.map(|data| &data.counterpart)
             {
                 items.push(subclass.clone());
             }
@@ -382,37 +380,11 @@ fn verify_objc_decl(entity: &Entity<'_>, _context: &Context<'_>) {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Default)]
-pub enum Mutability {
+pub enum Counterpart {
     #[default]
-    InteriorMutable,
-    InteriorMutableWithSubclass(ItemIdentifier),
-    InteriorMutableWithSuperclass(ItemIdentifier),
-    MainThreadOnly,
-}
-
-impl Mutability {
-    fn display<'a>(&'a self, other_generics: impl Display + 'a) -> impl Display + 'a {
-        FormatterFn(move |f| match self {
-            Self::InteriorMutable => write!(f, "InteriorMutable"),
-            Self::InteriorMutableWithSubclass(subclass) => {
-                write!(
-                    f,
-                    "InteriorMutableWithSubclass<{}{}>",
-                    subclass.path(),
-                    other_generics
-                )
-            }
-            Self::InteriorMutableWithSuperclass(superclass) => {
-                write!(
-                    f,
-                    "InteriorMutableWithSuperclass<{}{}>",
-                    superclass.path(),
-                    other_generics
-                )
-            }
-            Self::MainThreadOnly => write!(f, "MainThreadOnly"),
-        })
-    }
+    NoCounterpart,
+    ImmutableSuperclass(ItemIdentifier),
+    MutableSubclass(ItemIdentifier),
 }
 
 #[derive(Debug, Clone, PartialEq)]
@@ -428,7 +400,7 @@ pub enum Stmt {
         superclasses: Vec<(ItemIdentifier, Vec<String>)>,
         designated_initializers: Vec<String>,
         derives: Derives,
-        mutability: Mutability,
+        main_thread_only: bool,
         skipped: bool,
         sendable: bool,
     },
@@ -475,6 +447,7 @@ pub enum Stmt {
         location: Location,
         cls: ItemIdentifier,
         cls_required_items: Vec<ItemIdentifier>,
+        cls_counterpart: Counterpart,
         protocol: ItemIdentifier,
         protocol_required_items: Vec<ItemIdentifier>,
         generics: Vec<String>,
@@ -639,6 +612,9 @@ impl Stmt {
                 let thread_safety = ThreadSafety::from_decl(entity, context);
 
                 let required_items = items_required_by_decl(entity, context);
+                let counterpart = data
+                    .map(|data| data.counterpart.clone())
+                    .unwrap_or_default();
 
                 verify_objc_decl(entity, context);
                 let generics = parse_class_generics(entity, context);
@@ -732,11 +708,7 @@ impl Stmt {
                     superclasses,
                     designated_initializers,
                     derives: data.map(|data| data.derives.clone()).unwrap_or_default(),
-                    mutability: if thread_safety.inferred_mainthreadonly() {
-                        Mutability::MainThreadOnly
-                    } else {
-                        data.map(|data| data.mutability.clone()).unwrap_or_default()
-                    },
+                    main_thread_only: thread_safety.inferred_mainthreadonly(),
                     skipped: data.map(|data| data.definition_skipped).unwrap_or_default(),
                     // Ignore sendability on superclasses; since it's an auto
                     // trait, it's propagated to subclasses anyhow!
@@ -746,6 +718,7 @@ impl Stmt {
                     location: id.location().clone(),
                     cls: id.clone(),
                     cls_required_items: required_items.clone(),
+                    cls_counterpart: counterpart.clone(),
                     protocol: context.replace_protocol_name(p),
                     protocol_required_items: items_required_by_decl(&entity, context),
                     generics: generics.clone(),
@@ -800,6 +773,9 @@ impl Stmt {
 
                 let cls_thread_safety = ThreadSafety::from_decl(&cls_entity, context);
                 let cls_required_items = items_required_by_decl(&cls_entity, context);
+                let cls_counterpart = data
+                    .map(|data| data.counterpart.clone())
+                    .unwrap_or_default();
 
                 verify_objc_decl(entity, context);
                 let generics = parse_class_generics(entity, context);
@@ -818,6 +794,7 @@ impl Stmt {
                     location: category.location().clone(),
                     cls: cls.clone(),
                     cls_required_items: cls_required_items.clone(),
+                    cls_counterpart: cls_counterpart.clone(),
                     generics: generics.clone(),
                     availability: availability.clone(),
                     protocol: context.replace_protocol_name(p),
@@ -849,8 +826,9 @@ impl Stmt {
                         );
                     }
 
-                    let extra_methods = if let Mutability::InteriorMutableWithSubclass(subclass) =
-                        data.map(|data| data.mutability.clone()).unwrap_or_default()
+                    let extra_methods = if let Counterpart::MutableSubclass(subclass) = data
+                        .map(|data| data.counterpart.clone())
+                        .unwrap_or_default()
                     {
                         let subclass_data = context
                             .library(subclass.library_name())
@@ -1663,7 +1641,7 @@ impl Stmt {
                     superclasses,
                     designated_initializers: _,
                     derives,
-                    mutability,
+                    main_thread_only,
                     skipped,
                     sendable,
                 } => {
@@ -1751,12 +1729,11 @@ impl Stmt {
                         superclass.path_in_relation_to(id.location()),
                         GenericTyHelper(superclass_generics),
                     )?;
-                    writeln!(
-                        f,
-                        "        type Mutability = {};",
-                        // Counterpart classes are required to have the same generics.
-                        mutability.display(GenericTyHelper(generics)),
-                    )?;
+                    if *main_thread_only {
+                        writeln!(f, "        type Mutability = MainThreadOnly;")?;
+                    } else {
+                        writeln!(f, "        type Mutability = InteriorMutable;")?;
+                    }
                     if !generics.is_empty() {
                         writeln!(f)?;
                         writeln!(
@@ -1921,6 +1898,7 @@ impl Stmt {
                     location: id,
                     cls,
                     cls_required_items: _,
+                    cls_counterpart,
                     generics,
                     protocol,
                     protocol_required_items: _,
@@ -1974,6 +1952,47 @@ impl Stmt {
                         GenericTyHelper(generics),
                         WhereBoundHelper(generics, where_bound)
                     )?;
+
+                    // To make `NSCopying` and `NSMutableCopying` work, we
+                    // need to emit `CopyingHelper` impls to tell Rust which
+                    // return types they have.
+                    if matches!(&*protocol.name, "NSCopying" | "NSMutableCopying") {
+                        let copy_helper = if protocol.name == "NSCopying" {
+                            protocol.clone().map_name(|_| "CopyingHelper".to_string())
+                        } else {
+                            protocol
+                                .clone()
+                                .map_name(|_| "MutableCopyingHelper".to_string())
+                        };
+
+                        writeln!(f)?;
+                        write!(f, "{}", self.cfg_gate_ln(config))?;
+                        writeln!(
+                            f,
+                            "unsafe impl{} {} for {}{} {{",
+                            GenericParamsHelper(generics, "?Sized + Message"),
+                            copy_helper.path_in_relation_to(id),
+                            cls.path_in_relation_to(id),
+                            GenericTyHelper(generics),
+                        )?;
+
+                        write!(f, "    type Result = ")?;
+                        // Assume counterparts have the same generics.
+                        match (cls_counterpart, &*protocol.name) {
+                            (Counterpart::ImmutableSuperclass(superclass), "NSCopying") => {
+                                write!(f, "{}", superclass.path_in_relation_to(id))?;
+                                write!(f, "{}", GenericTyHelper(generics))?;
+                            }
+                            (Counterpart::MutableSubclass(subclass), "NSMutableCopying") => {
+                                write!(f, "{}", subclass.path_in_relation_to(id))?;
+                                write!(f, "{}", GenericTyHelper(generics))?;
+                            }
+                            _ => write!(f, "Self")?,
+                        }
+                        writeln!(f, ";")?;
+
+                        writeln!(f, "}}")?;
+                    }
                 }
                 Self::ProtocolDecl {
                     id,

crates/header-translator/src/thread_safety.rs

@@ -1,11 +1,12 @@
 use clang::{Entity, EntityKind};
+use serde::Deserialize;
 
 use crate::{
     immediate_children,
     method::MethodModifiers,
     stmt::{method_or_property_entities, parse_direct_protocols, parse_superclasses},
     unexposed_attr::UnexposedAttr,
-    Context, ItemIdentifier, Mutability,
+    Context, ItemIdentifier,
 };
 
 fn parse(entity: &Entity<'_>, context: &Context<'_>) -> (Option<bool>, bool) {
@@ -34,7 +35,7 @@ fn parse(entity: &Entity<'_>, context: &Context<'_>) -> (Option<bool>, bool) {
     (sendable, mainthreadonly)
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Deserialize)]
 pub(crate) enum ThreadSafetyAttr {
     /// The item is only accessible from the main thread.
     ///
@@ -79,9 +80,7 @@ impl ThreadSafetyAttr {
                 let id = ItemIdentifier::new(entity, context);
                 let data = context.library(id.library_name()).class_data.get(&id.name);
 
-                if data.map(|data| data.mutability.clone()).unwrap_or_default()
-                    == Mutability::MainThreadOnly
-                {
+                if data.map(|data| data.main_thread_only).unwrap_or_default() {
                     return Some(Self::MainThreadOnly);
                 }
             }

crates/objc2/CHANGELOG.md

@@ -55,9 +55,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * **BREAKING**: Removed `AsMut`, `BorrowMut` and `DerefMut` implementations in
   `extern_class!` and `declare_class!`.
 * **BREAKING**: Removed `mutability` options `Immutable`, `Mutable`,
-  `ImmutableWithMutableSubclass` and `MutableWithImmutableSuperclass`, along
-  with the related `IsAllowedMutable`, `IsIdCloneable`, `IsRetainable` and
-  `IsMutable` traits.
+  `ImmutableWithMutableSubclass`, `MutableWithImmutableSuperclass`,
+  `InteriorMutableWithSubclass` and `InteriorMutableWithSuperclass`, along
+  with the related `IsAllowedMutable`, `CounterpartOrSelf`, `IsIdCloneable`,
+  `IsRetainable` and `IsMutable` traits.
 * **BREAKING**: Removed `DerefMut` implementation for `Retained<T>` when the
   `Retained` was mutable.
 * **BREAKING**: Moved `retain` from `ClassType` to `Message`.

crates/objc2/src/__framework_prelude.rs

@@ -13,10 +13,7 @@ pub use std::os::raw::{
 
 pub use crate::encode::{Encode, Encoding, RefEncode};
 pub use crate::ffi::{NSInteger, NSIntegerMax, NSUInteger, NSUIntegerMax};
-pub use crate::mutability::{
-    InteriorMutable, InteriorMutableWithSubclass, InteriorMutableWithSuperclass, IsMainThreadOnly,
-    MainThreadOnly,
-};
+pub use crate::mutability::{InteriorMutable, IsMainThreadOnly, MainThreadOnly};
 pub use crate::rc::{Allocated, DefaultId, DefaultRetained, Id, Retained};
 pub use crate::runtime::{
     AnyClass, AnyObject, Bool, NSObject, NSObjectProtocol, ProtocolObject, Sel,