Change ivar documentation to match that Object contains UnsafeCell

Author: madsmtm

objc2-foundation/examples/class_with_lifetime.rs

@@ -34,7 +34,7 @@ impl<'a> MyObject<'a> {
     }
 
     fn get(&self) -> Option<&'a u8> {
-        unsafe { *self.inner.ivar("_number_ptr") }
+        unsafe { *self.inner.ivar::<Option<&'a u8>>("_number_ptr") }
     }
 
     fn write(&mut self, number: u8) {
@@ -57,9 +57,7 @@ impl<'a> MyObject<'a> {
             ) -> *mut Object {
                 let this: *mut Object = unsafe { msg_send![super(this, NSObject::class()), init] };
                 if let Some(this) = unsafe { this.as_mut() } {
-                    unsafe {
-                        this.set_ivar("_number_ptr", ptr);
-                    }
+                    unsafe { this.set_ivar::<*mut u8>("_number_ptr", ptr) };
                 }
                 this
             }

objc2/CHANGELOG.md

@@ -31,6 +31,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * Added `"unstable-c-unwind"` feature.
 * Added unsafe function `Id::cast` for converting between different types of
   objects.
+* Added `Object::ivar_ptr` to allow direct access to instance variables
+  through `&Object`.
 
 ### Changed
 * **BREAKING:** `Sel` is now required to be non-null, which means that you

objc2/examples/introspection.rs

@@ -26,8 +26,10 @@ fn main() {
     println!("NSObject address: {:p}", obj);
 
     // Access an ivar of the object
-    // TODO: Fix this!
-    let isa: *const Class = unsafe { *obj.ivar("isa") };
+    //
+    // Note: You should not rely on the `isa` ivar being available,
+    // this is only for demonstration.
+    let isa = unsafe { *obj.ivar::<*const Class>("isa") };
     println!("NSObject isa: {:?}", isa);
 
     #[cfg(feature = "malloc")]

objc2/src/declare.rs

@@ -22,7 +22,7 @@
 //!
 //! // Add an ObjC method for getting the number
 //! extern "C" fn my_number_get(this: &Object, _cmd: Sel) -> u32 {
-//!     unsafe { *this.ivar("_number") }
+//!     unsafe { *this.ivar::<u32>("_number") }
 //! }
 //! unsafe {
 //!     decl.add_method(

objc2/src/runtime.rs

@@ -625,19 +625,45 @@ impl Object {
         unsafe { ptr.as_ref().unwrap_unchecked() }
     }
 
-    /// Returns a shared reference to the ivar with the given name.
+    /// Returns a pointer to the instance variable / ivar with the given name.
+    ///
+    /// This is similar to [`UnsafeCell::get`], see that for more information
+    /// on what is and isn't safe to do.
+    ///
+    /// Usually you will have defined the instance variable yourself with
+    /// [`ClassBuilder::add_ivar`], the type of the ivar `T` must match the
+    /// type used in that.
+    ///
+    /// Attempting to access or modify private implementation details of a
+    /// class that you do no control using this is not supported, and may
+    /// invoke undefined behaviour.
+    ///
+    /// Library implementors are strongly encouraged to expose a safe
+    /// interface to the ivar.
+    ///
+    /// [`ClassBuilder::add_ivar`]: crate::declare::ClassBuilder::add_ivar
+    ///
     ///
     /// # Panics
     ///
-    /// Panics if the object has no ivar with the given name, or the type
-    /// encoding of the ivar differs from the type encoding of `T`.
+    /// May panic if the object has no ivar with the given name. May also
+    /// panic if the type encoding of the ivar differs from the type encoding
+    /// of `T`.
+    ///
+    /// This should purely seen as help while debugging and is not guaranteed
+    /// (e.g. it may be disabled when `debug_assertions` are off).
+    ///
     ///
     /// # Safety
     ///
-    /// The caller must ensure that the ivar is actually of type `T`.
+    /// The object must have an instance variable with the given name, and it
+    /// must be of type `T`. Any invariants that the object have assumed about
+    /// the value of the instance variable must not be violated.
     ///
-    /// Library implementors should expose a safe interface to the ivar.
-    pub unsafe fn ivar<T: Encode>(&self, name: &str) -> &T {
+    /// No thread syncronization is done on accesses to the variable, so you
+    /// must ensure that any access to the returned pointer do not cause data
+    /// races, and that Rust's mutability rules are not otherwise violated.
+    pub unsafe fn ivar_ptr<T: Encode>(&self, name: &str) -> *mut T {
         let offset = ivar_offset::<T>(self.class(), name);
         let ptr: *const Self = self;
 
@@ -646,32 +672,56 @@ impl Object {
         let ptr = unsafe { ptr.offset(offset) };
         let ptr: *const T = ptr.cast();
 
-        unsafe { ptr.as_ref().unwrap_unchecked() }
+        // Safe as *mut T because `self` is `UnsafeCell`
+        ptr as *mut T
+    }
+
+    /// Returns a reference to the instance variable with the given name.
+    ///
+    /// See [`Object::ivar_ptr`] for more information, including on when this
+    /// panics.
+    ///
+    ///
+    /// # Safety
+    ///
+    /// The object must have an instance variable with the given name, and it
+    /// must be of type `T`.
+    ///
+    /// No thread syncronization is done, so you must ensure that no other
+    /// thread is concurrently mutating the variable. This requirement can be
+    /// considered upheld if all mutation happens through [`Object::ivar_mut`]
+    /// (since that  takes `&mut self`).
+    pub unsafe fn ivar<T: Encode>(&self, name: &str) -> &T {
+        // SAFETY: Upheld by caller.
+        unsafe { self.ivar_ptr::<T>(name).as_ref().unwrap_unchecked() }
     }
 
-    /// Use [`ivar`][`Self::ivar`] instead.
+    /// Use [`Object::ivar`] instead.
+    ///
     ///
     /// # Safety
     ///
-    /// Same as [`ivar`][`Self::ivar`].
+    /// See [`Object::ivar`].
     #[deprecated = "Use `Object::ivar` instead."]
     pub unsafe fn get_ivar<T: Encode>(&self, name: &str) -> &T {
         // SAFETY: Upheld by caller
-        unsafe { self.ivar(name) }
+        unsafe { self.ivar::<T>(name) }
     }
 
     /// Returns a mutable reference to the ivar with the given name.
     ///
-    /// # Panics
+    /// See [`Object::ivar_ptr`] for more information, including on when this
+    /// panics.
     ///
-    /// Panics if the object has no ivar with the given name, or the type
-    /// encoding of the ivar differs from the type encoding of `T`.
     ///
     /// # Safety
     ///
-    /// The caller must ensure that the ivar is actually of type `T`.
+    /// The object must have an instance variable with the given name, and it
+    /// must be of type `T`.
     ///
-    /// Library implementors should expose a safe interface to the ivar.
+    /// This access happens through `&mut self`, which means we know it to be
+    /// the only reference, hence you do not need to do any work to ensure
+    /// that data races do not happen.
     pub unsafe fn ivar_mut<T: Encode>(&mut self, name: &str) -> &mut T {
         let offset = ivar_offset::<T>(self.class(), name);
         let ptr: *mut Self = self;
@@ -684,29 +734,27 @@ impl Object {
         unsafe { ptr.as_mut().unwrap_unchecked() }
     }
 
-    /// Use [`ivar_mut`](`Self::ivar_mut`) instead.
+    /// Use [`Object::ivar_mut`] instead.
+    ///
     ///
     /// # Safety
     ///
-    /// Same as [`ivar_mut`][`Self::ivar_mut`].
+    /// Same as [`Object::ivar_mut`].
     #[deprecated = "Use `Object::ivar_mut` instead."]
     pub unsafe fn get_mut_ivar<T: Encode>(&mut self, name: &str) -> &mut T {
         // SAFETY: Upheld by caller
-        unsafe { self.ivar_mut(name) }
+        unsafe { self.ivar_mut::<T>(name) }
     }
 
     /// Sets the value of the ivar with the given name.
     ///
-    /// # Panics
+    /// This is just a helpful shorthand for [`Object::ivar_mut`], see that
+    /// for more information.
     ///
-    /// Panics if the object has no ivar with the given name, or the type
-    /// encoding of the ivar differs from the type encoding of `T`.
     ///
     /// # Safety
     ///
-    /// The caller must ensure that the ivar is actually of type `T`.
-    ///
-    /// Library implementors should expose a safe interface to the ivar.
+    /// Same as [`Object::ivar_mut`].
     pub unsafe fn set_ivar<T: Encode>(&mut self, name: &str, value: T) {
         // SAFETY: Invariants upheld by caller
         unsafe { *self.ivar_mut::<T>(name) = value };
@@ -858,13 +906,28 @@ mod tests {
     fn test_object() {
         let mut obj = test_utils::custom_object();
         assert_eq!(obj.class(), test_utils::custom_class());
-        let result: u32 = unsafe {
-            obj.set_ivar("_foo", 4u32);
-            *obj.ivar("_foo")
+
+        let result = unsafe {
+            obj.set_ivar::<u32>("_foo", 4);
+            *obj.ivar::<u32>("_foo")
         };
         assert_eq!(result, 4);
     }
 
+    #[test]
+    #[should_panic = "Ivar unknown not found on class CustomObject"]
+    fn test_object_ivar_unknown() {
+        let obj = test_utils::custom_object();
+        let _ = unsafe { *obj.ivar::<u32>("unknown") };
+    }
+
+    #[test]
+    #[should_panic = "assertion failed: T::ENCODING.equivalent_to_str(ivar.type_encoding())"]
+    fn test_object_ivar_wrong_type() {
+        let obj = test_utils::custom_object();
+        let _ = unsafe { *obj.ivar::<u8>("_foo") };
+    }
+
     #[test]
     fn test_encode() {
         fn assert_enc<T: Encode>(expected: &str) {

objc2/src/test_utils.rs

@@ -108,9 +108,7 @@ pub(crate) fn custom_class() -> &'static Class {
         }
 
         extern "C" fn custom_obj_set_foo(this: &mut Object, _cmd: Sel, foo: u32) {
-            unsafe {
-                this.set_ivar::<u32>("_foo", foo);
-            }
+            unsafe { this.set_ivar::<u32>("_foo", foo) }
         }
 
         extern "C" fn custom_obj_get_foo(this: &Object, _cmd: Sel) -> u32 {