Skip to content

Better ivar initialization #252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions objc2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 14 additions & 30 deletions objc2/examples/class_with_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(_, _, _) -> _,
);
}

Expand Down
25 changes: 17 additions & 8 deletions objc2/examples/delegate.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)]
Expand Down
73 changes: 46 additions & 27 deletions objc2/src/declare/ivar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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() };
Expand All @@ -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)
}
}

Expand All @@ -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() }
}
}

Expand Down