Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Allocated<T> in preparation for arbitrary self types #520

Merged
merged 1 commit into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/header-translator/src/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ impl fmt::Display for Method {

// Receiver
if let MemoryManagement::IdInit = self.memory_management {
write!(f, "this: Option<Allocated<Self>>, ")?;
write!(f, "this: Allocated<Self>, ")?;
} else if self.is_class {
// Insert nothing; a class method is assumed
} else if self.mutating {
Expand Down
2 changes: 1 addition & 1 deletion crates/icrate/src/additions/Foundation/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ impl IdFromIterator<u8> for NSMutableData {
}

#[cfg(feature = "block")]
unsafe fn with_vec<T: Message>(obj: Option<Allocated<T>>, bytes: Vec<u8>) -> Id<T> {
unsafe fn with_vec<T: Message>(obj: Allocated<T>, bytes: Vec<u8>) -> Id<T> {
use core::mem::ManuallyDrop;

use block2::{Block, ConcreteBlock};
Expand Down
2 changes: 1 addition & 1 deletion crates/icrate/src/additions/Foundation/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl NSMutableString {
}
}

unsafe fn init_with_str<T: Message>(obj: Option<Allocated<T>>, string: &str) -> Id<T> {
unsafe fn init_with_str<T: Message>(obj: Allocated<T>, string: &str) -> Id<T> {
let bytes: *const c_void = string.as_ptr().cast();
// We use `msg_send_id` instead of the generated method from `icrate`,
// since that assumes the encoding is `usize`, whereas GNUStep assumes
Expand Down
2 changes: 1 addition & 1 deletion crates/icrate/src/additions/Foundation/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl MainThreadMarker {
/// let obj: Id<SomeClass> = unsafe { msg_send_id![obj, init] };
/// ```
#[inline]
pub fn alloc<T: ClassType>(self) -> Option<Allocated<T>> {
pub fn alloc<T: ClassType>(self) -> Allocated<T> {
// SAFETY: Same as `ClassType::alloc`, with the addition that since we
// take `self: MainThreadMarker`, the `IsAllocableAnyThread` bound is
// not required.
Expand Down
2 changes: 1 addition & 1 deletion crates/icrate/src/additions/Metal/private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ extern_methods!(
#[cfg(feature = "Metal_MTLDevice")]
#[method_id(initWithVertexData:fragmentData:serializedVertexDescriptor:device:options:flags:)]
pub unsafe fn initWithVertexData(
this: Option<Allocated<Self>>,
this: Allocated<Self>,
vertex_data: *mut c_void,
fragment_data: *mut c_void,
vertex_desc: *mut c_void,
Expand Down
5 changes: 1 addition & 4 deletions crates/icrate/src/fixes/Foundation/NSUUID.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ unsafe impl RefEncode for UuidBytes {
extern_methods!(
unsafe impl Foundation::NSUUID {
#[method_id(initWithUUIDBytes:)]
pub(crate) fn initWithUUIDBytes(
this: Option<Allocated<Self>>,
bytes: &UuidBytes,
) -> Id<Self>;
pub(crate) fn initWithUUIDBytes(this: Allocated<Self>, bytes: &UuidBytes) -> Id<Self>;

#[method(getUUIDBytes:)]
pub(crate) fn getUUIDBytes(&self, bytes: &mut UuidBytes);
Expand Down
2 changes: 1 addition & 1 deletion crates/icrate/src/generated
5 changes: 3 additions & 2 deletions crates/icrate/tests/mutable_array.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![cfg(feature = "Foundation_NSMutableArray")]
use objc2::rc::{__RcTestObject, __ThreadTestData, autoreleasepool};
use objc2::rc::{__RcTestObject, __ThreadTestData, autoreleasepool, Allocated};
use objc2::{msg_send, ClassType};

#[cfg(feature = "Foundation_NSNumber")]
Expand Down Expand Up @@ -123,7 +123,8 @@ fn test_threaded() {

s.spawn(|| {
let array = <NSMutableArray<NSObject>>::alloc();
assert!(array.is_some());
let ptr = Allocated::as_ptr(&array);
assert!(!ptr.is_null());
});
});
}
Expand Down
3 changes: 3 additions & 0 deletions crates/objc2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- `CounterpartOrSelf`.
* Added new `encode` traits `EncodeReturn`, `EncodeArgument` and
`EncodeArguments`.
* Added methods `as_ptr` and `as_mut_ptr` to `Allocated`.

### Changed
* **BREAKING**: `AnyClass::verify_sel` now take more well-defined types
Expand Down Expand Up @@ -47,6 +48,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
a method.
* **BREAKING**: Renamed the associated types `Ret` and `Args` on
`MethodImplementation` to `Return` and `Arguments`.
* **BREAKING**: Make `rc::Allocated` allowed to be `NULL` internally, such
that uses of `Option<Allocated<T>>` is now simply `Allocated<T>`.

### Deprecated
* Soft deprecated using `msg_send!` without a comma between arguments (i.e.
Expand Down
4 changes: 2 additions & 2 deletions crates/objc2/src/__macro_helpers/declare_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ where
}

// Explicitly left unimplemented for now!
// impl MessageRecieveId<impl MessageReceiver, Option<Allocated<T>>> for Alloc {}
// impl MessageRecieveId<impl MessageReceiver, Allocated<T>> for Alloc {}

// Note: `MethodImplementation` allows for `Allocated` as the receiver, so we
// restrict it here to only be when the selector is `init`.
Expand All @@ -56,7 +56,7 @@ where
impl<Ret, T> MessageRecieveId<Allocated<T>, Ret> for Init
where
T: Message,
Ret: MaybeOptionId<Input = Id<T>>,
Ret: MaybeOptionId<Input = Option<Id<T>>>,
{
#[inline]
fn into_return(obj: Ret) -> IdReturnValue {
Expand Down
4 changes: 2 additions & 2 deletions crates/objc2/src/__macro_helpers/method_family.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
/// retainable object pointer type - we ensure this by making
/// `message_send_id` return `Id`.
/// - `init`: The method must be an instance method and must return an
/// Objective-C pointer type - We ensure this by taking
/// `Option<Allocated<T>>`, which means it can't be a class method!
/// Objective-C pointer type - We ensure this by taking `Allocated<T>`,
/// which means it can't be a class method!
///
/// <https://clang.llvm.org/docs/AutomaticReferenceCounting.html#retainable-object-pointers-as-operands-and-arguments>
// TODO: Use an enum instead of u8 here when stable
Expand Down
78 changes: 33 additions & 45 deletions crates/objc2/src/__macro_helpers/msg_send_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ pub trait MsgSendId<T, U> {
/// `send_message_id` with that, and return an error if one occurred.
#[inline]
#[track_caller]
unsafe fn send_message_id_error<A, E>(obj: T, sel: Sel, args: A) -> Result<U, Id<E>>
unsafe fn send_message_id_error<A, E, R>(obj: T, sel: Sel, args: A) -> Result<R, Id<E>>
where
*mut *mut E: Encode,
A: TupleExtender<*mut *mut E>,
<A as TupleExtender<*mut *mut E>>::PlusOneArgument: ConvertArguments,
E: Message,
Option<U>: MaybeUnwrap<Input = U>,
Option<R>: MaybeUnwrap<Input = U>,
{
let mut err: *mut E = ptr::null_mut();
let args = args.add_argument(&mut err);
let res: Option<U> = unsafe { Self::send_message_id(obj, sel, args) };
let res: Option<R> = unsafe { Self::send_message_id(obj, sel, args) };
// As per the Cocoa documentation:
// > Success or failure is indicated by the return value of the
// > method. Although Cocoa methods that indirectly return error
Expand Down Expand Up @@ -66,9 +66,9 @@ unsafe fn encountered_error<E: Message>(err: *mut E) -> Id<E> {
unsafe { Id::retain(err) }.expect("error parameter should be set if the method returns NULL")
}

impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Id<U>> for New {
impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Option<Id<U>>> for New {
#[inline]
unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<U>>>(
unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Option<Id<U>>>>(
obj: T,
sel: Sel,
args: A,
Expand Down Expand Up @@ -96,18 +96,18 @@ impl<T: ?Sized + Message> MsgSendId<&'_ AnyClass, Allocated<T>> for Alloc {
let obj = unsafe { MsgSend::send_message(cls, sel, args) };
// SAFETY: The selector is `alloc`, so this has +1 retain count
let obj = unsafe { Allocated::new(obj) };
R::maybe_unwrap::<Self>(obj, (cls, sel))
R::maybe_unwrap::<Self>(obj, ())
}
}

impl<T: ?Sized + Message> MsgSendId<Option<Allocated<T>>, Id<T>> for Init {
impl<T: ?Sized + Message> MsgSendId<Allocated<T>, Option<Id<T>>> for Init {
#[inline]
unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<T>>>(
obj: Option<Allocated<T>>,
unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Option<Id<T>>>>(
obj: Allocated<T>,
sel: Sel,
args: A,
) -> R {
let ptr = Allocated::option_into_ptr(obj);
let ptr = Allocated::into_ptr(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.
Expand All @@ -121,9 +121,9 @@ impl<T: ?Sized + Message> MsgSendId<Option<Allocated<T>>, Id<T>> for Init {
}
}

impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Id<U>> for CopyOrMutCopy {
impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Option<Id<U>>> for CopyOrMutCopy {
#[inline]
unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<U>>>(
unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Option<Id<U>>>>(
obj: T,
sel: Sel,
args: A,
Expand All @@ -137,9 +137,9 @@ impl<T: MsgSend, U: ?Sized + Message> MsgSendId<T, Id<U>> for CopyOrMutCopy {
}
}

impl<T: MsgSend, U: Message> MsgSendId<T, Id<U>> for Other {
impl<T: MsgSend, U: Message> MsgSendId<T, Option<Id<U>>> for Other {
#[inline]
unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Id<U>>>(
unsafe fn send_message_id<A: ConvertArguments, R: MaybeUnwrap<Input = Option<Id<U>>>>(
obj: T,
sel: Sel,
args: A,
Expand All @@ -163,11 +163,11 @@ impl<T: MsgSend, U: Message> MsgSendId<T, Id<U>> for Other {
pub trait MaybeUnwrap {
type Input;
#[track_caller]
fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Option<Self::Input>, args: F::Args) -> Self;
fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Self::Input, args: F::Args) -> Self;
}

impl<T: ?Sized> MaybeUnwrap for Option<Id<T>> {
type Input = Id<T>;
type Input = Option<Id<T>>;

#[inline]
fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Option<Id<T>>, _args: F::Args) -> Self {
Expand All @@ -176,7 +176,7 @@ impl<T: ?Sized> MaybeUnwrap for Option<Id<T>> {
}

impl<T: ?Sized> MaybeUnwrap for Id<T> {
type Input = Id<T>;
type Input = Option<Id<T>>;

#[inline]
fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Option<Id<T>>, args: F::Args) -> Self {
Expand All @@ -187,24 +187,12 @@ impl<T: ?Sized> MaybeUnwrap for Id<T> {
}
}

impl<T: ?Sized> MaybeUnwrap for Option<Allocated<T>> {
type Input = Allocated<T>;

#[inline]
fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Option<Allocated<T>>, _args: F::Args) -> Self {
obj
}
}

impl<T: ?Sized> MaybeUnwrap for Allocated<T> {
type Input = Allocated<T>;

#[inline]
fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Option<Allocated<T>>, args: F::Args) -> Self {
match obj {
Some(obj) => obj,
None => F::failed(args),
}
fn maybe_unwrap<'a, F: MsgSendIdFailed<'a>>(obj: Allocated<T>, _args: F::Args) -> Self {
obj
}
}

Expand Down Expand Up @@ -244,15 +232,11 @@ impl<'a> MsgSendIdFailed<'a> for New {
}

impl<'a> MsgSendIdFailed<'a> for Alloc {
type Args = (&'a AnyClass, Sel);
type Args = ();

#[cold]
fn failed((cls, sel): Self::Args) -> ! {
if sel == sel!(alloc) {
panic!("failed allocating {cls}")
} else {
panic!("failed allocating with +[{cls} {sel}]")
}
fn failed(_: Self::Args) -> ! {
unreachable!()
}
}

Expand Down Expand Up @@ -385,18 +369,22 @@ mod tests {
let mut expected = __ThreadTestData::current();
let cls = __RcTestObject::class();

let obj: Option<Allocated<__RcTestObject>> = unsafe { msg_send_id![cls, alloc] };
let obj: Allocated<__RcTestObject> = unsafe { msg_send_id![cls, alloc] };
expected.alloc += 1;
expected.assert_current();

// Don't check allocation error
let _obj: Id<__RcTestObject> = unsafe { msg_send_id![obj, init] };
expected.init += 1;
expected.assert_current();

let obj: Option<Allocated<__RcTestObject>> = unsafe { msg_send_id![cls, alloc] };
let obj: Allocated<__RcTestObject> = unsafe { msg_send_id![cls, alloc] };
expected.alloc += 1;
expected.assert_current();

// Check allocation error before init
let obj = obj.unwrap();
let _obj: Id<__RcTestObject> = unsafe { msg_send_id![Some(obj), init] };
assert!(!Allocated::as_ptr(&obj).is_null());
let _obj: Id<__RcTestObject> = unsafe { msg_send_id![obj, init] };
expected.init += 1;
expected.assert_current();
}
Expand Down Expand Up @@ -474,7 +462,6 @@ mod tests {
}

#[test]
#[should_panic = "failed allocating with +[__RcTestObject allocReturningNull]"]
fn test_alloc_with_null() {
let _obj: Allocated<__RcTestObject> =
unsafe { msg_send_id![__RcTestObject::class(), allocReturningNull] };
Expand All @@ -483,7 +470,7 @@ mod tests {
#[test]
#[should_panic = "failed initializing object with -initReturningNull"]
fn test_init_with_null() {
let obj: Option<Allocated<__RcTestObject>> =
let obj: Allocated<__RcTestObject> =
unsafe { msg_send_id![__RcTestObject::class(), alloc] };
let _obj: Id<__RcTestObject> = unsafe { msg_send_id![obj, initReturningNull] };
}
Expand All @@ -492,7 +479,8 @@ mod tests {
#[should_panic = "failed allocating object"]
#[cfg(not(debug_assertions))] // Does NULL receiver checks
fn test_init_with_null_receiver() {
let obj: Option<Allocated<__RcTestObject>> = None;
let obj: Allocated<__RcTestObject> =
unsafe { msg_send_id![__RcTestObject::class(), allocReturningNull] };
let _obj: Id<__RcTestObject> = unsafe { msg_send_id![obj, init] };
}

Expand Down
4 changes: 2 additions & 2 deletions crates/objc2/src/macros/extern_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
/// #[method_id(initWithVal:)]
/// // Arbitary self types are not stable, but we can work around it
/// // with the special name `this`.
/// pub fn init(this: Option<Allocated<Self>>, val: usize) -> Id<Self>;
/// pub fn init(this: Allocated<Self>, val: usize) -> Id<Self>;
/// }
///
/// /// Instance accessor methods.
Expand Down Expand Up @@ -150,7 +150,7 @@
/// unsafe { msg_send_id![Self::class(), new] }
/// }
///
/// pub fn init(this: Option<Allocated<Self>>, val: usize) -> Id<Self> {
/// pub fn init(this: Allocated<Self>, val: usize) -> Id<Self> {
/// unsafe { msg_send_id![this, initWithVal: val] }
/// }
/// }
Expand Down
13 changes: 6 additions & 7 deletions crates/objc2/src/macros/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1081,12 +1081,11 @@ macro_rules! msg_send_bool {
/// return type is a generic `Id<T>` or `Option<Id<T>>`.
///
/// - The `alloc` family: The receiver must be `&AnyClass`, and the return
/// type is a generic `Allocated<T>` or `Option<Allocated<T>>`.
/// type is a generic `Allocated<T>`.
///
/// - The `init` family: The receiver must be `Option<Allocated<T>>` as
/// returned from `alloc`. The receiver is consumed, and a the
/// now-initialized `Id<T>` or `Option<Id<T>>` (with the same `T`) is
/// returned.
/// - The `init` family: The receiver must be `Allocated<T>` as returned from
/// `alloc`. The receiver is consumed, and a the now-initialized `Id<T>` or
/// `Option<Id<T>>` (with the same `T`) is returned.
///
/// - The `copy` family: The receiver may be anything that implements
/// [`MessageReceiver`] and the return type is a generic `Id<T>` or
Expand Down Expand Up @@ -1276,7 +1275,7 @@ macro_rules! __msg_send_id_helper {
($($selector:tt)*)
($($argument:expr,)*)
} => ({
<$crate::__macro_helpers::$retain_semantics as $crate::__macro_helpers::MsgSendId<_, _>>::$fn::<_, _>(
<$crate::__macro_helpers::$retain_semantics as $crate::__macro_helpers::MsgSendId<_, _>>::$fn(
$obj,
$crate::sel!($($selector)*),
($($argument,)*),
Expand All @@ -1296,7 +1295,7 @@ macro_rules! __msg_send_id_helper {
let result;
result = <$crate::__macro_helpers::RetainSemantics<{
$crate::__macro_helpers::retain_semantics(__SELECTOR_DATA)
}> as $crate::__macro_helpers::MsgSendId<_, _>>::$fn::<_, _>(
}> as $crate::__macro_helpers::MsgSendId<_, _>>::$fn(
$obj,
$crate::__sel_inner!(
__SELECTOR_DATA,
Expand Down
Loading