Merge pull request #150 from madsmtm/msg-send-receiver-soundness

Change how mutability in `msg_send!` is done

Author: madsmtm

objc2-foundation/examples/custom_class.rs

@@ -70,12 +70,12 @@ fn main() {
 
     obj.set_number(7);
     println!("Number: {}", unsafe {
-        let number: u32 = msg_send![obj, number];
+        let number: u32 = msg_send![&obj, number];
         number
     });
 
     unsafe {
-        let _: () = msg_send![obj, setNumber: 12u32];
+        let _: () = msg_send![&mut obj, setNumber: 12u32];
     }
     println!("Number: {}", obj.number());
 }

objc2-foundation/src/array.rs

@@ -272,15 +272,18 @@ impl<T: Message, O: Ownership> NSMutableArray<T, O> {
         obj
     }
 
+    fn remove_last(&mut self) {
+        unsafe { msg_send![self, removeLastObject] }
+    }
+
     #[doc(alias = "removeLastObject")]
     pub fn pop(&mut self) -> Option<Id<T, O>> {
-        self.last().map(|obj| {
-            let obj = unsafe { Id::retain(obj as *const T as *mut T).unwrap_unchecked() };
-            unsafe {
-                let _: () = msg_send![self, removeLastObject];
-            }
-            obj
-        })
+        self.last()
+            .map(|obj| unsafe { Id::retain(obj as *const T as *mut T).unwrap_unchecked() })
+            .map(|obj| {
+                self.remove_last();
+                obj
+            })
     }
 
     #[doc(alias = "removeAllObjects")]

objc2-foundation/src/attributed_string.rs

@@ -153,11 +153,14 @@ mod tests {
         let s2 = s1.copy();
         // NSAttributedString performs this optimization in GNUStep's runtime,
         // but not in Apple's; so we don't test for it!
-        // assert_eq!(s1.as_ptr(), s2.as_ptr());
+        // assert_eq!(Id::as_ptr(&s1), Id::as_ptr(&s2));
         assert!(s2.is_kind_of(NSAttributedString::class()));
 
         let s3 = s1.mutable_copy();
-        assert_ne!(s1.as_ptr(), s3.as_ptr() as *mut NSAttributedString);
+        assert_ne!(
+            Id::as_ptr(&s1),
+            Id::as_ptr(&s3) as *const NSAttributedString
+        );
         assert!(s3.is_kind_of(NSMutableAttributedString::class()));
     }
 }

objc2-foundation/src/data.rs

@@ -154,9 +154,14 @@ impl NSMutableData {
 
 /// Mutation methods
 impl NSMutableData {
+    // Helps with reborrowing issue
+    fn raw_bytes_mut(&mut self) -> *mut c_void {
+        unsafe { msg_send![self, mutableBytes] }
+    }
+
     #[doc(alias = "mutableBytes")]
     pub fn bytes_mut(&mut self) -> &mut [u8] {
-        let ptr: *mut c_void = unsafe { msg_send![self, mutableBytes] };
+        let ptr = self.raw_bytes_mut();
         // The bytes pointer may be null for length zero
         if ptr.is_null() {
             &mut []

objc2-foundation/src/dictionary.rs

@@ -129,7 +129,7 @@ impl<K: Message, V: Message> NSDictionary<K, V> {
 
     pub fn into_values_array(dict: Id<Self, Owned>) -> Id<NSArray<V, Owned>, Shared> {
         unsafe {
-            let vals = msg_send![dict, allValues];
+            let vals = msg_send![&dict, allValues];
             Id::retain_autoreleased(vals).unwrap()
         }
     }

objc2-foundation/src/enumerator.rs

@@ -33,7 +33,7 @@ impl<'a, T: Message> Iterator for NSEnumerator<'a, T> {
     type Item = &'a T;
 
     fn next(&mut self) -> Option<&'a T> {
-        unsafe { msg_send![self.id, nextObject] }
+        unsafe { msg_send![&mut self.id, nextObject] }
     }
 }
 

objc2-foundation/src/mutable_attributed_string.rs

@@ -90,11 +90,14 @@ mod tests {
     fn test_copy() {
         let s1 = NSMutableAttributedString::from_nsstring(&NSString::from_str("abc"));
         let s2 = s1.copy();
-        assert_ne!(s1.as_ptr() as *const NSAttributedString, s2.as_ptr());
+        assert_ne!(
+            Id::as_ptr(&s1) as *const NSAttributedString,
+            Id::as_ptr(&s2)
+        );
         assert!(s2.is_kind_of(NSAttributedString::class()));
 
         let s3 = s1.mutable_copy();
-        assert_ne!(s1.as_ptr(), s3.as_ptr());
+        assert_ne!(Id::as_ptr(&s1), Id::as_ptr(&s3));
         assert!(s3.is_kind_of(NSMutableAttributedString::class()));
     }
 }

objc2-foundation/src/mutable_string.rs

@@ -200,11 +200,11 @@ mod tests {
     fn test_copy() {
         let s1 = NSMutableString::from_str("abc");
         let s2 = s1.copy();
-        assert_ne!(s1.as_ptr(), s2.as_ptr() as *mut NSMutableString);
+        assert_ne!(Id::as_ptr(&s1), Id::as_ptr(&s2) as *const NSMutableString);
         assert!(s2.is_kind_of(NSString::class()));
 
         let s3 = s1.mutable_copy();
-        assert_ne!(s1.as_ptr(), s3.as_ptr());
+        assert_ne!(Id::as_ptr(&s1), Id::as_ptr(&s3));
         assert!(s3.is_kind_of(NSMutableString::class()));
     }
 }

objc2-foundation/src/string.rs

@@ -346,11 +346,11 @@ mod tests {
         let s1 = NSString::from_str("abc");
         let s2 = s1.copy();
         // An optimization that NSString makes, since it is immutable
-        assert_eq!(s1.as_ptr(), s2.as_ptr());
+        assert_eq!(Id::as_ptr(&s1), Id::as_ptr(&s2));
         assert!(s2.is_kind_of(NSString::class()));
 
         let s3 = s1.mutable_copy();
-        assert_ne!(s1.as_ptr(), s3.as_ptr() as *mut NSString);
+        assert_ne!(Id::as_ptr(&s1), Id::as_ptr(&s3) as *const NSString);
         assert!(s3.is_kind_of(NSMutableString::class()));
     }
 

objc2-foundation/src/value.rs

@@ -177,7 +177,7 @@ mod tests {
     fn test_value_nsrange() {
         let val = NSValue::new(NSRange::from(1..2));
         assert!(NSRange::ENCODING.equivalent_to_str(val.encoding().unwrap()));
-        let range: NSRange = unsafe { objc2::msg_send![val, rangeValue] };
+        let range: NSRange = unsafe { objc2::msg_send![&val, rangeValue] };
         assert_eq!(range, NSRange::from(1..2));
         // NSValue -getValue is broken on GNUStep for some types
         #[cfg(not(feature = "gnustep-1-7"))]

objc2/CHANGELOG.md

@@ -11,7 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   upgrading easier.
 * Allow using `From`/`TryFrom` to convert between `rc::Id` and `rc::WeakId`.
 * Added `Bool::as_bool` (more descriptive name than `Bool::is_true`).
-* Added convenience method `Id::as_ptr`.
+* Added convenience method `Id::as_ptr` and `Id::as_mut_ptr`.
 * The `objc2-encode` dependency is now exposed as `objc2::encode`.
 * Added `Id::retain_autoreleased` to allow following Cocoas memory management
   rules more efficiently.
@@ -38,6 +38,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   `ClassBuilder::add_method`.
 * Renamed `ClassDecl` and `ProtocolDecl` to `ClassBuilder` and
   `ProtocolBuilder`. The old names are kept as deprecated aliases.
+* **BREAKING**: Changed how `msg_send!` works wrt. capturing its arguments.
+
+  This will require changes to your code wherever you used `Id`, for example:
+  ```rust
+  // Before
+  let obj: Id<Object, Owned> = ...;
+  let p: i32 = unsafe { msg_send![obj, parameter] };
+  let _: () = unsafe { msg_send![obj, setParameter: p + 1] };
+  // After
+  let mut obj: Id<Object, Owned> = ...;
+  let p: i32 = unsafe { msg_send![&obj, parameter] };
+  let _: () = unsafe { msg_send![&mut obj, setParameter: p + 1] };
+  ```
+
+  Notice that we now clearly pass `obj` by reference, and therein also
+  communicate the mutability of the object (in the first case, immutable, and
+  in the second, mutable).
+
+  If you previously used `*mut Object` or `&Object` as the receiver, message
+  sending should work exactly as before.
 
 ### Fixed
 * Properly sealed the `MessageArguments` trait (it already had a hidden

objc2/benches/autorelease.rs

@@ -61,11 +61,11 @@ fn new_nsdata() -> Id<Object, Shared> {
     unsafe { Id::new(obj).unwrap_unchecked() }
 }
 
-fn new_leaked_nsdata() -> *mut Object {
-    ManuallyDrop::new(new_nsdata()).as_ptr()
+fn new_leaked_nsdata() -> *const Object {
+    Id::as_ptr(&*ManuallyDrop::new(new_nsdata()))
 }
 
-fn autoreleased_nsdata() -> *mut Object {
+fn autoreleased_nsdata() -> *const Object {
     // let bytes_ptr = BYTES.as_ptr() as *const c_void;
     // unsafe {
     //     msg_send![
@@ -83,20 +83,20 @@ fn new_nsstring() -> Id<Object, Shared> {
     unsafe { Id::new(obj).unwrap_unchecked() }
 }
 
-fn new_leaked_nsstring() -> *mut Object {
-    ManuallyDrop::new(new_nsstring()).as_ptr()
+fn new_leaked_nsstring() -> *const Object {
+    Id::as_ptr(&*ManuallyDrop::new(new_nsstring()))
 }
 
-fn autoreleased_nsstring() -> *mut Object {
+fn autoreleased_nsstring() -> *const Object {
     // unsafe { msg_send![class!(NSString), string] }
     unsafe { msg_send![new_leaked_nsstring(), autorelease] }
 }
 
-fn retain_autoreleased(obj: *mut Object) -> Id<Object, Shared> {
-    unsafe { Id::retain_autoreleased(obj.cast()).unwrap_unchecked() }
+fn retain_autoreleased(obj: *const Object) -> Id<Object, Shared> {
+    unsafe { Id::retain_autoreleased((obj as *mut Object).cast()).unwrap_unchecked() }
 }
 
-fn autoreleased_nsdata_pool_cleanup() -> *mut Object {
+fn autoreleased_nsdata_pool_cleanup() -> *const Object {
     autoreleasepool(|_| autoreleased_nsdata())
 }
 
@@ -108,7 +108,7 @@ fn autoreleased_nsdata_fast_caller_cleanup_pool_cleanup() -> Id<Object, Shared>
     autoreleasepool(|_| retain_autoreleased(autoreleased_nsdata()))
 }
 
-fn autoreleased_nsstring_pool_cleanup() -> *mut Object {
+fn autoreleased_nsstring_pool_cleanup() -> *const Object {
     autoreleasepool(|_| autoreleased_nsstring())
 }
 

objc2/examples/introspection.rs

@@ -42,6 +42,6 @@ fn main() {
     }
 
     // Invoke a method on the object
-    let hash: usize = unsafe { msg_send![obj, hash] };
+    let hash: usize = unsafe { msg_send![&obj, hash] };
     println!("NSObject hash: {}", hash);
 }

objc2/examples/talk_to_me.rs

@@ -31,9 +31,9 @@ fn main() {
     let utterance: *mut Object = unsafe { msg_send![utterance, initWithString: &*string] };
     let utterance: Id<Object, Owned> = unsafe { Id::new(utterance).unwrap() };
 
-    // let _: () = unsafe { msg_send![utterance, setVolume: 90.0f32 };
-    // let _: () = unsafe { msg_send![utterance, setRate: 0.50f32 };
-    // let _: () = unsafe { msg_send![utterance, setPitchMultiplier: 0.80f32 };
+    // let _: () = unsafe { msg_send![&utterance, setVolume: 90.0f32 };
+    // let _: () = unsafe { msg_send![&utterance, setRate: 0.50f32 };
+    // let _: () = unsafe { msg_send![&utterance, setPitchMultiplier: 0.80f32 };
 
-    let _: () = unsafe { msg_send![synthesizer, speakUtterance: &*utterance] };
+    let _: () = unsafe { msg_send![&synthesizer, speakUtterance: &*utterance] };
 }

objc2/src/declare.rs

@@ -415,9 +415,9 @@ mod tests {
     #[test]
     fn test_custom_class() {
         // Registering the custom class is in test_utils
-        let obj = test_utils::custom_object();
-        let _: () = unsafe { msg_send![obj, setFoo: 13u32] };
-        let result: u32 = unsafe { msg_send![obj, foo] };
+        let mut obj = test_utils::custom_object();
+        let _: () = unsafe { msg_send![&mut obj, setFoo: 13u32] };
+        let result: u32 = unsafe { msg_send![&obj, foo] };
         assert_eq!(result, 13);
     }
 

objc2/src/lib.rs

@@ -42,8 +42,8 @@
 //! };
 //!
 //! // Usage
-//! let hash: NSUInteger = unsafe { msg_send![obj, hash] };
-//! let is_kind = unsafe { msg_send_bool![obj, isKindOfClass: cls] };
+//! let hash: NSUInteger = unsafe { msg_send![&obj, hash] };
+//! let is_kind = unsafe { msg_send_bool![&obj, isKindOfClass: cls] };
 //! assert!(is_kind);
 //! ```
 //!