Verify class mutability in extern_class! and declare_class!

Author: madsmtm

crates/objc2/src/__macro_helpers/declare_class.rs

@@ -3,9 +3,10 @@ use core::ptr;
 
 use crate::declare::__IdReturnValue;
 use crate::rc::{Allocated, Id};
-use crate::{Message, MessageReceiver};
+use crate::{ClassType, Message, MessageReceiver};
 
 use super::{CopyOrMutCopy, Init, MaybeUnwrap, New, Other};
+use crate::mutability;
 
 // One could imagine a different design where we simply had a method like
 // `fn convert_receiver()`, but that won't work in `declare_class!` since we
@@ -110,3 +111,70 @@ impl<T: Message> MaybeOptionId for Option<Id<T>> {
         __IdReturnValue(ptr.cast())
     }
 }
+
+/// Helper for ensuring that `ClassType::Mutability` is implemented correctly
+/// for subclasses.
+pub trait ValidSubclassMutability<T: mutability::Mutability> {}
+
+// Root
+impl ValidSubclassMutability<mutability::Immutable> for mutability::Root {}
+impl ValidSubclassMutability<mutability::Mutable> for mutability::Root {}
+impl<MS, IS> ValidSubclassMutability<mutability::ImmutableWithMutableSubclass<MS>>
+    for mutability::Root
+where
+    MS: ?Sized + ClassType<Mutability = mutability::MutableWithImmutableSuperclass<IS>>,
+    IS: ?Sized + ClassType<Mutability = mutability::ImmutableWithMutableSubclass<MS>>,
+{
+}
+impl ValidSubclassMutability<mutability::InteriorMutable> for mutability::Root {}
+impl ValidSubclassMutability<mutability::MainThreadOnly> for mutability::Root {}
+
+// Immutable
+impl ValidSubclassMutability<mutability::Immutable> for mutability::Immutable {}
+
+// Mutable
+impl ValidSubclassMutability<mutability::Mutable> for mutability::Mutable {}
+
+// ImmutableWithMutableSubclass
+impl<MS, IS> ValidSubclassMutability<mutability::MutableWithImmutableSuperclass<IS>>
+    for mutability::ImmutableWithMutableSubclass<MS>
+where
+    MS: ?Sized + ClassType<Mutability = mutability::MutableWithImmutableSuperclass<IS>>,
+    IS: ?Sized + ClassType<Mutability = mutability::ImmutableWithMutableSubclass<MS>>,
+{
+}
+// Only valid when `NSCopying`/`NSMutableCopying` is not implemented!
+impl<MS: ?Sized + ClassType> ValidSubclassMutability<mutability::Immutable>
+    for mutability::ImmutableWithMutableSubclass<MS>
+{
+}
+
+// MutableWithImmutableSuperclass
+// Only valid when `NSCopying`/`NSMutableCopying` is not implemented!
+impl<IS: ?Sized + ClassType> ValidSubclassMutability<mutability::Mutable>
+    for mutability::MutableWithImmutableSuperclass<IS>
+{
+}
+
+// InteriorMutable
+impl ValidSubclassMutability<mutability::InteriorMutable> for mutability::InteriorMutable {}
+impl ValidSubclassMutability<mutability::MainThreadOnly> for mutability::InteriorMutable {}
+
+// MainThreadOnly
+impl ValidSubclassMutability<mutability::MainThreadOnly> for mutability::MainThreadOnly {}
+
+/// Ensure that:
+/// 1. The type is not a root class (it's superclass implements `ClassType`,
+///    and it's mutability is not `Root`), and therefore also implements basic
+///    memory management methods, as required by `unsafe impl Message`.
+/// 2. The mutability is valid according to the superclass' mutability.
+#[inline]
+pub fn assert_mutability_matches_superclass_mutability<T>()
+where
+    T: ?Sized + ClassType,
+    T::Super: ClassType,
+    T::Mutability: mutability::Mutability,
+    <T::Super as ClassType>::Mutability: ValidSubclassMutability<T::Mutability>,
+{
+    // Noop
+}

crates/objc2/src/__macro_helpers/mod.rs

@@ -32,7 +32,10 @@ mod cache;
 mod declare_class;
 
 pub use self::cache::{CachedClass, CachedSel};
-pub use self::declare_class::{MaybeOptionId, MessageRecieveId};
+pub use self::declare_class::{
+    assert_mutability_matches_superclass_mutability, MaybeOptionId, MessageRecieveId,
+    ValidSubclassMutability,
+};
 
 // Common selectors.
 //

crates/objc2/src/macros/declare_class.rs

@@ -533,6 +533,8 @@ macro_rules! __inner_declare_class {
             const NAME: &'static $crate::__macro_helpers::str = $name_const;
 
             fn class() -> &'static $crate::runtime::Class {
+                $crate::__macro_helpers::assert_mutability_matches_superclass_mutability::<Self>();
+
                 // TODO: Use `core::cell::LazyCell`
                 static REGISTER_CLASS: $crate::__macro_helpers::Once = $crate::__macro_helpers::Once::new();
 

crates/objc2/src/macros/extern_class.rs

@@ -365,6 +365,8 @@ macro_rules! __inner_extern_class {
 
             #[inline]
             fn class() -> &'static $crate::runtime::Class {
+                $crate::__macro_helpers::assert_mutability_matches_superclass_mutability::<Self>();
+
                 $crate::__class_inner!(
                     $crate::__select_name!($name; $($name_const)?),
                     $crate::__hash_idents!($name),
@@ -416,8 +418,12 @@ macro_rules! __extern_class_impl_traits {
         // (we even ensure that `Object` is always last in our inheritance
         // tree), so it is always safe to reinterpret as that.
         //
-        // That the object must work with standard memory management is upheld
-        // by the caller.
+        // That the object must work with standard memory management is
+        // properly upheld by the fact that the superclass is required by
+        // `assert_mutability_matches_superclass_mutability` to implement
+        // `ClassType`, and hence must be a subclass of one of `NSObject`,
+        // `NSProxy` or some other class that ensures this (e.g. the object
+        // itself is not a root class).
         $(#[$impl_m])*
         unsafe impl<$($t)*> $crate::Message for $for {}
 

crates/test-ui/ui/extern_class_root.rs

@@ -0,0 +1,39 @@
+use core::ops::{Deref, DerefMut};
+
+use objc2::encode::{Encoding, RefEncode};
+use objc2::runtime::Object;
+use objc2::{extern_class, mutability, ClassType, Message};
+
+#[repr(transparent)]
+struct MyObject(Object);
+
+unsafe impl RefEncode for MyObject {
+    const ENCODING_REF: Encoding = Encoding::Object;
+}
+
+unsafe impl Message for MyObject {}
+
+impl Deref for MyObject {
+    type Target = Object;
+
+    fn deref(&self) -> &Object {
+        &self.0
+    }
+}
+
+impl DerefMut for MyObject {
+    fn deref_mut(&mut self) -> &mut Object {
+        &mut self.0
+    }
+}
+
+extern_class!(
+    pub struct MyRootClass;
+
+    unsafe impl ClassType for MyRootClass {
+        type Super = MyObject;
+        type Mutability = mutability::InteriorMutable;
+    }
+);
+
+fn main() {}

crates/test-ui/ui/extern_class_root.stderr

@@ -0,0 +1,18 @@
+error[E0277]: the trait bound `MyObject: ClassType` is not satisfied
+ --> ui/extern_class_root.rs
+  |
+  | / extern_class!(
+  | |     pub struct MyRootClass;
+  | |
+  | |     unsafe impl ClassType for MyRootClass {
+... |
+  | |     }
+  | | );
+  | |_^ the trait `ClassType` is not implemented for `MyObject`
+  |
+  = help: the following other types implement trait `ClassType`:
+            MyRootClass
+            NSObject
+            __NSProxy
+            __RcTestObject
+  = note: this error originates in the macro `$crate::__inner_extern_class` which comes from the expansion of the macro `extern_class` (in Nightly builds, run with -Z macro-backtrace for more info)

crates/test-ui/ui/extern_class_subclass_object.rs

@@ -0,0 +1,13 @@
+use objc2::runtime::Object;
+use objc2::{extern_class, mutability, ClassType};
+
+extern_class!(
+    pub struct MyRootClass;
+
+    unsafe impl ClassType for MyRootClass {
+        type Super = Object;
+        type Mutability = mutability::InteriorMutable;
+    }
+);
+
+fn main() {}

crates/test-ui/ui/extern_class_subclass_object.stderr

@@ -0,0 +1,67 @@
+error[E0119]: conflicting implementations of trait `AsRef<objc2::runtime::Object>` for type `MyRootClass`
+ --> ui/extern_class_subclass_object.rs
+  |
+  | / extern_class!(
+  | |     pub struct MyRootClass;
+  | |
+  | |     unsafe impl ClassType for MyRootClass {
+... |
+  | |     }
+  | | );
+  | | ^
+  | | |
+  | |_first implementation here
+  |   conflicting implementation for `MyRootClass`
+  |
+  = note: this error originates in the macro `$crate::__impl_as_ref_borrow` which comes from the expansion of the macro `extern_class` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0119]: conflicting implementations of trait `AsMut<objc2::runtime::Object>` for type `MyRootClass`
+ --> ui/extern_class_subclass_object.rs
+  |
+  | / extern_class!(
+  | |     pub struct MyRootClass;
+  | |
+  | |     unsafe impl ClassType for MyRootClass {
+... |
+  | |     }
+  | | );
+  | | ^
+  | | |
+  | |_first implementation here
+  |   conflicting implementation for `MyRootClass`
+  |
+  = note: this error originates in the macro `$crate::__impl_as_ref_borrow` which comes from the expansion of the macro `extern_class` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0119]: conflicting implementations of trait `Borrow<objc2::runtime::Object>` for type `MyRootClass`
+ --> ui/extern_class_subclass_object.rs
+  |
+  | / extern_class!(
+  | |     pub struct MyRootClass;
+  | |
+  | |     unsafe impl ClassType for MyRootClass {
+... |
+  | |     }
+  | | );
+  | | ^
+  | | |
+  | |_first implementation here
+  |   conflicting implementation for `MyRootClass`
+  |
+  = note: this error originates in the macro `$crate::__impl_as_ref_borrow` which comes from the expansion of the macro `extern_class` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0119]: conflicting implementations of trait `BorrowMut<objc2::runtime::Object>` for type `MyRootClass`
+ --> ui/extern_class_subclass_object.rs
+  |
+  | / extern_class!(
+  | |     pub struct MyRootClass;
+  | |
+  | |     unsafe impl ClassType for MyRootClass {
+... |
+  | |     }
+  | | );
+  | | ^
+  | | |
+  | |_first implementation here
+  |   conflicting implementation for `MyRootClass`
+  |
+  = note: this error originates in the macro `$crate::__impl_as_ref_borrow` which comes from the expansion of the macro `extern_class` (in Nightly builds, run with -Z macro-backtrace for more info)

crates/test-ui/ui/extern_class_wrong_mutability.rs

@@ -0,0 +1,58 @@
+use objc2::runtime::NSObject;
+use objc2::{extern_class, mutability, ClassType};
+
+extern_class!(
+    pub struct MyMainThreadClass;
+
+    unsafe impl ClassType for MyMainThreadClass {
+        type Super = NSObject;
+        type Mutability = mutability::MainThreadOnly;
+    }
+);
+
+extern_class!(
+    pub struct MyAnyThreadClass;
+
+    unsafe impl ClassType for MyAnyThreadClass {
+        type Super = MyMainThreadClass;
+        type Mutability = mutability::InteriorMutable;
+    }
+);
+
+extern_class!(
+    pub struct MyImmutableClass1;
+
+    unsafe impl ClassType for MyImmutableClass1 {
+        type Super = NSObject;
+        type Mutability = mutability::ImmutableWithMutableSubclass<MyMutableClass1>;
+    }
+);
+
+extern_class!(
+    pub struct MyMutableClass1;
+
+    unsafe impl ClassType for MyMutableClass1 {
+        type Super = MyImmutableClass1;
+        type Mutability = mutability::MutableWithImmutableSuperclass<NSObject>;
+    }
+);
+
+extern_class!(
+    pub struct MyImmutableClass2;
+
+    unsafe impl ClassType for MyImmutableClass2 {
+        type Super = NSObject;
+        type Mutability = mutability::ImmutableWithMutableSubclass<NSObject>;
+    }
+);
+
+extern_class!(
+    pub struct MyMutableClass2;
+
+    unsafe impl ClassType for MyMutableClass2 {
+        type Super = MyImmutableClass2;
+        type Mutability = mutability::MutableWithImmutableSuperclass<MyImmutableClass2>;
+    }
+);
+
+fn main() {}