From 1ade9a898b8cd3c21feadf60d9d8a04dae1855c9 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 29 Aug 2022 16:21:56 +0200 Subject: [PATCH] Better ivar initialization --- objc2/CHANGELOG.md | 8 +++ objc2/examples/class_with_lifetime.rs | 44 +++++----------- objc2/examples/delegate.rs | 25 ++++++--- objc2/src/declare/ivar.rs | 73 +++++++++++++++++---------- 4 files changed, 85 insertions(+), 65 deletions(-) diff --git a/objc2/CHANGELOG.md b/objc2/CHANGELOG.md index db8763569..a52ebdb66 100644 --- a/objc2/CHANGELOG.md +++ b/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 diff --git a/objc2/examples/class_with_lifetime.rs b/objc2/examples/class_with_lifetime.rs index fcbef573f..dd3c18d6c 100644 --- a/objc2/examples/class_with_lifetime.rs +++ b/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 { // 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::>(); - /// 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>>, - } - 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(_, _, _) -> _, ); } diff --git a/objc2/examples/delegate.rs b/objc2/examples/delegate.rs index a0dc2c88c..696f31636 100644 --- a/objc2/examples/delegate.rs +++ b/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)] diff --git a/objc2/src/declare/ivar.rs b/objc2/src/declare/ivar.rs index 783c9989e..e1c2c0c99 100644 --- a/objc2/src/declare/ivar.rs +++ b/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 IvarType for MaybeUninit -where - T::Type: Encode, -{ - type Type = MaybeUninit; - 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 { } impl Ivar { - 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 Ivar { // 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::(); + let ptr = NonNull::from(this).cast::(); let obj = unsafe { ptr.as_ref() }; // SAFETY: User ensures that the `Ivar` is only used when the ivar // exists and has the correct type - unsafe { - obj.inner_ivar_ptr::(T::NAME) - .as_ref() - .unwrap_unchecked() - } + unsafe { obj.inner_ivar_ptr::(T::NAME) } } - fn get_mut_ptr(&mut self) -> *mut T::Type { - let ptr = NonNull::from(self).cast::(); - // 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::(); + // 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 Ivar { // 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 Ivar { unsafe { obj.inner_ivar_ptr::(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 = Self::as_mut_ptr(this).cast(); + let ivar = unsafe { ptr.as_mut().unwrap_unchecked() }; + ivar.write(val) } } @@ -208,14 +221,20 @@ impl Deref for Ivar { #[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 DerefMut for Ivar { #[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() } } }