From a925100abfe59ba3f3e407a86bc12bd23a8d4d60 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 15 Jun 2022 02:54:54 +0200 Subject: [PATCH] Add rc::Allocated --- objc2/CHANGELOG.md | 1 + objc2/src/__macro_helpers.rs | 28 +++++----- objc2/src/macros.rs | 10 ++-- objc2/src/rc/allocated.rs | 15 ++++++ objc2/src/rc/id.rs | 44 ++++++++++++---- objc2/src/rc/mod.rs | 2 + tests/assembly/test_msg_send_id/lib.rs | 11 ++-- tests/ui/msg_send_id_invalid_receiver.rs | 8 ++- tests/ui/msg_send_id_invalid_receiver.stderr | 54 ++++++++++++++++---- tests/ui/msg_send_id_invalid_return.rs | 13 ++--- tests/ui/msg_send_id_invalid_return.stderr | 44 +++++++++------- 11 files changed, 160 insertions(+), 70 deletions(-) create mode 100644 objc2/src/rc/allocated.rs diff --git a/objc2/CHANGELOG.md b/objc2/CHANGELOG.md index a76824285..4a7bcf2d1 100644 --- a/objc2/CHANGELOG.md +++ b/objc2/CHANGELOG.md @@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). through `&Object`. * Added `VerificationError` as more specific return type from `Class::verify_sel`. +* Added `rc::Allocated` struct which is used within `msg_send_id!`. ### Changed * **BREAKING**: `Sel` is now required to be non-null, which means that you diff --git a/objc2/src/__macro_helpers.rs b/objc2/src/__macro_helpers.rs index 73bb752ae..7ce2ec021 100644 --- a/objc2/src/__macro_helpers.rs +++ b/objc2/src/__macro_helpers.rs @@ -1,4 +1,4 @@ -use crate::rc::{Id, Ownership}; +use crate::rc::{Allocated, Id, Ownership}; use crate::runtime::{Class, Sel}; use crate::{Message, MessageArguments, MessageReceiver}; @@ -68,8 +68,8 @@ impl MsgSendId<&'_ Class, Id> } } -// `alloc`, should mark the return value as "allocated, not initialized" somehow -impl MsgSendId<&'_ Class, Id> +// `alloc` +impl MsgSendId<&'_ Class, Id, O>> for RetainSemantics { #[inline] @@ -78,26 +78,26 @@ impl MsgSendId<&'_ Class, Id> cls: &Class, sel: Sel, args: A, - ) -> Option> { + ) -> Option, O>> { // SAFETY: Checked by caller let obj = unsafe { MessageReceiver::send_message(cls, sel, args) }; // SAFETY: The selector is `alloc`, so this has +1 retain count - unsafe { Id::new(obj) } + unsafe { Id::new_allocated(obj) } } } -// `init`, should mark the input value as "allocated, not initialized" somehow -impl MsgSendId>, Id> +// `init` +impl MsgSendId, O>>, Id> for RetainSemantics { #[inline] #[track_caller] unsafe fn send_message_id( - obj: Option>, + obj: Option, O>>, sel: Sel, args: A, ) -> Option> { - let ptr = Id::option_into_ptr(obj); + let ptr = Id::option_into_ptr(obj.map(|obj| unsafe { Id::assume_init(obj) })); // SAFETY: `ptr` may be null here, but that's fine since the return // is `*mut T`, which is one of the few types where messages to nil is // allowed. @@ -191,7 +191,7 @@ mod tests { use core::ptr; - use crate::rc::{Owned, RcTestObject, Shared, ThreadTestData}; + use crate::rc::{Allocated, Owned, RcTestObject, Shared, ThreadTestData}; use crate::runtime::Object; use crate::{Encoding, RefEncode}; @@ -200,7 +200,7 @@ mod tests { let mut expected = ThreadTestData::current(); let cls = RcTestObject::class(); - let obj: Id = unsafe { msg_send_id![cls, alloc].unwrap() }; + let obj: Id, Shared> = unsafe { msg_send_id![cls, alloc].unwrap() }; expected.alloc += 1; expected.assert_current(); @@ -230,7 +230,7 @@ mod tests { let cls = RcTestObject::class(); let zone: *const _NSZone = ptr::null(); - let _obj: Id = + let _obj: Id, Owned> = unsafe { msg_send_id![cls, allocWithZone: zone].unwrap() }; // `+[NSObject alloc]` delegates to `+[NSObject allocWithZone:]`, but // `RcTestObject` only catches `alloc`. @@ -243,14 +243,14 @@ mod tests { let mut expected = ThreadTestData::current(); let cls = RcTestObject::class(); - let obj: Option> = unsafe { msg_send_id![cls, alloc] }; + let obj: Option, Shared>> = unsafe { msg_send_id![cls, alloc] }; expected.alloc += 1; // Don't check allocation error let _obj: Id = unsafe { msg_send_id![obj, init].unwrap() }; expected.init += 1; expected.assert_current(); - let obj: Option> = unsafe { msg_send_id![cls, alloc] }; + let obj: Option, Shared>> = unsafe { msg_send_id![cls, alloc] }; expected.alloc += 1; // Check allocation error before init let obj = obj.unwrap(); diff --git a/objc2/src/macros.rs b/objc2/src/macros.rs index d6a691f6e..ab69033ab 100644 --- a/objc2/src/macros.rs +++ b/objc2/src/macros.rs @@ -590,11 +590,12 @@ macro_rules! msg_send_bool { /// is a generic `Option>`. /// /// - The `alloc` family: The receiver must be `&Class`, and the return type -/// is a generic `Option>`. (This will change, see [#172]). +/// is a generic `Option, O>>`. /// -/// - The `init` family: The receiver must be `Option>` as returned -/// from `alloc`. The receiver is consumed, and a the now-initialized -/// `Option>` (with the same `T` and `O`) is returned. +/// - The `init` family: The receiver must be `Option, O>>` +/// as returned from `alloc`. The receiver is consumed, and a the +/// now-initialized `Option>` (with the same `T` and `O`) is +/// returned. /// /// - The `copy` family: The receiver may be anything that implements /// [`MessageReceiver`] and the return type is a generic `Option>`. @@ -615,7 +616,6 @@ macro_rules! msg_send_bool { /// [`Id::retain`], [`Id::drop`] and [`Id::autorelease`] for that. /// /// [sel-families]: https://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-method-families -/// [#172]: https://github.com/madsmtm/objc2/pull/172 /// [`MessageReceiver`]: crate::MessageReceiver /// [`Id::retain_autoreleased`]: crate::rc::Id::retain_autoreleased /// [arc-retainable]: https://clang.llvm.org/docs/AutomaticReferenceCounting.html#retainable-object-pointers-as-operands-and-arguments diff --git a/objc2/src/rc/allocated.rs b/objc2/src/rc/allocated.rs new file mode 100644 index 000000000..163baceb3 --- /dev/null +++ b/objc2/src/rc/allocated.rs @@ -0,0 +1,15 @@ +/// A marker type that can be used within [`Id`] to indicate that the object +/// has been allocated but not initialized. +/// +/// The reason we use `Option, O>>` instead of just `*mut T` +/// is: +/// - To allow releasing allocated objects, e.g. in the face of panics. +/// - To safely know the object is valid (albeit uninitialized). +/// - To allow specifying ownership. +/// +/// [`Id`]: crate::rc::Id +#[repr(transparent)] +#[derive(Debug)] +pub struct Allocated(T); + +// Explicitly don't implement `Deref`, `Message` nor `RefEncode`! diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index 1b795a2d3..2d67247a5 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -5,6 +5,7 @@ use core::ops::{Deref, DerefMut}; use core::panic::{RefUnwindSafe, UnwindSafe}; use core::ptr::NonNull; +use super::Allocated; use super::AutoreleasePool; use super::{Owned, Ownership, Shared}; use crate::ffi; @@ -124,6 +125,39 @@ pub struct Id { notunwindsafe: PhantomData<&'static mut ()>, } +impl Id { + #[inline] + unsafe fn new_nonnull(ptr: NonNull) -> Self { + Self { + ptr, + item: PhantomData, + own: PhantomData, + notunwindsafe: PhantomData, + } + } +} + +impl Id, O> { + #[inline] + pub(crate) unsafe fn new_allocated(ptr: *mut T) -> Option { + // SAFETY: Upheld by the caller + NonNull::new(ptr as *mut Allocated).map(|ptr| unsafe { Self::new_nonnull(ptr) }) + } + + #[inline] + pub(crate) unsafe fn assume_init(this: Self) -> Id { + let ptr = ManuallyDrop::new(this).ptr; + + // NonNull::cast + let ptr = ptr.as_ptr() as *mut T; + let ptr = unsafe { NonNull::new_unchecked(ptr) }; + + // SAFETY: The pointer is valid. + // Caller verifies that the object is allocated. + unsafe { Id::new_nonnull(ptr) } + } +} + impl Id { /// Constructs an [`Id`] to an object that already has +1 retain count. /// @@ -181,16 +215,6 @@ impl Id { NonNull::new(ptr).map(|ptr| unsafe { Id::new_nonnull(ptr) }) } - #[inline] - unsafe fn new_nonnull(ptr: NonNull) -> Id { - Self { - ptr, - item: PhantomData, - own: PhantomData, - notunwindsafe: PhantomData, - } - } - /// Returns a raw pointer to the object. /// /// The pointer is valid for at least as long as the `Id` is held. diff --git a/objc2/src/rc/mod.rs b/objc2/src/rc/mod.rs index bbc04e1f0..ebbfdf19c 100644 --- a/objc2/src/rc/mod.rs +++ b/objc2/src/rc/mod.rs @@ -55,6 +55,7 @@ //! assert!(weak.load().is_none()); //! ``` +mod allocated; mod autorelease; mod id; mod id_forwarding_impls; @@ -65,6 +66,7 @@ mod weak_id; #[cfg(test)] mod test_object; +pub use self::allocated::Allocated; pub use self::autorelease::{autoreleasepool, AutoreleasePool, AutoreleaseSafe}; pub use self::id::Id; pub use self::id_traits::{DefaultId, SliceId, SliceIdMut}; diff --git a/tests/assembly/test_msg_send_id/lib.rs b/tests/assembly/test_msg_send_id/lib.rs index c1026aedb..3f64f5266 100644 --- a/tests/assembly/test_msg_send_id/lib.rs +++ b/tests/assembly/test_msg_send_id/lib.rs @@ -1,15 +1,18 @@ //! Test assembly output of `msg_send_id!` internals. use objc2::__macro_helpers::{MsgSendId, RetainSemantics}; -use objc2::rc::{Id, Shared}; +use objc2::rc::{Allocated, Id, Shared}; use objc2::runtime::{Class, Object, Sel}; #[no_mangle] -unsafe fn handle_alloc(obj: &Class, sel: Sel) -> Option> { +unsafe fn handle_alloc(obj: &Class, sel: Sel) -> Option, Shared>> { >::send_message_id(obj, sel, ()) } #[no_mangle] -unsafe fn handle_init(obj: Option>, sel: Sel) -> Option> { +unsafe fn handle_init( + obj: Option, Shared>>, + sel: Sel, +) -> Option> { >::send_message_id(obj, sel, ()) } @@ -21,7 +24,7 @@ unsafe fn handle_alloc_init(obj: &Class, sel1: Sel, sel2: Sel) -> Option = + let _obj: Id, Shared> = >::send_message_id(cls, sel, ()) .unwrap_unchecked(); } diff --git a/tests/ui/msg_send_id_invalid_receiver.rs b/tests/ui/msg_send_id_invalid_receiver.rs index 2a85010f6..71ae789b6 100644 --- a/tests/ui/msg_send_id_invalid_receiver.rs +++ b/tests/ui/msg_send_id_invalid_receiver.rs @@ -1,16 +1,20 @@ //! Test compiler output with invalid msg_send_id receivers. use objc2::msg_send_id; use objc2::runtime::{Class, Object}; -use objc2::rc::{Id, Shared}; +use objc2::rc::{Allocated, Id, Shared}; fn main() { let obj: &Object; let _: Id = unsafe { msg_send_id![obj, new].unwrap() }; - let _: Id = unsafe { msg_send_id![obj, alloc].unwrap() }; + let _: Id, Shared> = unsafe { msg_send_id![obj, alloc].unwrap() }; let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; let cls: &Class; let _: Id = unsafe { msg_send_id![cls, init].unwrap() }; + let obj: Id; + let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; + let obj: Option>; + let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; let obj: Id; let _: Id = unsafe { msg_send_id![obj, copy].unwrap() }; diff --git a/tests/ui/msg_send_id_invalid_receiver.stderr b/tests/ui/msg_send_id_invalid_receiver.stderr index 9ecb34e0e..7f9b8a417 100644 --- a/tests/ui/msg_send_id_invalid_receiver.stderr +++ b/tests/ui/msg_send_id_invalid_receiver.stderr @@ -16,13 +16,13 @@ note: associated function defined here | ^^^^^^^^^^^^^^^ error[E0308]: mismatched types - --> ui/msg_send_id_invalid_receiver.rs:9:55 + --> ui/msg_send_id_invalid_receiver.rs:9:66 | -9 | let _: Id = unsafe { msg_send_id![obj, alloc].unwrap() }; - | -------------^^^-------- - | | | - | | expected struct `objc2::runtime::Class`, found struct `objc2::runtime::Object` - | arguments to this function are incorrect +9 | let _: Id, Shared> = unsafe { msg_send_id![obj, alloc].unwrap() }; + | -------------^^^-------- + | | | + | | expected struct `objc2::runtime::Class`, found struct `objc2::runtime::Object` + | arguments to this function are incorrect | = note: expected reference `&objc2::runtime::Class` found reference `&objc2::runtime::Object` @@ -41,7 +41,7 @@ error[E0308]: mismatched types | | expected enum `Option`, found `&objc2::runtime::Object` | arguments to this function are incorrect | - = note: expected enum `Option>` + = note: expected enum `Option, _>>` found reference `&objc2::runtime::Object` note: associated function defined here --> $WORKSPACE/objc2/src/__macro_helpers.rs @@ -58,7 +58,7 @@ error[E0308]: mismatched types | | expected enum `Option`, found `&objc2::runtime::Class` | arguments to this function are incorrect | - = note: expected enum `Option>` + = note: expected enum `Option, _>>` found reference `&objc2::runtime::Class` note: associated function defined here --> $WORKSPACE/objc2/src/__macro_helpers.rs @@ -66,10 +66,44 @@ note: associated function defined here | unsafe fn send_message_id(obj: T, sel: Sel, args: A) -> Option; | ^^^^^^^^^^^^^^^ +error[E0308]: mismatched types + --> ui/msg_send_id_invalid_receiver.rs:15:55 + | +15 | let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; + | -------------^^^------- + | | | + | | expected enum `Option`, found struct `Id` + | arguments to this function are incorrect + | + = note: expected enum `Option, _>>` + found struct `Id` +note: associated function defined here + --> $WORKSPACE/objc2/src/__macro_helpers.rs + | + | unsafe fn send_message_id(obj: T, sel: Sel, args: A) -> Option; + | ^^^^^^^^^^^^^^^ + +error[E0308]: mismatched types + --> ui/msg_send_id_invalid_receiver.rs:17:55 + | +17 | let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; + | -------------^^^------- + | | | + | | expected struct `Allocated`, found struct `objc2::runtime::Object` + | arguments to this function are incorrect + | + = note: expected enum `Option, _>>` + found enum `Option>` +note: associated function defined here + --> $WORKSPACE/objc2/src/__macro_helpers.rs + | + | unsafe fn send_message_id(obj: T, sel: Sel, args: A) -> Option; + | ^^^^^^^^^^^^^^^ + error[E0277]: the trait bound `Id: MessageReceiver` is not satisfied - --> ui/msg_send_id_invalid_receiver.rs:16:42 + --> ui/msg_send_id_invalid_receiver.rs:20:42 | -16 | let _: Id = unsafe { msg_send_id![obj, copy].unwrap() }; +20 | let _: Id = unsafe { msg_send_id![obj, copy].unwrap() }; | ^^^^^^^^^^^^^^^^^^^^^^^ the trait `MessageReceiver` is not implemented for `Id` | = help: the following other types implement trait `MessageReceiver`: diff --git a/tests/ui/msg_send_id_invalid_return.rs b/tests/ui/msg_send_id_invalid_return.rs index 303f71ce2..816b1987c 100644 --- a/tests/ui/msg_send_id_invalid_return.rs +++ b/tests/ui/msg_send_id_invalid_return.rs @@ -1,7 +1,7 @@ //! Test compiler output with invalid msg_send_id receivers. use objc2::msg_send_id; use objc2::runtime::{Class, Object}; -use objc2::rc::{Id, Owned, Shared}; +use objc2::rc::{Allocated, Id, Owned, Shared}; use objc2_foundation::NSObject; fn main() { @@ -9,15 +9,16 @@ fn main() { let _: &Object = unsafe { msg_send_id![cls, new].unwrap() }; let _: Id = unsafe { msg_send_id![cls, new].unwrap() }; let _: &Object = unsafe { msg_send_id![cls, alloc].unwrap() }; - let _: Id = unsafe { msg_send_id![cls, alloc].unwrap() }; + let _: Id, Shared> = unsafe { msg_send_id![cls, alloc].unwrap() }; + let _: Id = unsafe { msg_send_id![cls, alloc].unwrap() }; - let obj: Option>; + let obj: Option, Shared>>; let _: &Object = unsafe { msg_send_id![obj, init].unwrap() }; - let obj: Option>; + let obj: Option, Shared>>; let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; - let obj: Option>; + let obj: Option, Shared>>; let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; - let obj: Option>; + let obj: Option, Shared>>; let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; let obj: Id; diff --git a/tests/ui/msg_send_id_invalid_return.stderr b/tests/ui/msg_send_id_invalid_return.stderr index 3152f36d8..e110a0d25 100644 --- a/tests/ui/msg_send_id_invalid_return.stderr +++ b/tests/ui/msg_send_id_invalid_return.stderr @@ -33,19 +33,16 @@ error[E0308]: mismatched types --> ui/msg_send_id_invalid_return.rs:11:31 | 11 | let _: &Object = unsafe { msg_send_id![cls, alloc].unwrap() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | | - | expected `&objc2::runtime::Object`, found struct `Id` - | help: consider borrowing here: `&msg_send_id![cls, alloc].unwrap()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&objc2::runtime::Object`, found struct `Id` | = note: expected reference `&objc2::runtime::Object` - found struct `Id<_, _>` + found struct `Id, _>` error[E0277]: the trait bound `objc2::runtime::Class: Message` is not satisfied - --> ui/msg_send_id_invalid_return.rs:12:41 + --> ui/msg_send_id_invalid_return.rs:12:52 | -12 | let _: Id = unsafe { msg_send_id![cls, alloc].unwrap() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Message` is not implemented for `objc2::runtime::Class` +12 | let _: Id, Shared> = unsafe { msg_send_id![cls, alloc].unwrap() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Message` is not implemented for `objc2::runtime::Class` | = help: the following other types implement trait `Message`: NSArray @@ -57,13 +54,22 @@ error[E0277]: the trait bound `objc2::runtime::Class: Message` is not satisfied NSMutableData NSMutableString and 7 others - = note: required because of the requirements on the impl of `MsgSendId<&objc2::runtime::Class, Id>` for `RetainSemantics` + = note: required because of the requirements on the impl of `MsgSendId<&objc2::runtime::Class, Id, Shared>>` for `RetainSemantics` = note: this error originates in the macro `msg_send_id` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0308]: mismatched types - --> ui/msg_send_id_invalid_return.rs:15:31 + --> ui/msg_send_id_invalid_return.rs:13:42 + | +13 | let _: Id = unsafe { msg_send_id![cls, alloc].unwrap() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `objc2::runtime::Object`, found struct `Allocated` + | + = note: expected struct `Id` + found struct `Id, _>` + +error[E0308]: mismatched types + --> ui/msg_send_id_invalid_return.rs:16:31 | -15 | let _: &Object = unsafe { msg_send_id![obj, init].unwrap() }; +16 | let _: &Object = unsafe { msg_send_id![obj, init].unwrap() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | | expected `&objc2::runtime::Object`, found struct `Id` @@ -73,36 +79,36 @@ error[E0308]: mismatched types found struct `Id` error[E0308]: mismatched types - --> ui/msg_send_id_invalid_return.rs:17:41 + --> ui/msg_send_id_invalid_return.rs:18:41 | -17 | let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; +18 | let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `objc2::runtime::Class`, found struct `objc2::runtime::Object` | = note: expected struct `Id` found struct `Id` error[E0308]: mismatched types - --> ui/msg_send_id_invalid_return.rs:19:44 + --> ui/msg_send_id_invalid_return.rs:20:44 | -19 | let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; +20 | let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `NSObject`, found struct `objc2::runtime::Object` | = note: expected struct `Id` found struct `Id` error[E0308]: mismatched types - --> ui/msg_send_id_invalid_return.rs:21:41 + --> ui/msg_send_id_invalid_return.rs:22:41 | -21 | let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; +22 | let _: Id = unsafe { msg_send_id![obj, init].unwrap() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `objc2::rc::Owned`, found enum `Shared` | = note: expected struct `Id<_, objc2::rc::Owned>` found struct `Id<_, Shared>` error[E0308]: mismatched types - --> ui/msg_send_id_invalid_return.rs:24:31 + --> ui/msg_send_id_invalid_return.rs:25:31 | -24 | let _: &Object = unsafe { msg_send_id![&obj, description].unwrap() }; +25 | let _: &Object = unsafe { msg_send_id![&obj, description].unwrap() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | | expected `&objc2::runtime::Object`, found struct `Id`