Allow creating and loading instance variables with the same name

Author: madsmtm

Cargo.lock

@@ -85,6 +85,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   implement `IsAllowedMutable`.
 * Disallow the ability to use non-`Self`-like types as the receiver in
   `declare_class!`.
+* Allow adding instance variables with the same name on Apple platforms.
+* **BREAKING**: Make loading instance variables robust and sound in the face
+  of instance variables with the same name.
+
+  To read or write the instance variable for an object, you should now use the
+  `load`, `load_ptr` and `load_mut` methods on `Ivar`, instead of the `ivar`,
+  `ivar_ptr` and `ivar_mut` methods on `AnyObject`.
+
+  This _is_ more verbose, but it also ensures that the class for the instance
+  variable you're loading is the same as the one the instance variable you
+  want to access is defined on.
+
+  ```rust
+  // Before
+  let number = unsafe { *obj.ivar::<u32>("number") };
+
+  // After
+  let ivar = cls.instance_variable("number").unwrap();
+  let number = unsafe { *ivar.load::<u32>(&obj) };
+  ```
 
 ### Removed
 * **BREAKING**: Removed `ProtocolType` implementation for `NSObject`.

crates/objc2/CHANGELOG.md

@@ -108,6 +108,7 @@ objc2-proc-macros = { path = "../objc2-proc-macros", version = "0.1.1", optional
 [dev-dependencies]
 iai = { version = "0.1", git = "https://github.com/madsmtm/iai", branch = "callgrind" }
 static_assertions = "1.1.0"
+memoffset = "0.9.0"
 
 [[bench]]
 name = "autorelease"

crates/objc2/Cargo.toml

@@ -4,36 +4,19 @@
 use std::marker::PhantomData;
 use std::sync::Once;
 
-use objc2::declare::{ClassBuilder, Ivar, IvarEncode, IvarType};
+use objc2::declare::ClassBuilder;
 use objc2::mutability::Mutable;
 use objc2::rc::Id;
 use objc2::runtime::{AnyClass, NSObject, Sel};
 use objc2::{msg_send, msg_send_id, sel};
 use objc2::{ClassType, Encoding, Message, RefEncode};
 
-/// Helper type for the instance variable
-struct NumberIvar<'a> {
-    // Doesn't actually matter what we put here, but we have to use the
-    // lifetime parameter somehow
-    p: PhantomData<&'a mut u8>,
-}
-
-unsafe impl<'a> IvarType for NumberIvar<'a> {
-    type Type = IvarEncode<&'a mut u8>;
-    const NAME: &'static str = "_number_ptr";
-}
-
 /// Struct that represents our custom object.
 #[repr(C)]
 pub struct MyObject<'a> {
     // Required to give MyObject the proper layout
     superclass: NSObject,
-    // SAFETY: The ivar is declared below, and is properly initialized in the
-    // designated initializer.
-    //
-    // Note! Attempting to acess the ivar before it has been initialized is
-    // undefined behaviour!
-    number: Ivar<NumberIvar<'a>>,
+    p: PhantomData<&'a mut u8>,
 }
 
 unsafe impl RefEncode for MyObject<'_> {
@@ -50,8 +33,12 @@ impl<'a> MyObject<'a> {
     ) -> Option<&'s 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"));
+            let ivar = Self::class().instance_variable("number").unwrap();
+            // SAFETY: The ivar is added with the same type below
+            unsafe {
+                ivar.load_ptr::<&mut u8>(&this.superclass)
+                    .write(ptr.expect("got NULL number ptr"))
+            };
             this
         })
     }
@@ -62,12 +49,16 @@ impl<'a> MyObject<'a> {
         unsafe { msg_send_id![Self::alloc(), initWithPtr: number] }
     }
 
-    pub fn get(&self) -> &u8 {
-        &self.number
+    pub fn get(&self) -> u8 {
+        let ivar = Self::class().instance_variable("number").unwrap();
+        // SAFETY: The ivar is added with the same type below, and is initialized in `init_with_ptr`
+        unsafe { **ivar.load::<&mut u8>(&self.superclass) }
     }
 
     pub fn set(&mut self, number: u8) {
-        **self.number = number;
+        let ivar = Self::class().instance_variable("number").unwrap();
+        // SAFETY: The ivar is added with the same type below, and is initialized in `init_with_ptr`
+        unsafe { **ivar.load_mut::<&mut u8>(&mut self.superclass) = number };
     }
 }
 
@@ -84,7 +75,7 @@ unsafe impl<'a> ClassType for MyObject<'a> {
             let superclass = NSObject::class();
             let mut builder = ClassBuilder::new(Self::NAME, superclass).unwrap();
 
-            builder.add_static_ivar::<NumberIvar<'a>>();
+            builder.add_ivar::<&mut u8>("number");
 
             unsafe {
                 builder.add_method(
@@ -112,7 +103,7 @@ fn main() {
     let mut number = 54;
 
     let mut obj = MyObject::new(&mut number);
-    assert_eq!(*obj.get(), 54);
+    assert_eq!(obj.get(), 54);
 
     // It is not possible to convert to `Id<NSObject>`, since that would loose
     // the lifetime information that `MyObject` stores.
@@ -131,7 +122,7 @@ fn main() {
 
     // But we can now mutate the referenced `number`
     obj.set(7);
-    assert_eq!(*obj.get(), 7);
+    assert_eq!(obj.get(), 7);
 
     drop(obj);
     // And now that we've dropped `obj`, we can access `number` again

crates/objc2/examples/class_with_lifetime.rs

@@ -49,9 +49,7 @@ fn main() {
 
     println!("NSObject address: {obj:p}");
 
-    // Access an ivar of the object
-    //
-    // As before, you should not rely on the `isa` ivar being available!
-    let isa = unsafe { *obj.ivar::<*const AnyClass>("isa") };
+    // Read an ivar on the object
+    let isa: *const AnyClass = unsafe { *ivar.load(&obj) };
     println!("NSObject isa: {isa:?}");
 }

crates/objc2/examples/introspection.rs

@@ -5,7 +5,7 @@ use core::ops::{Deref, DerefMut};
 use core::ptr::{self, NonNull};
 
 use crate::encode::Encode;
-use crate::runtime::{ivar_offset, AnyObject};
+use crate::runtime::AnyObject;
 
 pub(crate) mod private {
     pub trait Sealed {}
@@ -92,9 +92,13 @@ pub unsafe trait IvarType {
     const NAME: &'static str;
 
     #[doc(hidden)]
-    unsafe fn __offset(ptr: NonNull<AnyObject>) -> isize {
-        let obj = unsafe { ptr.as_ref() };
-        ivar_offset(obj.class(), Self::NAME, &Self::Type::ENCODING)
+    unsafe fn __ivar_ptr(ptr: NonNull<AnyObject>) -> NonNull<Self::Type> {
+        // FIXME: This is currently unsound! Looking up the instance variable
+        // dynamically will return the wrong variable if two variables with
+        // the same name exist.
+        let ivar = unsafe { ptr.as_ref() }.lookup_instance_variable_dynamically(Self::NAME);
+        ivar.debug_assert_encoding(&Self::Type::ENCODING);
+        unsafe { AnyObject::ivar_at_offset(ptr, ivar.offset()) }
     }
 }
 
@@ -215,9 +219,7 @@ 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 offset = unsafe { T::__offset(ptr) };
-        // SAFETY: The offset is valid
-        unsafe { AnyObject::ivar_at_offset::<T::Type>(ptr, offset) }
+        unsafe { T::__ivar_ptr(ptr) }
     }
 
     /// Get a mutable pointer to the instance variable.
@@ -238,9 +240,7 @@ impl<T: IvarType> Ivar<T> {
         let ptr: NonNull<AnyObject> = NonNull::from(self).cast();
 
         // SAFETY: Same as `as_inner_ptr`
-        let offset = unsafe { T::__offset(ptr) };
-        // SAFETY: The offset is valid
-        unsafe { AnyObject::ivar_at_offset::<T::Type>(ptr, offset) }
+        unsafe { T::__ivar_ptr(ptr) }
     }
 
     /// Sets the value of the instance variable.