Add support for storing Id and Box in ivars

Author: madsmtm

objc2/CHANGELOG.md

@@ -8,8 +8,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ## Unreleased - YYYY-MM-DD
 
 ### Added
-* `Ivar::write`, `Ivar::as_ptr` and `Ivar::as_mut_ptr` for querying/modifying
-  the instance variable inside `init` methods.
+* Added `Ivar::write`, `Ivar::as_ptr` and `Ivar::as_mut_ptr` for safely
+  querying and modifying instance variables inside `init` methods.
+* Added `IvarDrop<T>` to allow storing complex `Drop` values in ivars
+  (currently `rc::Id<T, O>`, `Box<T>`, `Option<rc::Id<T, O>>` or
+  `Option<Box<T>>`).
 
 ### Removed
 * **BREAKING**: `MaybeUninit` no longer implements `IvarType` directly; use

objc2/examples/delegate.rs

@@ -1,9 +1,9 @@
 #![cfg_attr(not(all(feature = "apple", target_os = "macos")), allow(unused))]
-use objc2::declare::Ivar;
-use objc2::foundation::NSObject;
+use objc2::declare::{Ivar, IvarDrop};
+use objc2::foundation::{NSCopying, NSObject, NSString};
 use objc2::rc::{Id, Shared};
 use objc2::runtime::Object;
-use objc2::{declare_class, extern_class, msg_send, msg_send_id, ClassType};
+use objc2::{declare_class, extern_class, msg_send, msg_send_id, ns_string, ClassType};
 
 #[cfg(all(feature = "apple", target_os = "macos"))]
 #[link(name = "AppKit", kind = "framework")]
@@ -23,6 +23,10 @@ declare_class!(
     struct CustomAppDelegate {
         pub ivar: u8,
         another_ivar: bool,
+        box_ivar: IvarDrop<Box<i32>>,
+        maybe_box_ivar: IvarDrop<Option<Box<i32>>>,
+        id_ivar: IvarDrop<Id<NSString, Shared>>,
+        maybe_id_ivar: IvarDrop<Option<Id<NSString, Shared>>>,
     }
 
     unsafe impl ClassType for CustomAppDelegate {
@@ -34,18 +38,28 @@ declare_class!(
         #[sel(initWith:another:)]
         fn init_with(self: &mut Self, ivar: u8, another_ivar: bool) -> Option<&mut Self> {
             let this: Option<&mut Self> = unsafe { msg_send![super(self), init] };
+
+            // TODO: `ns_string` can't be used inside closures; investigate!
+            let s = ns_string!("def");
+
             this.map(|this| {
+                // Initialize instance variables
+
+                // The explicit way to initialize an instance variable:
                 Ivar::write(&mut this.ivar, ivar);
-                Ivar::write(&mut this.another_ivar, another_ivar);
-                // Note that we could have done this with just:
-                // *this.ivar = ivar;
-                // *this.another_ivar = another_ivar;
-                //
-                // Since these two ivar types (`u8` and `bool`) are safe to
-                // initialize from all zeroes; but for this example, we chose
-                // to be explicit.
-
-                // SAFETY: All the instance variables have been initialized
+                // For some types like `u8`, `bool`, `Option<Box<T>>` and
+                // `Option<Id<T, O>>` which are safe to zero-initialize, we
+                // can simply write to the variable as normal:
+                *this.another_ivar = another_ivar;
+                *this.maybe_box_ivar = None;
+                *this.maybe_id_ivar = Some(s.copy());
+                // While for others like `&u8`, `Box<T>` or `Id<T, O>`, we
+                // have to initialize it with `Ivar::write`:
+                Ivar::write(&mut this.box_ivar, Box::new(2));
+                Ivar::write(&mut this.id_ivar, NSString::from_str("abc"));
+
+                // All the instance variables have been initialized; our
+                // initializer is sound
                 this
             })
         }
@@ -97,8 +111,12 @@ impl CustomAppDelegate {
 fn main() {
     let delegate = CustomAppDelegate::new(42, true);
 
-    println!("{}", delegate.ivar);
-    println!("{}", delegate.another_ivar);
+    println!("{:?}", delegate.ivar);
+    println!("{:?}", delegate.another_ivar);
+    println!("{:?}", delegate.box_ivar);
+    println!("{:?}", delegate.maybe_box_ivar);
+    println!("{:?}", delegate.id_ivar);
+    println!("{:?}", delegate.maybe_id_ivar);
 }
 
 #[cfg(not(all(feature = "apple", target_os = "macos")))]

objc2/src/__macro_helpers.rs

@@ -13,6 +13,7 @@ pub use core::mem::size_of;
 pub use core::ops::{Deref, DerefMut};
 pub use core::option::Option::{self, None, Some};
 pub use core::primitive::{bool, str, u8};
+pub use core::ptr::drop_in_place;
 pub use core::{compile_error, concat, panic, stringify};
 // TODO: Use `core::cell::LazyCell`
 pub use std::sync::Once;

objc2/src/declare.rs

@@ -112,6 +112,7 @@
 //! ```
 
 mod ivar;
+mod ivar_drop;
 mod ivar_forwarding_impls;
 
 use alloc::format;
@@ -122,13 +123,14 @@ use core::ptr;
 use core::ptr::NonNull;
 use std::ffi::CString;
 
-use crate::encode::{Encode, EncodeArguments, EncodeConvert, Encoding, RefEncode};
+use crate::encode::{Encode, EncodeArguments, Encoding, RefEncode};
 use crate::ffi;
 use crate::runtime::{Bool, Class, Imp, Object, Protocol, Sel};
 use crate::sel;
 use crate::Message;
 
-pub use ivar::{Ivar, IvarType};
+pub use ivar::{InnerIvarType, Ivar, IvarType};
+pub use ivar_drop::IvarDrop;
 
 pub(crate) mod private {
     pub trait Sealed {}
@@ -439,7 +441,12 @@ impl ClassBuilder {
     /// Same as [`ClassBuilder::add_ivar`].
     pub fn add_static_ivar<T: IvarType>(&mut self) {
         // SAFETY: The encoding is correct
-        unsafe { self.add_ivar_inner::<T::Type>(T::NAME, &T::Type::__ENCODING) }
+        unsafe {
+            self.add_ivar_inner::<<T::Type as InnerIvarType>::__Inner>(
+                T::NAME,
+                &T::Type::__ENCODING,
+            )
+        }
     }
 
     /// Adds the given protocol to self.

objc2/src/declare/ivar.rs

@@ -2,11 +2,79 @@ use core::fmt;
 use core::marker::PhantomData;
 use core::mem::MaybeUninit;
 use core::ops::{Deref, DerefMut};
-use core::ptr::NonNull;
+use core::ptr::{self, NonNull};
 
-use crate::encode::EncodeConvert;
+use crate::encode::{EncodeConvert, Encoding};
 use crate::runtime::{ivar_offset, Object};
 
+/// Types that may be used in ivars.
+///
+/// This may be either:
+/// - [`bool`].
+/// - [`IvarDrop<T>`][super::IvarDrop].
+/// - Something that implements [`Encode`][crate::Encode].
+///
+/// This is a sealed
+///
+///
+/// # Safety
+///
+/// You cannot rely on any safety guarantees from this.
+pub unsafe trait InnerIvarType {
+    #[doc(hidden)]
+    const __ENCODING: Encoding;
+
+    // SAFETY: It must be safe to transmute from `__Inner` to `Output`.
+    #[doc(hidden)]
+    type __Inner;
+
+    /// The type that an `Ivar` containing this will dereference to.
+    ///
+    /// E.g. `Ivar<IvarDrop<Box<u8>>>` will deref to `Box<u8>`.
+    type Output;
+
+    // SAFETY: The __Inner type must be safe to drop even if zero-initialized.
+    #[doc(hidden)]
+    const __MAY_DROP: bool;
+
+    #[doc(hidden)]
+    unsafe fn __to_ref(inner: &Self::__Inner) -> &Self::Output;
+
+    #[doc(hidden)]
+    unsafe fn __to_mut(inner: &mut Self::__Inner) -> &mut Self::Output;
+
+    #[doc(hidden)]
+    fn __to_ptr(inner: NonNull<Self::__Inner>) -> NonNull<Self::Output>;
+}
+
+unsafe impl<T: EncodeConvert> InnerIvarType for T {
+    const __ENCODING: Encoding = <Self as EncodeConvert>::__ENCODING;
+    type __Inner = Self;
+    type Output = Self;
+    // Note: We explicitly tell `Ivar` that it shouldn't do anything to drop,
+    // since if the object was deallocated before an `init` method was called,
+    // the ivar would not have been initialized properly!
+    //
+    // For example in the case of `NonNull<u8>`, it would be zero-initialized
+    // which is an invalid state for that.
+    const __MAY_DROP: bool = false;
+
+    #[inline]
+    unsafe fn __to_ref(inner: &Self::__Inner) -> &Self::Output {
+        inner
+    }
+
+    #[inline]
+    unsafe fn __to_mut(inner: &mut Self::__Inner) -> &mut Self::Output {
+        inner
+    }
+
+    #[inline]
+    fn __to_ptr(inner: NonNull<Self::__Inner>) -> NonNull<Self::Output> {
+        inner
+    }
+}
+
 /// Helper trait for defining instance variables.
 ///
 /// This should be implemented for an empty marker type, which can then be
@@ -39,7 +107,7 @@ use crate::runtime::{ivar_offset, Object};
 /// ```
 pub unsafe trait IvarType {
     /// The type of the instance variable.
-    type Type: EncodeConvert;
+    type Type: InnerIvarType;
     /// The name of the instance variable.
     const NAME: &'static str;
 
@@ -132,7 +200,18 @@ pub struct Ivar<T: IvarType> {
     /// Make this type allowed in `repr(C)`
     inner: [u8; 0],
     /// For proper variance and auto traits
-    item: PhantomData<T::Type>,
+    item: PhantomData<<T::Type as InnerIvarType>::Output>,
+}
+
+impl<T: IvarType> Drop for Ivar<T> {
+    #[inline]
+    fn drop(&mut self) {
+        if <T::Type as InnerIvarType>::__MAY_DROP {
+            // SAFETY: We drop the inner type, which is guaranteed by
+            // `__MAY_DROP` to always be safe to drop.
+            unsafe { ptr::drop_in_place(self.as_inner_mut_ptr().as_ptr()) }
+        }
+    }
 }
 
 impl<T: IvarType> Ivar<T> {
@@ -143,8 +222,12 @@ impl<T: IvarType> Ivar<T> {
     ///
     /// This is similar to [`MaybeUninit::as_ptr`], see that for usage
     /// instructions.
-    pub fn as_ptr(this: &Self) -> *const T::Type {
-        let ptr: NonNull<Object> = NonNull::from(this).cast();
+    pub fn as_ptr(this: &Self) -> *const <T::Type as InnerIvarType>::Output {
+        T::Type::__to_ptr(this.as_inner_ptr()).as_ptr()
+    }
+
+    fn as_inner_ptr(&self) -> NonNull<<T::Type as InnerIvarType>::__Inner> {
+        let ptr: NonNull<Object> = NonNull::from(self).cast();
 
         // SAFETY: The user ensures that this is placed in a struct that can
         // be reinterpreted as an `Object`. Since `Ivar` can never be
@@ -159,9 +242,7 @@ impl<T: IvarType> Ivar<T> {
         // so that is fine.
         let offset = unsafe { T::__offset(ptr) };
         // SAFETY: The offset is valid
-        let ptr = unsafe { Object::ivar_at_offset::<T::Type>(ptr, offset) };
-
-        ptr.as_ptr()
+        unsafe { Object::ivar_at_offset::<<T::Type as InnerIvarType>::__Inner>(ptr, offset) }
     }
 
     /// Get a mutable pointer to the instance variable.
@@ -174,16 +255,17 @@ impl<T: IvarType> Ivar<T> {
     ///
     /// This is similar to [`MaybeUninit::as_mut_ptr`], see that for usage
     /// instructions.
-    fn as_mut_ptr(this: &mut Self) -> *mut T::Type {
-        let ptr: NonNull<Object> = NonNull::from(this).cast();
+    pub fn as_mut_ptr(this: &mut Self) -> *mut <T::Type as InnerIvarType>::Output {
+        T::Type::__to_ptr(this.as_inner_mut_ptr()).as_ptr()
+    }
+
+    fn as_inner_mut_ptr(&mut self) -> NonNull<<T::Type as InnerIvarType>::__Inner> {
+        let ptr: NonNull<Object> = NonNull::from(self).cast();
 
-        // SAFETY: Same as `as_ptr`
+        // SAFETY: Same as `as_inner_ptr`
         let offset = unsafe { T::__offset(ptr) };
         // SAFETY: The offset is valid
-        let ptr = unsafe { Object::ivar_at_offset::<T::Type>(ptr, offset) };
-
-        // Safe as *mut T because it came from `&mut Self`
-        ptr.as_ptr()
+        unsafe { Object::ivar_at_offset::<<T::Type as InnerIvarType>::__Inner>(ptr, offset) }
     }
 
     /// Sets the value of the instance variable.
@@ -193,15 +275,19 @@ impl<T: IvarType> Ivar<T> {
     ///
     /// This is similar to [`MaybeUninit::write`], see that for usage
     /// instructions.
-    pub fn write(this: &mut Self, val: T::Type) -> &mut T::Type {
-        let ptr: *mut MaybeUninit<T::Type> = Self::as_mut_ptr(this).cast();
+    pub fn write(
+        this: &mut Self,
+        val: <T::Type as InnerIvarType>::Output,
+    ) -> &mut <T::Type as InnerIvarType>::Output {
+        let ptr: *mut MaybeUninit<<T::Type as InnerIvarType>::Output> =
+            Self::as_mut_ptr(this).cast();
         let ivar = unsafe { ptr.as_mut().unwrap_unchecked() };
         ivar.write(val)
     }
 }
 
 impl<T: IvarType> Deref for Ivar<T> {
-    type Target = T::Type;
+    type Target = <T::Type as InnerIvarType>::Output;
 
     #[inline]
     fn deref(&self) -> &Self::Target {
@@ -210,7 +296,7 @@ impl<T: IvarType> Deref for Ivar<T> {
         //
         // Since all accesses to a particular ivar only goes through one
         // `Ivar`, if we have `&Ivar` we know that `&T` is safe.
-        unsafe { Self::as_ptr(self).as_ref().unwrap_unchecked() }
+        unsafe { T::Type::__to_ref(self.as_inner_ptr().as_ref()) }
     }
 }
 
@@ -241,25 +327,27 @@ impl<T: IvarType> DerefMut for Ivar<T> {
         //
         // And using `mut` would create aliasing mutable reference to the
         // object.
-        unsafe { Self::as_mut_ptr(self).as_mut().unwrap_unchecked() }
+        unsafe { T::Type::__to_mut(self.as_inner_mut_ptr().as_mut()) }
     }
 }
 
 /// Format as a pointer to the instance variable.
 impl<T: IvarType> fmt::Pointer for Ivar<T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let ptr: *const T::Type = &**self;
-        fmt::Pointer::fmt(&ptr, f)
+        fmt::Pointer::fmt(&Self::as_ptr(self), f)
     }
 }
 
 #[cfg(test)]
 mod tests {
     use core::mem;
     use core::panic::{RefUnwindSafe, UnwindSafe};
+    use std::sync::atomic::{AtomicBool, Ordering};
 
     use super::*;
-    use crate::{msg_send, test_utils, MessageReceiver};
+    use crate::foundation::NSObject;
+    use crate::rc::{Id, Owned};
+    use crate::{declare_class, msg_send, msg_send_id, test_utils, ClassType, MessageReceiver};
 
     struct TestIvar;
 
@@ -297,4 +385,31 @@ mod tests {
         };
         assert_eq!(*obj.foo, 42);
     }
+
+    #[test]
+    fn ensure_custom_drop_is_possible() {
+        static HAS_RUN_DEALLOC: AtomicBool = AtomicBool::new(false);
+
+        declare_class!(
+            #[derive(Debug, PartialEq)]
+            struct CustomDrop {
+                ivar: u8,
+            }
+
+            unsafe impl ClassType for CustomDrop {
+                type Super = NSObject;
+            }
+
+            unsafe impl CustomDrop {
+                #[sel(dealloc)]
+                fn dealloc(&mut self) {
+                    HAS_RUN_DEALLOC.store(true, Ordering::SeqCst);
+                }
+            }
+        );
+
+        let _: Id<CustomDrop, Owned> = unsafe { msg_send_id![CustomDrop::class(), new] };
+
+        assert!(HAS_RUN_DEALLOC.load(Ordering::SeqCst));
+    }
 }