From 36912b071ee08674b61f520de8413d1291f53c81 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 6 Jun 2022 17:33:04 +0200 Subject: [PATCH 1/7] Change msg_send! such that callers can properly communicate mutability This fixes a long-standing soundness issue with how message sending is done whilst mutating the receiver, see: https://github.com/SSheldon/rust-objc/issues/112. We were effectively mutating behind either `&&mut T` or `&T`, where `T` is zero-sized and contains `UnsafeCell`, so while it is still uncertain exactly how much of an issue this actually is, the approach we use now is definitely sound! Also makes it clearer that `msg_send!` does not consume `Id`s, it only needs a reference to those. --- objc2-foundation/examples/custom_class.rs | 4 +- objc2-foundation/src/array.rs | 15 ++-- objc2-foundation/src/data.rs | 3 +- objc2-foundation/src/dictionary.rs | 2 +- objc2-foundation/src/enumerator.rs | 2 +- objc2-foundation/src/value.rs | 2 +- objc2/CHANGELOG.md | 20 +++++ objc2/examples/introspection.rs | 2 +- objc2/examples/talk_to_me.rs | 8 +- objc2/src/declare.rs | 6 +- objc2/src/lib.rs | 4 +- objc2/src/macros.rs | 81 +++++++++++------ objc2/src/message/mod.rs | 89 +++++++++++-------- objc2/src/test_utils.rs | 33 ++++++- tests/assembly/test_msg_send_zero_cost/lib.rs | 2 +- .../assembly/test_retain_autoreleased/lib.rs | 2 +- tests/ui/msg_send_only_message.stderr | 6 +- 17 files changed, 186 insertions(+), 95 deletions(-) diff --git a/objc2-foundation/examples/custom_class.rs b/objc2-foundation/examples/custom_class.rs index 2029ec105..eaede8547 100644 --- a/objc2-foundation/examples/custom_class.rs +++ b/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()); } diff --git a/objc2-foundation/src/array.rs b/objc2-foundation/src/array.rs index e9d31187a..5f79e3f6a 100644 --- a/objc2-foundation/src/array.rs +++ b/objc2-foundation/src/array.rs @@ -274,13 +274,14 @@ impl NSMutableArray { #[doc(alias = "removeLastObject")] pub fn pop(&mut self) -> Option> { - 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| { + unsafe { + let _: () = msg_send![self, removeLastObject]; + } + obj + }) } #[doc(alias = "removeAllObjects")] diff --git a/objc2-foundation/src/data.rs b/objc2-foundation/src/data.rs index f072ac87b..c3829a800 100644 --- a/objc2-foundation/src/data.rs +++ b/objc2-foundation/src/data.rs @@ -156,7 +156,8 @@ impl NSMutableData { impl NSMutableData { #[doc(alias = "mutableBytes")] pub fn bytes_mut(&mut self) -> &mut [u8] { - let ptr: *mut c_void = unsafe { msg_send![self, mutableBytes] }; + let this = &mut *self; // Reborrow + let ptr: *mut c_void = unsafe { msg_send![this, mutableBytes] }; // The bytes pointer may be null for length zero if ptr.is_null() { &mut [] diff --git a/objc2-foundation/src/dictionary.rs b/objc2-foundation/src/dictionary.rs index 072b3190c..69fde1907 100644 --- a/objc2-foundation/src/dictionary.rs +++ b/objc2-foundation/src/dictionary.rs @@ -129,7 +129,7 @@ impl NSDictionary { pub fn into_values_array(dict: Id) -> Id, Shared> { unsafe { - let vals = msg_send![dict, allValues]; + let vals = msg_send![&dict, allValues]; Id::retain_autoreleased(vals).unwrap() } } diff --git a/objc2-foundation/src/enumerator.rs b/objc2-foundation/src/enumerator.rs index 305f4c094..35f2b3776 100644 --- a/objc2-foundation/src/enumerator.rs +++ b/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] } } } diff --git a/objc2-foundation/src/value.rs b/objc2-foundation/src/value.rs index 206a5a2ca..2a5c7da6f 100644 --- a/objc2-foundation/src/value.rs +++ b/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"))] diff --git a/objc2/CHANGELOG.md b/objc2/CHANGELOG.md index 1ac1ca7ad..a01897653 100644 --- a/objc2/CHANGELOG.md +++ b/objc2/CHANGELOG.md @@ -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 = ...; + let p: i32 = unsafe { msg_send![obj, parameter] }; + let _: () = unsafe { msg_send![obj, setParameter: p + 1] }; + // After + let mut obj: Id = ...; + 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 diff --git a/objc2/examples/introspection.rs b/objc2/examples/introspection.rs index 51d7a31d0..cfe126b15 100644 --- a/objc2/examples/introspection.rs +++ b/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); } diff --git a/objc2/examples/talk_to_me.rs b/objc2/examples/talk_to_me.rs index ba1418256..1ab776884 100644 --- a/objc2/examples/talk_to_me.rs +++ b/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 = 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] }; } diff --git a/objc2/src/declare.rs b/objc2/src/declare.rs index c1e22184e..49aa62fea 100644 --- a/objc2/src/declare.rs +++ b/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); } diff --git a/objc2/src/lib.rs b/objc2/src/lib.rs index 577524d13..83f654b2e 100644 --- a/objc2/src/lib.rs +++ b/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); //! ``` //! diff --git a/objc2/src/macros.rs b/objc2/src/macros.rs index 6d9ea7356..227cfb4b2 100644 --- a/objc2/src/macros.rs +++ b/objc2/src/macros.rs @@ -54,22 +54,32 @@ macro_rules! sel { /// Sends a message to an object or class. /// -/// The first argument can be any type that implements [`MessageReceiver`], -/// like a reference, a pointer, or an [`rc::Id`] to an object (where the -/// object implements [`Message`]). -/// -/// In general this is wildly `unsafe`, even more so than sending messages in +/// This is wildly `unsafe`, even more so than sending messages in /// Objective-C, because this macro doesn't know the expected types and /// because Rust has more safety invariants to uphold. Make sure to review the -/// safety section below. +/// safety section below! +/// +/// # General information +/// +/// The syntax is similar to the message syntax in Objective-C, except we +/// allow an optional comma between arguments (works better with rustfmt). /// -/// The syntax is similar to the message syntax in Objective-C. +/// The first argument (know as the "receiver") can be any type that +/// implements [`MessageReceiver`], like a reference or a pointer to an +/// object, or even a reference to an [`rc::Id`] containing an object. +/// Each subsequent argument must implement [`Encode`]. +/// +/// Behind the scenes this translates into a call to [`sel!`], and afterwards +/// a fully qualified call to [`MessageReceiver::send_message`] (note that +/// this means that auto-dereferencing of the receiver is not supported, +/// making the ergonomics when using this slightly worse). /// /// Variadic arguments are not currently supported. /// /// [`MessageReceiver`]: crate::MessageReceiver -/// [`Message`]: crate::Message /// [`rc::Id`]: crate::rc::Id +/// [`Encode`]: crate::Encode +/// [`MessageReceiver::send_message`]: crate::MessageReceiver::send_message /// /// # Panics /// @@ -83,13 +93,12 @@ macro_rules! sel { /// /// # Safety /// -/// The user must ensure that the selector is a valid method and is available -/// on the given receiver. +/// This macro can't inspect header files to see the expected types, so it is +/// your responsibility that the selector exists on the receiver, and that the +/// argument types and return type are what the receiver excepts for this +/// selector - similar to defining an external function in FFI. /// -/// Since this macro can't inspect header files to see the expected types, it -/// is the users responsibility that the argument types and return type are -/// what the receiver excepts for this selector. A way of doing this is by -/// defining a wrapper function: +/// The recommended way of doing this is by defining a wrapper function: /// ``` /// # use std::os::raw::{c_int, c_char}; /// # use objc2::msg_send; @@ -99,19 +108,35 @@ macro_rules! sel { /// } /// ``` /// -/// The user must also uphold any safety requirements (explicit and implicit) -/// that the method has (e.g. methods that take pointers as an argument -/// usually require that the pointer is valid and often non-null). +/// This way we are clearly communicating to Rust that this method takes an +/// immutable object, a C-integer, and returns a pointer to (probably) a +/// C-compatible string. Afterwards, it becomes fairly trivial to make a safe +/// abstraction around this. +/// +/// In particular, you must uphold the following requirements: +/// +/// 1. The selector is a valid method that is available on the given receiver. +/// +/// 2. The types of the receiver and arguments must match what is expected on +/// the Objective-C side. +/// +/// 3. The call must not violate Rust's mutability rules, e.g. if passing an +/// `&T`, the Objective-C method must not mutate the variable (this is true +/// for receivers as well). +/// +/// 4. If the receiver is a raw pointer the user must ensure that it is valid +/// (aligned, dereferenceable, initialized and so on). Messages to `null` +/// pointers are allowed (though heavily discouraged), but only if the +/// return type itself is a pointer. /// -/// Additionally, the call must not violate Rust's mutability rules, e.g. if -/// passing an `&T` the Objective-C method must not mutate the variable. +/// 5. The method must not (yet, see [RFC-2945]) throw an exception. /// -/// If the receiver is a raw pointer the user must ensure that it is valid -/// (aligned, dereferenceable, initialized and so on). Messages to `null` -/// pointers are allowed (though discouraged), but only if the return type -/// itself is a pointer. +/// 6. You must uphold any additional safety requirements (explicit and +/// implicit) that the method has (for example, methods that take pointers +/// usually require that the pointer is valid, and sometimes non-null. +/// Another example, some methods may only be called on the main thread). /// -/// Finally, the method must not (yet, see [RFC-2945]) throw an exception. +/// 7. TODO: Maybe more? /// /// # Examples /// @@ -132,7 +157,7 @@ macro_rules! msg_send { [super($obj:expr, $superclass:expr), $selector:ident $(,)?] => ({ let sel = $crate::sel!($selector); let result; - match $crate::MessageReceiver::send_super_message(&$obj, $superclass, sel, ()) { + match $crate::MessageReceiver::send_super_message($obj, $superclass, sel, ()) { Err(s) => panic!("{}", s), Ok(r) => result = r, } @@ -141,7 +166,7 @@ macro_rules! msg_send { [super($obj:expr, $superclass:expr), $($selector:ident : $argument:expr $(,)?)+] => ({ let sel = $crate::sel!($($selector :)+); let result; - match $crate::MessageReceiver::send_super_message(&$obj, $superclass, sel, ($($argument,)+)) { + match $crate::MessageReceiver::send_super_message($obj, $superclass, sel, ($($argument,)+)) { Err(s) => panic!("{}", s), Ok(r) => result = r, } @@ -150,7 +175,7 @@ macro_rules! msg_send { [$obj:expr, $selector:ident $(,)?] => ({ let sel = $crate::sel!($selector); let result; - match $crate::MessageReceiver::send_message(&$obj, sel, ()) { + match $crate::MessageReceiver::send_message($obj, sel, ()) { Err(s) => panic!("{}", s), Ok(r) => result = r, } @@ -159,7 +184,7 @@ macro_rules! msg_send { [$obj:expr, $($selector:ident : $argument:expr $(,)?)+] => ({ let sel = $crate::sel!($($selector :)+); let result; - match $crate::MessageReceiver::send_message(&$obj, sel, ($($argument,)+)) { + match $crate::MessageReceiver::send_message($obj, sel, ($($argument,)+)) { Err(s) => panic!("{}", s), Ok(r) => result = r, } diff --git a/objc2/src/message/mod.rs b/objc2/src/message/mod.rs index 3a29772ec..1371029d3 100644 --- a/objc2/src/message/mod.rs +++ b/objc2/src/message/mod.rs @@ -5,7 +5,7 @@ use core::mem::ManuallyDrop; use core::ptr::NonNull; use std::error::Error; -use crate::rc::{Id, Ownership}; +use crate::rc::{Id, Owned, Ownership}; use crate::runtime::{Class, Imp, Object, Sel}; use crate::{Encode, EncodeArguments, RefEncode}; @@ -69,19 +69,22 @@ unsafe impl Message for Class {} // TODO: Make this fully private pub(crate) mod private { - use super::{Id, ManuallyDrop, Message, MessageReceiver, NonNull, Ownership}; + use super::*; pub trait Sealed {} impl Sealed for *const T {} impl Sealed for *mut T {} + impl Sealed for NonNull {} impl<'a, T: Message + ?Sized> Sealed for &'a T {} impl<'a, T: Message + ?Sized> Sealed for &'a mut T {} - impl Sealed for NonNull {} - impl Sealed for Id {} - impl Sealed for ManuallyDrop {} + impl<'a, T: Message + ?Sized, O: Ownership> Sealed for &'a Id {} + impl<'a, T: Message + ?Sized> Sealed for &'a mut Id {} + + impl<'a, T: Message + ?Sized, O: Ownership> Sealed for &'a ManuallyDrop> {} + impl<'a, T: Message + ?Sized> Sealed for &'a mut ManuallyDrop> {} } /// Types that can directly be used as the receiver of Objective-C messages. @@ -96,9 +99,9 @@ pub(crate) mod private { /// # Safety /// /// [`Self::as_raw_receiver`] must be implemented correctly. -pub unsafe trait MessageReceiver: private::Sealed { +pub unsafe trait MessageReceiver: private::Sealed + Sized { /// Get a raw pointer to the receiver of the message. - fn as_raw_receiver(&self) -> *mut Object; + fn as_raw_receiver(self) -> *mut Object; /// Sends a message to self with the given selector and arguments. /// @@ -118,7 +121,7 @@ pub unsafe trait MessageReceiver: private::Sealed { /// The added invariant is that the selector must take the same number of /// arguments as is given. #[cfg_attr(not(feature = "verify_message"), inline(always))] - unsafe fn send_message(&self, sel: Sel, args: A) -> Result + unsafe fn send_message(self, sel: Sel, args: A) -> Result where A: MessageArguments, R: Encode, @@ -161,7 +164,7 @@ pub unsafe trait MessageReceiver: private::Sealed { /// arguments as is given. #[cfg_attr(not(feature = "verify_message"), inline(always))] unsafe fn send_super_message( - &self, + self, superclass: &Class, sel: Sel, args: A, @@ -202,7 +205,7 @@ pub unsafe trait MessageReceiver: private::Sealed { /// assert!(result.is_ok()); /// ``` #[cfg(feature = "malloc")] - fn verify_message(&self, sel: Sel) -> Result<(), MessageError> + fn verify_message(self, sel: Sel) -> Result<(), MessageError> where A: EncodeArguments, R: Encode, @@ -217,50 +220,64 @@ pub unsafe trait MessageReceiver: private::Sealed { unsafe impl MessageReceiver for *const T { #[inline] - fn as_raw_receiver(&self) -> *mut Object { - *self as *mut T as *mut Object + fn as_raw_receiver(self) -> *mut Object { + self as *mut T as *mut Object } } unsafe impl MessageReceiver for *mut T { #[inline] - fn as_raw_receiver(&self) -> *mut Object { - *self as *mut Object + fn as_raw_receiver(self) -> *mut Object { + self as *mut Object + } +} + +unsafe impl MessageReceiver for NonNull { + #[inline] + fn as_raw_receiver(self) -> *mut Object { + self.as_ptr() as *mut Object } } unsafe impl<'a, T: Message + ?Sized> MessageReceiver for &'a T { #[inline] - fn as_raw_receiver(&self) -> *mut Object { - *self as *const T as *mut T as *mut Object + fn as_raw_receiver(self) -> *mut Object { + self as *const T as *mut T as *mut Object } } unsafe impl<'a, T: Message + ?Sized> MessageReceiver for &'a mut T { #[inline] - fn as_raw_receiver(&self) -> *mut Object { - *self as *const T as *mut T as *mut Object + fn as_raw_receiver(self) -> *mut Object { + self as *const T as *mut T as *mut Object } } -unsafe impl MessageReceiver for NonNull { +unsafe impl<'a, T: Message + ?Sized, O: Ownership> MessageReceiver for &'a Id { #[inline] - fn as_raw_receiver(&self) -> *mut Object { + fn as_raw_receiver(self) -> *mut Object { self.as_ptr() as *mut Object } } -unsafe impl MessageReceiver for Id { +unsafe impl<'a, T: Message + ?Sized> MessageReceiver for &'a mut Id { #[inline] - fn as_raw_receiver(&self) -> *mut Object { + fn as_raw_receiver(self) -> *mut Object { self.as_ptr() as *mut Object } } -unsafe impl MessageReceiver for ManuallyDrop { +unsafe impl<'a, T: Message + ?Sized, O: Ownership> MessageReceiver for &'a ManuallyDrop> { + #[inline] + fn as_raw_receiver(self) -> *mut Object { + (**self).as_ptr() as *mut Object + } +} + +unsafe impl<'a, T: Message + ?Sized> MessageReceiver for &'a mut ManuallyDrop> { #[inline] - fn as_raw_receiver(&self) -> *mut Object { - (**self).as_raw_receiver() + fn as_raw_receiver(self) -> *mut Object { + (**self).as_ptr() as *mut Object } } @@ -382,10 +399,10 @@ mod tests { #[test] fn test_send_message() { - let obj = test_utils::custom_object(); + let mut obj = test_utils::custom_object(); let result: u32 = unsafe { - let _: () = msg_send![obj, setFoo: 4u32]; - msg_send![obj, foo] + let _: () = msg_send![&mut obj, setFoo: 4u32]; + msg_send![&obj, foo] }; assert_eq!(result, 4); } @@ -393,7 +410,7 @@ mod tests { #[test] fn test_send_message_stret() { let obj = test_utils::custom_object(); - let result: test_utils::CustomStruct = unsafe { msg_send![obj, customStruct] }; + let result: test_utils::CustomStruct = unsafe { msg_send![&obj, customStruct] }; let expected = test_utils::CustomStruct { a: 1, b: 2, @@ -431,15 +448,15 @@ mod tests { #[test] fn test_send_message_super() { - let obj = test_utils::custom_subclass_object(); + let mut obj = test_utils::custom_subclass_object(); let superclass = test_utils::custom_class(); unsafe { - let _: () = msg_send![obj, setFoo: 4u32]; - let foo: u32 = msg_send![super(obj, superclass), foo]; + let _: () = msg_send![&mut obj, setFoo: 4u32]; + let foo: u32 = msg_send![super(&obj, superclass), foo]; assert_eq!(foo, 4); // The subclass is overriden to return foo + 2 - let foo: u32 = msg_send![obj, foo]; + let foo: u32 = msg_send![&obj, foo]; assert_eq!(foo, 6); } } @@ -447,10 +464,10 @@ mod tests { #[test] fn test_send_message_manuallydrop() { let obj = test_utils::custom_object(); - let obj = ManuallyDrop::new(obj); + let mut obj = ManuallyDrop::new(obj); let result: u32 = unsafe { - let _: () = msg_send![obj, setFoo: 4u32]; - msg_send![obj, foo] + let _: () = msg_send![&mut obj, setFoo: 4u32]; + msg_send![&obj, foo] }; assert_eq!(result, 4); diff --git a/objc2/src/test_utils.rs b/objc2/src/test_utils.rs index 440d88955..2f2ce4c13 100644 --- a/objc2/src/test_utils.rs +++ b/objc2/src/test_utils.rs @@ -1,3 +1,4 @@ +use core::mem::ManuallyDrop; use core::ops::{Deref, DerefMut}; use std::os::raw::c_char; use std::sync::Once; @@ -20,12 +21,36 @@ impl CustomObject { } // TODO: Remove the need for this hack -impl crate::message::private::Sealed for CustomObject {} +impl crate::message::private::Sealed for &CustomObject {} +impl crate::message::private::Sealed for &mut CustomObject {} +impl crate::message::private::Sealed for &ManuallyDrop {} +impl crate::message::private::Sealed for &mut ManuallyDrop {} -unsafe impl MessageReceiver for CustomObject { +unsafe impl MessageReceiver for &CustomObject { #[inline] - fn as_raw_receiver(&self) -> *mut Object { - self.obj + fn as_raw_receiver(self) -> *mut Object { + (&**self).as_raw_receiver() + } +} + +unsafe impl MessageReceiver for &mut CustomObject { + #[inline] + fn as_raw_receiver(self) -> *mut Object { + (&**self).as_raw_receiver() + } +} + +unsafe impl MessageReceiver for &ManuallyDrop { + #[inline] + fn as_raw_receiver(self) -> *mut Object { + (&**self).as_raw_receiver() + } +} + +unsafe impl MessageReceiver for &mut ManuallyDrop { + #[inline] + fn as_raw_receiver(self) -> *mut Object { + (&**self).as_raw_receiver() } } diff --git a/tests/assembly/test_msg_send_zero_cost/lib.rs b/tests/assembly/test_msg_send_zero_cost/lib.rs index 9a29a4af4..faf37d5cb 100644 --- a/tests/assembly/test_msg_send_zero_cost/lib.rs +++ b/tests/assembly/test_msg_send_zero_cost/lib.rs @@ -5,5 +5,5 @@ use objc2::MessageReceiver; #[no_mangle] pub fn handle(obj: &Object, sel: Sel) -> *mut Object { - unsafe { MessageReceiver::send_message(&obj, sel, ()).unwrap() } + unsafe { MessageReceiver::send_message(obj, sel, ()).unwrap() } } diff --git a/tests/assembly/test_retain_autoreleased/lib.rs b/tests/assembly/test_retain_autoreleased/lib.rs index 9a722262c..ca1de6623 100644 --- a/tests/assembly/test_retain_autoreleased/lib.rs +++ b/tests/assembly/test_retain_autoreleased/lib.rs @@ -6,6 +6,6 @@ use objc2::MessageReceiver; #[no_mangle] pub fn handle(obj: &Object, sel: Sel) -> Option> { - let ptr: *mut Object = unsafe { MessageReceiver::send_message(&obj, sel, ()).unwrap() }; + let ptr: *mut Object = unsafe { MessageReceiver::send_message(obj, sel, ()).unwrap() }; unsafe { Id::retain_autoreleased(ptr) } } diff --git a/tests/ui/msg_send_only_message.stderr b/tests/ui/msg_send_only_message.stderr index 9dddfbff9..b3a2b28e2 100644 --- a/tests/ui/msg_send_only_message.stderr +++ b/tests/ui/msg_send_only_message.stderr @@ -5,11 +5,13 @@ error[E0277]: the trait bound `{integer}: MessageReceiver` is not satisfied | ^^^^^^^^^^^^^^^^^ the trait `MessageReceiver` is not implemented for `{integer}` | = help: the following other types implement trait `MessageReceiver`: + &'a Id + &'a ManuallyDrop> &'a T + &'a mut Id + &'a mut ManuallyDrop> &'a mut T *const T *mut T - Id - ManuallyDrop NonNull = note: this error originates in the macro `msg_send` (in Nightly builds, run with -Z macro-backtrace for more info) From 0f35f4fbb543a778529574235b7d76a9a9952c31 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 6 Jun 2022 17:58:31 +0200 Subject: [PATCH 2/7] Make reborrow fixes easier to read --- objc2-foundation/src/array.rs | 8 +++++--- objc2-foundation/src/data.rs | 8 ++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/objc2-foundation/src/array.rs b/objc2-foundation/src/array.rs index 5f79e3f6a..202e1bae9 100644 --- a/objc2-foundation/src/array.rs +++ b/objc2-foundation/src/array.rs @@ -272,14 +272,16 @@ impl NSMutableArray { obj } + fn remove_last(&mut self) { + unsafe { msg_send![self, removeLastObject] } + } + #[doc(alias = "removeLastObject")] pub fn pop(&mut self) -> Option> { self.last() .map(|obj| unsafe { Id::retain(obj as *const T as *mut T).unwrap_unchecked() }) .map(|obj| { - unsafe { - let _: () = msg_send![self, removeLastObject]; - } + self.remove_last(); obj }) } diff --git a/objc2-foundation/src/data.rs b/objc2-foundation/src/data.rs index c3829a800..c6351e3d5 100644 --- a/objc2-foundation/src/data.rs +++ b/objc2-foundation/src/data.rs @@ -154,10 +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 this = &mut *self; // Reborrow - let ptr: *mut c_void = unsafe { msg_send![this, mutableBytes] }; + let ptr = self.raw_bytes_mut(); // The bytes pointer may be null for length zero if ptr.is_null() { &mut [] From cdea41a45bc5006348184d83c096550f8797d19e Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Tue, 31 May 2022 22:48:29 +0200 Subject: [PATCH 3/7] Test that msg_send! consumes the receiver --- tests/ui/msg_send_mutable.rs | 14 ++++++++++++++ tests/ui/msg_send_mutable.stderr | 17 +++++++++++++++++ tests/ui/msg_send_not_encode.rs | 7 +++++-- tests/ui/msg_send_not_encode.stderr | 24 ++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 tests/ui/msg_send_mutable.rs create mode 100644 tests/ui/msg_send_mutable.stderr diff --git a/tests/ui/msg_send_mutable.rs b/tests/ui/msg_send_mutable.rs new file mode 100644 index 000000000..a1a431089 --- /dev/null +++ b/tests/ui/msg_send_mutable.rs @@ -0,0 +1,14 @@ +//! Test that `msg_send!` consumes their arguments, including the receiver. +//! +//! Ideally, it shouldn't be so, but that's how it works currently. +use objc2::{msg_send, class}; +use objc2::runtime::Object; + +fn main() { + let cls = class!(NSObject); + let obj: &mut Object = unsafe { msg_send![cls, new] }; + + let _: () = unsafe { msg_send![obj, selector] }; + // Could be solved with a reborrow + let _: () = unsafe { msg_send![obj, selector] }; +} diff --git a/tests/ui/msg_send_mutable.stderr b/tests/ui/msg_send_mutable.stderr new file mode 100644 index 000000000..b80c7f779 --- /dev/null +++ b/tests/ui/msg_send_mutable.stderr @@ -0,0 +1,17 @@ +error[E0382]: use of moved value: `obj` + --> ui/msg_send_mutable.rs:13:36 + | +9 | let obj: &mut Object = unsafe { msg_send![cls, new] }; + | --- move occurs because `obj` has type `&mut objc2::runtime::Object`, which does not implement the `Copy` trait +10 | +11 | let _: () = unsafe { msg_send![obj, selector] }; + | ------------------------ `obj` moved due to this method call +12 | // Could be solved with a reborrow +13 | let _: () = unsafe { msg_send![obj, selector] }; + | ^^^ value used here after move + | +note: this function takes ownership of the receiver `self`, which moves `obj` + --> $WORKSPACE/objc2/src/message/mod.rs + | + | unsafe fn send_message(self, sel: Sel, args: A) -> Result + | ^^^^ diff --git a/tests/ui/msg_send_not_encode.rs b/tests/ui/msg_send_not_encode.rs index f9e8ce57e..33587a7ac 100644 --- a/tests/ui/msg_send_not_encode.rs +++ b/tests/ui/msg_send_not_encode.rs @@ -1,9 +1,12 @@ -//! Test that return types that are not `Encode` are not accepted. +//! Test that types that are not `Encode` are not accepted. use objc2::{class, msg_send}; fn main() { + let cls = class!(NSData); unsafe { - let cls = class!(NSData); let _: Vec = msg_send![cls, new]; + + let x: Vec; + let _: () = msg_send![cls, newWith: x]; } } diff --git a/tests/ui/msg_send_not_encode.stderr b/tests/ui/msg_send_not_encode.stderr index 7bc8d38d5..0d44844dd 100644 --- a/tests/ui/msg_send_not_encode.stderr +++ b/tests/ui/msg_send_not_encode.stderr @@ -20,3 +20,27 @@ note: required by a bound in `send_message` | R: Encode, | ^^^^^^ required by this bound in `send_message` = note: this error originates in the macro `msg_send` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the trait bound `Vec: Encode` is not satisfied + --> ui/msg_send_not_encode.rs:10:21 + | +10 | let _: () = msg_send![cls, newWith: x]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Encode` is not implemented for `Vec` + | + = help: the following other types implement trait `Encode`: + &'a T + &'a mut T + () + *const T + *const c_void + *mut T + *mut c_void + ManuallyDrop + and 142 others + = note: required because of the requirements on the impl of `MessageArguments` for `(Vec,)` +note: required by a bound in `send_message` + --> $WORKSPACE/objc2/src/message/mod.rs + | + | A: MessageArguments, + | ^^^^^^^^^^^^^^^^ required by this bound in `send_message` + = note: this error originates in the macro `msg_send` (in Nightly builds, run with -Z macro-backtrace for more info) From 09257054fc137db4587af986a9c574a662011800 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 26 May 2022 21:55:30 +0200 Subject: [PATCH 4/7] Test a few different ways of sending messages that mutate the receiver --- objc2/src/message/mod.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/objc2/src/message/mod.rs b/objc2/src/message/mod.rs index 1371029d3..a76dd2b81 100644 --- a/objc2/src/message/mod.rs +++ b/objc2/src/message/mod.rs @@ -395,8 +395,26 @@ impl<'a> From> for MessageError { #[cfg(test)] mod tests { use super::*; + use crate::rc::{Id, Owned}; use crate::test_utils; + #[allow(unused)] + fn test_different_receivers(mut obj: Id) { + unsafe { + let x = &mut obj; + let _: () = msg_send![x, mutable1]; + // let _: () = msg_send![x, mutable2]; + let _: () = msg_send![&mut *obj, mutable1]; + let _: () = msg_send![&mut *obj, mutable2]; + let obj: NonNull = (&mut *obj).into(); + let _: () = msg_send![obj, mutable1]; + let _: () = msg_send![obj, mutable2]; + let obj: *mut Object = obj.as_ptr(); + let _: () = msg_send![obj, mutable1]; + let _: () = msg_send![obj, mutable2]; + } + } + #[test] fn test_send_message() { let mut obj = test_utils::custom_object(); From 1158e4cfe1eebf351bb3a413b22bc2ed8f6ef164 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 6 Jun 2022 17:43:02 +0200 Subject: [PATCH 5/7] Make `Id::as_ptr` an associated method --- objc2-foundation/src/attributed_string.rs | 4 ++-- objc2-foundation/src/mutable_attributed_string.rs | 4 ++-- objc2-foundation/src/mutable_string.rs | 4 ++-- objc2-foundation/src/string.rs | 4 ++-- objc2/benches/autorelease.rs | 4 ++-- objc2/src/message/mod.rs | 8 ++++---- objc2/src/rc/id.rs | 13 +++++-------- objc2/src/rc/weak_id.rs | 2 +- tests/src/test_object.rs | 4 ++-- 9 files changed, 22 insertions(+), 25 deletions(-) diff --git a/objc2-foundation/src/attributed_string.rs b/objc2-foundation/src/attributed_string.rs index 6df466507..f5dd4d9bb 100644 --- a/objc2-foundation/src/attributed_string.rs +++ b/objc2-foundation/src/attributed_string.rs @@ -153,11 +153,11 @@ 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 *mut NSAttributedString); assert!(s3.is_kind_of(NSMutableAttributedString::class())); } } diff --git a/objc2-foundation/src/mutable_attributed_string.rs b/objc2-foundation/src/mutable_attributed_string.rs index 733e75830..8d407115a 100644 --- a/objc2-foundation/src/mutable_attributed_string.rs +++ b/objc2-foundation/src/mutable_attributed_string.rs @@ -90,11 +90,11 @@ 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 *mut 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())); } } diff --git a/objc2-foundation/src/mutable_string.rs b/objc2-foundation/src/mutable_string.rs index f804b2398..46a7973ea 100644 --- a/objc2-foundation/src/mutable_string.rs +++ b/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 *mut 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())); } } diff --git a/objc2-foundation/src/string.rs b/objc2-foundation/src/string.rs index d06a06c3d..e9459c67c 100644 --- a/objc2-foundation/src/string.rs +++ b/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 *mut NSString); assert!(s3.is_kind_of(NSMutableString::class())); } diff --git a/objc2/benches/autorelease.rs b/objc2/benches/autorelease.rs index a0bc82c81..5e0ae9ef3 100644 --- a/objc2/benches/autorelease.rs +++ b/objc2/benches/autorelease.rs @@ -62,7 +62,7 @@ fn new_nsdata() -> Id { } fn new_leaked_nsdata() -> *mut Object { - ManuallyDrop::new(new_nsdata()).as_ptr() + Id::as_ptr(&*ManuallyDrop::new(new_nsdata())) } fn autoreleased_nsdata() -> *mut Object { @@ -84,7 +84,7 @@ fn new_nsstring() -> Id { } fn new_leaked_nsstring() -> *mut Object { - ManuallyDrop::new(new_nsstring()).as_ptr() + Id::as_ptr(&*ManuallyDrop::new(new_nsstring())) } fn autoreleased_nsstring() -> *mut Object { diff --git a/objc2/src/message/mod.rs b/objc2/src/message/mod.rs index a76dd2b81..6a6a073e0 100644 --- a/objc2/src/message/mod.rs +++ b/objc2/src/message/mod.rs @@ -256,28 +256,28 @@ unsafe impl<'a, T: Message + ?Sized> MessageReceiver for &'a mut T { unsafe impl<'a, T: Message + ?Sized, O: Ownership> MessageReceiver for &'a Id { #[inline] fn as_raw_receiver(self) -> *mut Object { - self.as_ptr() as *mut Object + Id::as_ptr(self) as *mut Object } } unsafe impl<'a, T: Message + ?Sized> MessageReceiver for &'a mut Id { #[inline] fn as_raw_receiver(self) -> *mut Object { - self.as_ptr() as *mut Object + Id::as_ptr(self) as *mut Object } } unsafe impl<'a, T: Message + ?Sized, O: Ownership> MessageReceiver for &'a ManuallyDrop> { #[inline] fn as_raw_receiver(self) -> *mut Object { - (**self).as_ptr() as *mut Object + Id::as_ptr(&**self) as *mut Object } } unsafe impl<'a, T: Message + ?Sized> MessageReceiver for &'a mut ManuallyDrop> { #[inline] fn as_raw_receiver(self) -> *mut Object { - (**self).as_ptr() as *mut Object + Id::as_ptr(&mut **self) as *mut Object } } diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index ab9b95e7b..397b517b5 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -191,14 +191,11 @@ impl Id { /// Returns a raw pointer to the object. /// /// The pointer is valid for at least as long as the `Id` is held. + /// + /// This is an associated method, and must be called as `Id::as_ptr(obj)`. #[inline] - pub fn as_ptr(&self) -> *mut T { - // Note: This is not an associated function, which breaks the - // guideline that smart pointers shouldn't add inherent methods! - // - // However, this method is quite useful when migrating old codebases, - // so I think we'll let it be here for now. - self.ptr.as_ptr() + pub fn as_ptr(this: &Id) -> *mut T { + this.ptr.as_ptr() } } @@ -382,7 +379,7 @@ impl Id { // retained objects it is hard to imagine a case where the inner type // has a method with the same name. - let ptr = ManuallyDrop::new(self).as_ptr() as *mut ffi::objc_object; + let ptr = ManuallyDrop::new(self).ptr.as_ptr() as *mut ffi::objc_object; // SAFETY: The `ptr` is guaranteed to be valid and have at least one // retain count. // And because of the ManuallyDrop, we don't call the Drop diff --git a/objc2/src/rc/weak_id.rs b/objc2/src/rc/weak_id.rs index 3e19550da..973b9cd69 100644 --- a/objc2/src/rc/weak_id.rs +++ b/objc2/src/rc/weak_id.rs @@ -43,7 +43,7 @@ impl WeakId { // allow loading an `Id` later on. // SAFETY: `obj` is valid - unsafe { Self::new_inner(obj.as_ptr()) } + unsafe { Self::new_inner(Id::as_ptr(obj)) } } /// # Safety diff --git a/tests/src/test_object.rs b/tests/src/test_object.rs index ab9c182d3..4e9c598a6 100644 --- a/tests/src/test_object.rs +++ b/tests/src/test_object.rs @@ -179,12 +179,12 @@ fn test_object() { assert!(obj.var3().is_null()); assert!(obj.var3_ivar().is_null()); - let obj2 = ManuallyDrop::new(NSObject::new()).as_ptr().cast::(); + let obj2 = Id::as_ptr(&mut *ManuallyDrop::new(NSObject::new())).cast::(); obj.set_var3(obj2); assert_eq!(obj.var3(), obj2); assert_eq!(*obj.var3_ivar(), obj2); - let obj3 = ManuallyDrop::new(NSObject::new()).as_ptr().cast::(); + let obj3 = Id::as_ptr(&mut *ManuallyDrop::new(NSObject::new())).cast::(); *obj.var3_ivar_mut() = obj3; assert_ne!(obj.var3(), obj2); assert_ne!(*obj.var3_ivar(), obj2); From d9a328e6913118874f933c75e66094e3d7b58f59 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 28 May 2022 23:50:18 +0200 Subject: [PATCH 6/7] Add `Id::as_mut_ptr` --- objc2-foundation/src/attributed_string.rs | 5 ++++- .../src/mutable_attributed_string.rs | 5 ++++- objc2-foundation/src/mutable_string.rs | 2 +- objc2-foundation/src/string.rs | 2 +- objc2/CHANGELOG.md | 2 +- objc2/benches/autorelease.rs | 16 ++++++++-------- objc2/src/message/mod.rs | 4 ++-- objc2/src/rc/id.rs | 19 ++++++++++++++++++- objc2/src/rc/weak_id.rs | 6 +++--- tests/src/test_object.rs | 4 ++-- 10 files changed, 44 insertions(+), 21 deletions(-) diff --git a/objc2-foundation/src/attributed_string.rs b/objc2-foundation/src/attributed_string.rs index f5dd4d9bb..1417f5805 100644 --- a/objc2-foundation/src/attributed_string.rs +++ b/objc2-foundation/src/attributed_string.rs @@ -157,7 +157,10 @@ mod tests { assert!(s2.is_kind_of(NSAttributedString::class())); let s3 = s1.mutable_copy(); - assert_ne!(Id::as_ptr(&s1), Id::as_ptr(&s3) as *mut NSAttributedString); + assert_ne!( + Id::as_ptr(&s1), + Id::as_ptr(&s3) as *const NSAttributedString + ); assert!(s3.is_kind_of(NSMutableAttributedString::class())); } } diff --git a/objc2-foundation/src/mutable_attributed_string.rs b/objc2-foundation/src/mutable_attributed_string.rs index 8d407115a..77100dee9 100644 --- a/objc2-foundation/src/mutable_attributed_string.rs +++ b/objc2-foundation/src/mutable_attributed_string.rs @@ -90,7 +90,10 @@ mod tests { fn test_copy() { let s1 = NSMutableAttributedString::from_nsstring(&NSString::from_str("abc")); let s2 = s1.copy(); - assert_ne!(Id::as_ptr(&s1) as *mut NSAttributedString, Id::as_ptr(&s2)); + 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(); diff --git a/objc2-foundation/src/mutable_string.rs b/objc2-foundation/src/mutable_string.rs index 46a7973ea..d12e49eb6 100644 --- a/objc2-foundation/src/mutable_string.rs +++ b/objc2-foundation/src/mutable_string.rs @@ -200,7 +200,7 @@ mod tests { fn test_copy() { let s1 = NSMutableString::from_str("abc"); let s2 = s1.copy(); - assert_ne!(Id::as_ptr(&s1), Id::as_ptr(&s2) 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(); diff --git a/objc2-foundation/src/string.rs b/objc2-foundation/src/string.rs index e9459c67c..7c5660a46 100644 --- a/objc2-foundation/src/string.rs +++ b/objc2-foundation/src/string.rs @@ -350,7 +350,7 @@ mod tests { assert!(s2.is_kind_of(NSString::class())); let s3 = s1.mutable_copy(); - assert_ne!(Id::as_ptr(&s1), Id::as_ptr(&s3) as *mut NSString); + assert_ne!(Id::as_ptr(&s1), Id::as_ptr(&s3) as *const NSString); assert!(s3.is_kind_of(NSMutableString::class())); } diff --git a/objc2/CHANGELOG.md b/objc2/CHANGELOG.md index a01897653..13e73e5ce 100644 --- a/objc2/CHANGELOG.md +++ b/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. diff --git a/objc2/benches/autorelease.rs b/objc2/benches/autorelease.rs index 5e0ae9ef3..7bb047d16 100644 --- a/objc2/benches/autorelease.rs +++ b/objc2/benches/autorelease.rs @@ -61,11 +61,11 @@ fn new_nsdata() -> Id { unsafe { Id::new(obj).unwrap_unchecked() } } -fn new_leaked_nsdata() -> *mut Object { +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 { unsafe { Id::new(obj).unwrap_unchecked() } } -fn new_leaked_nsstring() -> *mut Object { +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 { - unsafe { Id::retain_autoreleased(obj.cast()).unwrap_unchecked() } +fn retain_autoreleased(obj: *const Object) -> Id { + 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 autoreleasepool(|_| retain_autoreleased(autoreleased_nsdata())) } -fn autoreleased_nsstring_pool_cleanup() -> *mut Object { +fn autoreleased_nsstring_pool_cleanup() -> *const Object { autoreleasepool(|_| autoreleased_nsstring()) } diff --git a/objc2/src/message/mod.rs b/objc2/src/message/mod.rs index 6a6a073e0..3ea1c390d 100644 --- a/objc2/src/message/mod.rs +++ b/objc2/src/message/mod.rs @@ -263,7 +263,7 @@ unsafe impl<'a, T: Message + ?Sized, O: Ownership> MessageReceiver for &'a Id MessageReceiver for &'a mut Id { #[inline] fn as_raw_receiver(self) -> *mut Object { - Id::as_ptr(self) as *mut Object + Id::as_mut_ptr(self) as *mut Object } } @@ -277,7 +277,7 @@ unsafe impl<'a, T: Message + ?Sized, O: Ownership> MessageReceiver for &'a Manua unsafe impl<'a, T: Message + ?Sized> MessageReceiver for &'a mut ManuallyDrop> { #[inline] fn as_raw_receiver(self) -> *mut Object { - Id::as_ptr(&mut **self) as *mut Object + Id::as_mut_ptr(&mut **self) as *mut Object } } diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index 397b517b5..dbf3bb7ad 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -192,9 +192,26 @@ impl Id { /// /// The pointer is valid for at least as long as the `Id` is held. /// + /// See [`Id::as_mut_ptr`] for the mutable equivalent. + /// /// This is an associated method, and must be called as `Id::as_ptr(obj)`. #[inline] - pub fn as_ptr(this: &Id) -> *mut T { + pub fn as_ptr(this: &Id) -> *const T { + this.ptr.as_ptr() + } +} + +impl Id { + /// Returns a raw mutable pointer to the object. + /// + /// The pointer is valid for at least as long as the `Id` is held. + /// + /// See [`Id::as_ptr`] for the immutable equivalent. + /// + /// This is an associated method, and must be called as + /// `Id::as_mut_ptr(obj)`. + #[inline] + pub fn as_mut_ptr(this: &mut Id) -> *mut T { this.ptr.as_ptr() } } diff --git a/objc2/src/rc/weak_id.rs b/objc2/src/rc/weak_id.rs index 973b9cd69..c5332dd6a 100644 --- a/objc2/src/rc/weak_id.rs +++ b/objc2/src/rc/weak_id.rs @@ -49,10 +49,10 @@ impl WeakId { /// # Safety /// /// The object must be valid or null. - unsafe fn new_inner(obj: *mut T) -> Self { + unsafe fn new_inner(obj: *const T) -> Self { let inner = Box::new(UnsafeCell::new(ptr::null_mut())); // SAFETY: `ptr` will never move, and the caller verifies `obj` - let _ = unsafe { ffi::objc_initWeak(inner.get(), obj as *mut ffi::objc_object) }; + let _ = unsafe { ffi::objc_initWeak(inner.get(), obj as *mut T as *mut ffi::objc_object) }; Self { inner, item: PhantomData, @@ -108,7 +108,7 @@ impl Default for WeakId { #[inline] fn default() -> Self { // SAFETY: The pointer is null - unsafe { Self::new_inner(ptr::null_mut()) } + unsafe { Self::new_inner(ptr::null()) } } } diff --git a/tests/src/test_object.rs b/tests/src/test_object.rs index 4e9c598a6..46d9d4a1d 100644 --- a/tests/src/test_object.rs +++ b/tests/src/test_object.rs @@ -179,12 +179,12 @@ fn test_object() { assert!(obj.var3().is_null()); assert!(obj.var3_ivar().is_null()); - let obj2 = Id::as_ptr(&mut *ManuallyDrop::new(NSObject::new())).cast::(); + let obj2 = Id::as_mut_ptr(&mut *ManuallyDrop::new(NSObject::new())).cast::(); obj.set_var3(obj2); assert_eq!(obj.var3(), obj2); assert_eq!(*obj.var3_ivar(), obj2); - let obj3 = Id::as_ptr(&mut *ManuallyDrop::new(NSObject::new())).cast::(); + let obj3 = Id::as_mut_ptr(&mut *ManuallyDrop::new(NSObject::new())).cast::(); *obj.var3_ivar_mut() = obj3; assert_ne!(obj.var3(), obj2); assert_ne!(*obj.var3_ivar(), obj2); From 2beef6140ba180c515d59a2b5761aa29b8751472 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 6 Jun 2022 18:57:36 +0200 Subject: [PATCH 7/7] Consume `ManuallyDrop>` in `msg_send!` If you do not want to consume it, the possibility of doing `msg_send![&*obj]` exists, but most of the time that is indeed what you want (I mean, why else would you wrap it in `ManuallyDrop`?) --- objc2/src/message/mod.rs | 28 +++++++-------------------- objc2/src/rc/id.rs | 5 +++++ objc2/src/test_utils.rs | 26 ++++++++++++------------- tests/ui/msg_send_only_message.stderr | 3 +-- 4 files changed, 26 insertions(+), 36 deletions(-) diff --git a/objc2/src/message/mod.rs b/objc2/src/message/mod.rs index 3ea1c390d..a1aebc473 100644 --- a/objc2/src/message/mod.rs +++ b/objc2/src/message/mod.rs @@ -83,8 +83,7 @@ pub(crate) mod private { impl<'a, T: Message + ?Sized, O: Ownership> Sealed for &'a Id {} impl<'a, T: Message + ?Sized> Sealed for &'a mut Id {} - impl<'a, T: Message + ?Sized, O: Ownership> Sealed for &'a ManuallyDrop> {} - impl<'a, T: Message + ?Sized> Sealed for &'a mut ManuallyDrop> {} + impl Sealed for ManuallyDrop> {} } /// Types that can directly be used as the receiver of Objective-C messages. @@ -267,17 +266,10 @@ unsafe impl<'a, T: Message + ?Sized> MessageReceiver for &'a mut Id { } } -unsafe impl<'a, T: Message + ?Sized, O: Ownership> MessageReceiver for &'a ManuallyDrop> { +unsafe impl MessageReceiver for ManuallyDrop> { #[inline] fn as_raw_receiver(self) -> *mut Object { - Id::as_ptr(&**self) as *mut Object - } -} - -unsafe impl<'a, T: Message + ?Sized> MessageReceiver for &'a mut ManuallyDrop> { - #[inline] - fn as_raw_receiver(self) -> *mut Object { - Id::as_mut_ptr(&mut **self) as *mut Object + Id::consume_as_ptr(self) as *mut Object } } @@ -481,17 +473,11 @@ mod tests { #[test] fn test_send_message_manuallydrop() { - let obj = test_utils::custom_object(); - let mut obj = ManuallyDrop::new(obj); - let result: u32 = unsafe { - let _: () = msg_send![&mut obj, setFoo: 4u32]; - msg_send![&obj, foo] + let obj = ManuallyDrop::new(test_utils::custom_object()); + unsafe { + let _: () = msg_send![obj, release]; }; - assert_eq!(result, 4); - - let obj: *const ManuallyDrop = obj.as_ptr().cast(); - let result: u32 = unsafe { msg_send![obj, foo] }; - assert_eq!(result, 4); + // `obj` is consumed, can't use here } #[test] diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index dbf3bb7ad..6dae8357c 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -199,6 +199,11 @@ impl Id { pub fn as_ptr(this: &Id) -> *const T { this.ptr.as_ptr() } + + #[inline] + pub(crate) fn consume_as_ptr(this: ManuallyDrop) -> *mut T { + this.ptr.as_ptr() + } } impl Id { diff --git a/objc2/src/test_utils.rs b/objc2/src/test_utils.rs index 2f2ce4c13..d62f0dfcb 100644 --- a/objc2/src/test_utils.rs +++ b/objc2/src/test_utils.rs @@ -23,34 +23,26 @@ impl CustomObject { // TODO: Remove the need for this hack impl crate::message::private::Sealed for &CustomObject {} impl crate::message::private::Sealed for &mut CustomObject {} -impl crate::message::private::Sealed for &ManuallyDrop {} -impl crate::message::private::Sealed for &mut ManuallyDrop {} +impl crate::message::private::Sealed for ManuallyDrop {} unsafe impl MessageReceiver for &CustomObject { #[inline] fn as_raw_receiver(self) -> *mut Object { - (&**self).as_raw_receiver() + self.obj } } unsafe impl MessageReceiver for &mut CustomObject { #[inline] fn as_raw_receiver(self) -> *mut Object { - (&**self).as_raw_receiver() + self.obj } } -unsafe impl MessageReceiver for &ManuallyDrop { +unsafe impl MessageReceiver for ManuallyDrop { #[inline] fn as_raw_receiver(self) -> *mut Object { - (&**self).as_raw_receiver() - } -} - -unsafe impl MessageReceiver for &mut ManuallyDrop { - #[inline] - fn as_raw_receiver(self) -> *mut Object { - (&**self).as_raw_receiver() + self.obj } } @@ -106,6 +98,11 @@ pub(crate) fn custom_class() -> &'static Class { builder.add_protocol(proto); builder.add_ivar::("_foo"); + unsafe extern "C" fn custom_obj_release(this: *mut Object, _cmd: Sel) { + // Drop the value + let _ = CustomObject { obj: this }; + } + extern "C" fn custom_obj_set_foo(this: &mut Object, _cmd: Sel, foo: u32) { unsafe { this.set_ivar::("_foo", foo); @@ -145,6 +142,9 @@ pub(crate) fn custom_class() -> &'static Class { } unsafe { + let release: unsafe extern "C" fn(*mut Object, Sel) = custom_obj_release; + builder.add_method(sel!(release), release); + let set_foo: extern "C" fn(&mut Object, Sel, u32) = custom_obj_set_foo; builder.add_method(sel!(setFoo:), set_foo); let get_foo: extern "C" fn(&Object, Sel) -> u32 = custom_obj_get_foo; diff --git a/tests/ui/msg_send_only_message.stderr b/tests/ui/msg_send_only_message.stderr index b3a2b28e2..912dcf9bb 100644 --- a/tests/ui/msg_send_only_message.stderr +++ b/tests/ui/msg_send_only_message.stderr @@ -6,12 +6,11 @@ error[E0277]: the trait bound `{integer}: MessageReceiver` is not satisfied | = help: the following other types implement trait `MessageReceiver`: &'a Id - &'a ManuallyDrop> &'a T &'a mut Id - &'a mut ManuallyDrop> &'a mut T *const T *mut T + ManuallyDrop> NonNull = note: this error originates in the macro `msg_send` (in Nightly builds, run with -Z macro-backtrace for more info)