Merge pull request #252 from madsmtm/ivar-init

Better ivar initialization

Author: madsmtm

objc2/CHANGELOG.md

@@ -7,6 +7,14 @@ 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.
+
+### Removed
+* **BREAKING**: `MaybeUninit` no longer implements `IvarType` directly; use
+  `Ivar::write` instead.
+
 ## 0.3.0-beta.2 - 2022-08-28
 
 ### Added

objc2/examples/class_with_lifetime.rs

@@ -4,7 +4,6 @@
 //! support such use-cases. Instead, we'll declare the class manually!
 #![deny(unsafe_op_in_unsafe_fn)]
 use std::marker::PhantomData;
-use std::mem::MaybeUninit;
 use std::sync::Once;
 
 use objc2::declare::{ClassBuilder, Ivar, IvarType};
@@ -46,6 +45,19 @@ unsafe impl RefEncode for MyObject<'_> {
 unsafe impl Message for MyObject<'_> {}
 
 impl<'a> MyObject<'a> {
+    unsafe extern "C" fn init_with_ptr(
+        &mut self,
+        _cmd: Sel,
+        ptr: Option<&'a mut u8>,
+    ) -> Option<&'a mut Self> {
+        let this: Option<&mut Self> = unsafe { msg_send![super(self), init] };
+        this.map(|this| {
+            // Properly initialize the number reference
+            Ivar::write(&mut this.number, ptr.expect("got NULL number ptr"));
+            this
+        })
+    }
+
     pub fn new(number: &'a mut u8) -> Id<Self, Owned> {
         // SAFETY: The lifetime of the reference is properly bound to the
         // returned type
@@ -77,38 +89,10 @@ unsafe impl<'a> ClassType for MyObject<'a> {
 
             builder.add_static_ivar::<NumberIvar<'a>>();
 
-            /// Helper struct since we can't access the instance variable
-            /// from inside MyObject, since it hasn't been initialized yet!
-            #[repr(C)]
-            struct PartialInit<'a> {
-                inner: NSObject,
-                number: Ivar<MaybeUninit<NumberIvar<'a>>>,
-            }
-            unsafe impl RefEncode for PartialInit<'_> {
-                const ENCODING_REF: Encoding = Encoding::Object;
-            }
-            unsafe impl Message for PartialInit<'_> {}
-
-            impl<'a> PartialInit<'a> {
-                unsafe extern "C" fn init_with_ptr(
-                    this: &mut Self,
-                    _cmd: Sel,
-                    ptr: Option<&'a mut u8>,
-                ) -> Option<&'a mut Self> {
-                    let this: Option<&mut Self> =
-                        unsafe { msg_send![super(this, NSObject::class()), init] };
-                    this.map(|this| {
-                        // Properly initialize the number reference
-                        this.number.write(ptr.expect("got NULL number ptr"));
-                        this
-                    })
-                }
-            }
-
             unsafe {
                 builder.add_method(
                     sel!(initWithPtr:),
-                    PartialInit::init_with_ptr as unsafe extern "C" fn(_, _, _) -> _,
+                    Self::init_with_ptr as unsafe extern "C" fn(_, _, _) -> _,
                 );
             }
 

objc2/examples/delegate.rs

@@ -1,4 +1,5 @@
 #![cfg_attr(not(all(feature = "apple", target_os = "macos")), allow(unused))]
+use objc2::declare::Ivar;
 use objc2::foundation::NSObject;
 use objc2::rc::{Id, Shared};
 use objc2::runtime::Object;
@@ -31,14 +32,22 @@ declare_class!(
 
     unsafe impl CustomAppDelegate {
         #[sel(initWith:another:)]
-        fn init_with(self: &mut Self, ivar: u8, another_ivar: bool) -> *mut Self {
-            let this: *mut Self = unsafe { msg_send![super(self), init] };
-            if let Some(this) = unsafe { this.as_mut() } {
-                // TODO: Allow initialization through MaybeUninit
-                *this.ivar = ivar;
-                *this.another_ivar = another_ivar;
-            }
-            this
+        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] };
+            this.map(|this| {
+                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
+                this
+            })
         }
 
         #[sel(myClassMethod)]

objc2/src/declare/ivar.rs

@@ -4,7 +4,7 @@ use core::mem::MaybeUninit;
 use core::ops::{Deref, DerefMut};
 use core::ptr::NonNull;
 
-use crate::encode::{Encode, EncodeConvert};
+use crate::encode::EncodeConvert;
 use crate::runtime::Object;
 
 /// Helper trait for defining instance variables.
@@ -44,14 +44,6 @@ pub unsafe trait IvarType {
     const NAME: &'static str;
 }
 
-unsafe impl<T: IvarType> IvarType for MaybeUninit<T>
-where
-    T::Type: Encode,
-{
-    type Type = MaybeUninit<T::Type>;
-    const NAME: &'static str = T::NAME;
-}
-
 /// A wrapper type over a custom instance variable.
 ///
 /// This type is not meant to be constructed by itself, it must reside within
@@ -138,7 +130,14 @@ pub struct Ivar<T: IvarType> {
 }
 
 impl<T: IvarType> Ivar<T> {
-    fn get_ref(&self) -> &T::Type {
+    /// Get a pointer to the instance variable.
+    ///
+    /// Note that if the ivar has already been initialized, you can simply
+    /// use the `Deref` implementation to get a reference.
+    ///
+    /// This is similar to [`MaybeUninit::as_ptr`], see that for usage
+    /// instructions.
+    pub fn as_ptr(this: &Self) -> *const T::Type {
         // SAFETY: The user ensures that this is placed in a struct that can
         // be reinterpreted as an `Object`. Since `Ivar` can never be
         // constructed by itself (and is neither Copy nor Clone), we know that
@@ -150,21 +149,27 @@ impl<T: IvarType> Ivar<T> {
         // Note: We technically don't have provenance over the object, nor the
         // ivar, but the object doesn't have provenance over the ivar either,
         // so that is fine.
-        let ptr = NonNull::from(self).cast::<Object>();
+        let ptr = NonNull::from(this).cast::<Object>();
         let obj = unsafe { ptr.as_ref() };
 
         // SAFETY: User ensures that the `Ivar<T>` is only used when the ivar
         // exists and has the correct type
-        unsafe {
-            obj.inner_ivar_ptr::<T::Type>(T::NAME)
-                .as_ref()
-                .unwrap_unchecked()
-        }
+        unsafe { obj.inner_ivar_ptr::<T::Type>(T::NAME) }
     }
 
-    fn get_mut_ptr(&mut self) -> *mut T::Type {
-        let ptr = NonNull::from(self).cast::<Object>();
-        // SAFETY: Same as `get_ref`.
+    /// Get a mutable pointer to the instance variable.
+    ///
+    /// This is useful when you want to initialize the ivar inside an `init`
+    /// method (where it may otherwise not have been safely initialized yet).
+    ///
+    /// Note that if the ivar has already been initialized, you can simply
+    /// use the `DerefMut` implementation to get a mutable reference.
+    ///
+    /// 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::from(this).cast::<Object>();
+        // SAFETY: Same as `as_ptr`.
         //
         // Note: We don't use `mut` because the user might have two mutable
         // references to different ivars, as such:
@@ -185,6 +190,9 @@ impl<T: IvarType> Ivar<T> {
         // And using `mut` would create aliasing mutable reference to the
         // object.
         //
+        // Since `Object` is `UnsafeCell`, so mutable access through `&Object`
+        // is allowed.
+        //
         // TODO: Not entirely sure, it might be safe to just do `as_mut`, but
         // this is definitely safe.
         let obj = unsafe { ptr.as_ref() };
@@ -194,12 +202,17 @@ impl<T: IvarType> Ivar<T> {
         unsafe { obj.inner_ivar_ptr::<T::Type>(T::NAME) }
     }
 
-    #[inline]
-    fn get_mut(&mut self) -> &mut T::Type {
-        // SAFETY: Safe as mutable because there is only one access to a
-        // particular ivar at a time (since we have `&mut self`). `Object` is
-        // `UnsafeCell`, so mutable access through `&Object` is allowed.
-        unsafe { self.get_mut_ptr().as_mut().unwrap_unchecked() }
+    /// Sets the value of the instance variable.
+    ///
+    /// This is useful when you want to initialize the ivar inside an `init`
+    /// method (where it may otherwise not have been safely initialized yet).
+    ///
+    /// 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();
+        let ivar = unsafe { ptr.as_mut().unwrap_unchecked() };
+        ivar.write(val)
     }
 }
 
@@ -208,14 +221,20 @@ impl<T: IvarType> Deref for Ivar<T> {
 
     #[inline]
     fn deref(&self) -> &Self::Target {
-        self.get_ref()
+        // SAFETY: The ivar pointer always points to a valid instance.
+        //
+        // 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() }
     }
 }
 
 impl<T: IvarType> DerefMut for Ivar<T> {
     #[inline]
     fn deref_mut(&mut self) -> &mut Self::Target {
-        self.get_mut()
+        // SAFETY: Safe as mutable because there is only one access to a
+        // particular ivar at a time (since we have `&mut self`).
+        unsafe { Self::as_mut_ptr(self).as_mut().unwrap_unchecked() }
     }
 }