Skip to content

Commit

Permalink
Merge pull request #516 from madsmtm/protocol-fixes
Browse files Browse the repository at this point in the history
Relax `ProtocolType` requirement on `ProtocolObject`
  • Loading branch information
madsmtm authored Dec 3, 2023
2 parents 810d88e + 1c663ba commit 1af01ab
Show file tree
Hide file tree
Showing 8 changed files with 450 additions and 37 deletions.
4 changes: 0 additions & 4 deletions crates/icrate/src/fixes/Foundation/copying.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ extern_protocol!(
#[optional]
fn copy(&self) -> Id<Self::Immutable>
where
Self: Sized,
Self: CounterpartOrSelf;

/// Returns a new instance that's a copy of the receiver.
Expand All @@ -37,7 +36,6 @@ extern_protocol!(
#[method_id(copyWithZone:)]
unsafe fn copyWithZone(&self, zone: *mut NSZone) -> Id<Self::Immutable>
where
Self: Sized,
Self: CounterpartOrSelf;
}

Expand Down Expand Up @@ -65,7 +63,6 @@ extern_protocol!(
#[optional]
fn mutableCopy(&self) -> Id<Self::Mutable>
where
Self: Sized,
Self: CounterpartOrSelf;

/// Returns a new instance that's a mutable copy of the receiver.
Expand All @@ -80,7 +77,6 @@ extern_protocol!(
#[method_id(mutableCopyWithZone:)]
unsafe fn mutableCopyWithZone(&self, zone: *mut NSZone) -> Id<Self::Mutable>
where
Self: Sized,
Self: CounterpartOrSelf;
}

Expand Down
5 changes: 5 additions & 0 deletions crates/objc2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
* Added `Allocated::set_ivars`, which sets the instance variables of an
object, and returns the new `rc::PartialInit`.
* Added the ability for `msg_send_id!` to call `super` methods.
* Implement `Send` and `Sync` for `ProtocolObject` if the underlying protocol
implements it.
* Added ability to create `Send` and `Sync` versions of
`ProtocolObject<dyn NSObjectProtocol>`.

### Changed
* **BREAKING**: Changed how instance variables work in `declare_class!`.
Expand Down Expand Up @@ -149,6 +153,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
* **BREAKING**: Make `rc::Allocated` allowed to be `NULL` internally, such
that uses of `Option<Allocated<T>>` is now simply `Allocated<T>`.
* `AnyObject::class` now returns a `'static` reference to the class.
* Relaxed `ProtocolType` requirement on `ProtocolObject`.

### Deprecated
* Soft deprecated using `msg_send!` without a comma between arguments (i.e.
Expand Down
5 changes: 4 additions & 1 deletion crates/objc2/src/macros/extern_protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ macro_rules! __inner_extern_protocol {
$(#[$impl_m])*
unsafe impl<T> $name for $crate::runtime::ProtocolObject<T>
where
T: ?$crate::__macro_helpers::Sized + $crate::ProtocolType + $name
T: ?$crate::__macro_helpers::Sized + $name
{}

// SAFETY: The specified name is ensured by caller to be a protocol,
Expand All @@ -204,6 +204,9 @@ macro_rules! __inner_extern_protocol {
{
const __INNER: () = ();
}

// TODO: Should we also implement `ImplementedBy` for `Send + Sync`
// types, as is done for `NSObjectProtocol`?
};
}

Expand Down
23 changes: 10 additions & 13 deletions crates/objc2/src/mutability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
use core::marker::PhantomData;

use crate::runtime::{AnyObject, ProtocolObject};
use crate::{ClassType, Message, ProtocolType};
use crate::{ClassType, Message};

mod private_mutability {
pub trait Sealed {}
Expand Down Expand Up @@ -269,7 +269,7 @@ mod private_traits {
}

impl<T: ?Sized + ClassType> private_traits::Sealed for T {}
impl<P: ?Sized + ProtocolType> private_traits::Sealed for ProtocolObject<P> {}
impl<P: ?Sized> private_traits::Sealed for ProtocolObject<P> {}
impl private_traits::Sealed for AnyObject {}

/// Marker trait for classes where [`Id::clone`] is safe.
Expand Down Expand Up @@ -303,7 +303,7 @@ impl MutabilityIsIdCloneable for InteriorMutable {}
impl MutabilityIsIdCloneable for MainThreadOnly {}

unsafe impl<T: ?Sized + ClassType> IsIdCloneable for T where T::Mutability: MutabilityIsIdCloneable {}
unsafe impl<P: ?Sized + ProtocolType + IsIdCloneable> IsIdCloneable for ProtocolObject<P> {}
unsafe impl<P: ?Sized + IsIdCloneable> IsIdCloneable for ProtocolObject<P> {}
// SAFETY: Same as for root classes.
unsafe impl IsIdCloneable for AnyObject {}

Expand Down Expand Up @@ -336,7 +336,7 @@ impl MutabilityIsRetainable for InteriorMutable {}
impl MutabilityIsRetainable for MainThreadOnly {}

unsafe impl<T: ?Sized + ClassType> IsRetainable for T where T::Mutability: MutabilityIsRetainable {}
unsafe impl<P: ?Sized + ProtocolType + IsRetainable> IsRetainable for ProtocolObject<P> {}
unsafe impl<P: ?Sized + IsRetainable> IsRetainable for ProtocolObject<P> {}

/// Marker trait for classes that can be allocated from any thread.
///
Expand Down Expand Up @@ -367,10 +367,7 @@ unsafe impl<T: ?Sized + ClassType> IsAllocableAnyThread for T where
T::Mutability: MutabilityIsAllocableAnyThread
{
}
unsafe impl<P: ?Sized + ProtocolType + IsAllocableAnyThread> IsAllocableAnyThread
for ProtocolObject<P>
{
}
unsafe impl<P: ?Sized + IsAllocableAnyThread> IsAllocableAnyThread for ProtocolObject<P> {}

/// Marker trait for classes that may feasibly be used behind a mutable
/// reference.
Expand Down Expand Up @@ -401,7 +398,7 @@ unsafe impl<T: ?Sized + ClassType> IsAllowedMutable for T where
T::Mutability: MutabilityIsAllowedMutable
{
}
unsafe impl<P: ?Sized + ProtocolType + IsAllowedMutable> IsAllowedMutable for ProtocolObject<P> {}
unsafe impl<P: ?Sized + IsAllowedMutable> IsAllowedMutable for ProtocolObject<P> {}
// SAFETY: Same as for root classes.
unsafe impl IsAllowedMutable for AnyObject {}

Expand Down Expand Up @@ -430,7 +427,7 @@ impl MutabilityIsMutable for Mutable {}
impl<IS: ?Sized> MutabilityIsMutable for MutableWithImmutableSuperclass<IS> {}

unsafe impl<T: ?Sized + ClassType> IsMutable for T where T::Mutability: MutabilityIsMutable {}
unsafe impl<P: ?Sized + ProtocolType + IsMutable> IsMutable for ProtocolObject<P> {}
unsafe impl<P: ?Sized + IsMutable> IsMutable for ProtocolObject<P> {}

/// Marker trait for classes that are only available on the main thread.
///
Expand All @@ -455,7 +452,7 @@ unsafe impl<T: ?Sized + ClassType> IsMainThreadOnly for T where
T::Mutability: MutabilityIsMainThreadOnly
{
}
unsafe impl<P: ?Sized + ProtocolType + IsMainThreadOnly> IsMainThreadOnly for ProtocolObject<P> {}
unsafe impl<P: ?Sized + IsMainThreadOnly> IsMainThreadOnly for ProtocolObject<P> {}

/// Marker trait for classes whose `hash` and `isEqual:` methods are stable.
///
Expand Down Expand Up @@ -489,7 +486,7 @@ impl<MS: ?Sized> MutabilityHashIsStable for ImmutableWithMutableSubclass<MS> {}
impl<IS: ?Sized> MutabilityHashIsStable for MutableWithImmutableSuperclass<IS> {}

unsafe impl<T: ?Sized + ClassType> HasStableHash for T where T::Mutability: MutabilityHashIsStable {}
unsafe impl<P: ?Sized + ProtocolType + HasStableHash> HasStableHash for ProtocolObject<P> {}
unsafe impl<P: ?Sized + HasStableHash> HasStableHash for ProtocolObject<P> {}

/// Retrieve the immutable/mutable counterpart class, and fall back to `Self`
/// if not applicable.
Expand Down Expand Up @@ -589,7 +586,7 @@ where
type Mutable = <T::Mutability as private_counterpart::MutabilityCounterpartOrSelf<T>>::Mutable;
}

unsafe impl<P: ?Sized + ProtocolType> CounterpartOrSelf for ProtocolObject<P> {
unsafe impl<P: ?Sized> CounterpartOrSelf for ProtocolObject<P> {
// SAFETY: The only place where this would differ from `Self` is for
// classes with `MutableWithImmutableSuperclass<IS>`.
//
Expand Down
29 changes: 29 additions & 0 deletions crates/objc2/src/runtime/nsobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use crate::runtime::{AnyClass, AnyObject, ProtocolObject};
use crate::{extern_methods, msg_send, msg_send_id, Message};
use crate::{ClassType, ProtocolType};

use super::ImplementedBy;

/// The root class of most Objective-C class hierarchies.
///
/// This represents the [`NSObject` class][cls]. The name "NSObject" also
Expand Down Expand Up @@ -163,6 +165,33 @@ crate::__inner_extern_protocol!(
("NSObject")
);

// SAFETY: Anything that implements `NSObjectProtocol` and is `Send` is valid
// to convert to `ProtocolObject<dyn NSObjectProtocol + Send>`.
unsafe impl<T> ImplementedBy<T> for dyn NSObjectProtocol + Send
where
T: ?Sized + Message + NSObjectProtocol + Send,
{
const __INNER: () = ();
}

// SAFETY: Anything that implements `NSObjectProtocol` and is `Sync` is valid
// to convert to `ProtocolObject<dyn NSObjectProtocol + Sync>`.
unsafe impl<T> ImplementedBy<T> for dyn NSObjectProtocol + Sync
where
T: ?Sized + Message + NSObjectProtocol + Sync,
{
const __INNER: () = ();
}

// SAFETY: Anything that implements `NSObjectProtocol` and is `Send + Sync` is
// valid to convert to `ProtocolObject<dyn NSObjectProtocol + Send + Sync>`.
unsafe impl<T> ImplementedBy<T> for dyn NSObjectProtocol + Send + Sync
where
T: ?Sized + Message + NSObjectProtocol + Send + Sync,
{
const __INNER: () = ();
}

unsafe impl NSObjectProtocol for NSObject {}

extern_methods!(
Expand Down
61 changes: 42 additions & 19 deletions crates/objc2/src/runtime/protocol_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::encode::{Encoding, RefEncode};
use crate::rc::{autoreleasepool_leaking, Id};
use crate::runtime::__nsstring::nsstring_to_str;
use crate::runtime::{AnyObject, NSObjectProtocol};
use crate::{Message, ProtocolType};
use crate::Message;

/// An internal helper trait for [`ProtocolObject`].
///
Expand Down Expand Up @@ -57,25 +57,35 @@ pub unsafe trait ImplementedBy<T: ?Sized + Message> {
/// ```
#[doc(alias = "id")]
#[repr(C)]
pub struct ProtocolObject<P: ?Sized + ProtocolType> {
pub struct ProtocolObject<P: ?Sized> {
inner: AnyObject,
p: PhantomData<P>,
}

// SAFETY: `Send` if the underlying trait promises `Send`.
//
// E.g. `ProtocolObject<dyn NSObjectProtocol + Send>` is naturally `Send`.
unsafe impl<P: ?Sized + Send> Send for ProtocolObject<P> {}

// SAFETY: `Sync` if the underlying trait promises `Sync`.
//
// E.g. `ProtocolObject<dyn NSObjectProtocol + Sync>` is naturally `Sync`.
unsafe impl<P: ?Sized + Sync> Sync for ProtocolObject<P> {}

// SAFETY: The type is `#[repr(C)]` and `AnyObject` internally
unsafe impl<P: ?Sized + ProtocolType> RefEncode for ProtocolObject<P> {
unsafe impl<P: ?Sized> RefEncode for ProtocolObject<P> {
const ENCODING_REF: Encoding = Encoding::Object;
}

// SAFETY: The type is `AnyObject` internally, and is mean to be messaged
// as-if it's an object.
unsafe impl<P: ?Sized + ProtocolType> Message for ProtocolObject<P> {}
unsafe impl<P: ?Sized> Message for ProtocolObject<P> {}

impl<P: ?Sized + ProtocolType> ProtocolObject<P> {
impl<P: ?Sized> ProtocolObject<P> {
/// Get an immutable type-erased reference from a type implementing a
/// protocol.
#[inline]
pub fn from_ref<T: Message>(obj: &T) -> &Self
pub fn from_ref<T: ?Sized + Message>(obj: &T) -> &Self
where
P: ImplementedBy<T>,
{
Expand All @@ -89,7 +99,7 @@ impl<P: ?Sized + ProtocolType> ProtocolObject<P> {
/// Get a mutable type-erased reference from a type implementing a
/// protocol.
#[inline]
pub fn from_mut<T: Message>(obj: &mut T) -> &mut Self
pub fn from_mut<T: ?Sized + Message>(obj: &mut T) -> &mut Self
where
P: ImplementedBy<T>,
{
Expand Down Expand Up @@ -118,24 +128,24 @@ impl<P: ?Sized + ProtocolType> ProtocolObject<P> {
}
}

impl<P: ?Sized + ProtocolType + NSObjectProtocol> PartialEq for ProtocolObject<P> {
impl<P: ?Sized + NSObjectProtocol> PartialEq for ProtocolObject<P> {
#[inline]
#[doc(alias = "isEqual:")]
fn eq(&self, other: &Self) -> bool {
self.__isEqual(other)
}
}

impl<P: ?Sized + ProtocolType + NSObjectProtocol> Eq for ProtocolObject<P> {}
impl<P: ?Sized + NSObjectProtocol> Eq for ProtocolObject<P> {}

impl<P: ?Sized + ProtocolType + NSObjectProtocol> hash::Hash for ProtocolObject<P> {
impl<P: ?Sized + NSObjectProtocol> hash::Hash for ProtocolObject<P> {
#[inline]
fn hash<H: hash::Hasher>(&self, state: &mut H) {
self.__hash().hash(state);
}
}

impl<P: ?Sized + ProtocolType + NSObjectProtocol> fmt::Debug for ProtocolObject<P> {
impl<P: ?Sized + NSObjectProtocol> fmt::Debug for ProtocolObject<P> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// Attempt to format description string
if let Some(description) = self.__description() {
Expand All @@ -159,29 +169,27 @@ impl<P: ?Sized + ProtocolType + NSObjectProtocol> fmt::Debug for ProtocolObject<
}
}

impl<P, T> AsRef<ProtocolObject<T>> for ProtocolObject<P>
impl<P: ?Sized, T> AsRef<ProtocolObject<T>> for ProtocolObject<P>
where
P: ?Sized + ProtocolType,
T: ?Sized + ProtocolType + ImplementedBy<ProtocolObject<P>>,
T: ?Sized + ImplementedBy<ProtocolObject<P>>,
{
#[inline]
fn as_ref(&self) -> &ProtocolObject<T> {
ProtocolObject::from_ref(self)
}
}

impl<P, T> AsMut<ProtocolObject<T>> for ProtocolObject<P>
impl<P: ?Sized, T> AsMut<ProtocolObject<T>> for ProtocolObject<P>
where
P: ?Sized + ProtocolType,
T: ?Sized + ProtocolType + ImplementedBy<ProtocolObject<P>>,
T: ?Sized + ImplementedBy<ProtocolObject<P>>,
{
#[inline]
fn as_mut(&mut self) -> &mut ProtocolObject<T> {
ProtocolObject::from_mut(self)
}
}

// TODO: Maybe iplement Borrow + BorrowMut?
// TODO: Maybe implement Borrow + BorrowMut?

#[cfg(test)]
#[allow(clippy::missing_safety_doc)]
Expand All @@ -194,7 +202,9 @@ mod tests {
use super::*;
use crate::mutability::Mutable;
use crate::runtime::{NSObject, NSObjectProtocol};
use crate::{declare_class, extern_methods, extern_protocol, ClassType, DeclaredClass};
use crate::{
declare_class, extern_methods, extern_protocol, ClassType, DeclaredClass, ProtocolType,
};

extern_protocol!(
unsafe trait Foo {
Expand Down Expand Up @@ -264,6 +274,9 @@ mod tests {
unsafe impl FooBar for DummyClass {}
// unsafe impl FooFooBar for DummyClass {}

unsafe impl Send for DummyClass {}
unsafe impl Sync for DummyClass {}

extern_methods!(
unsafe impl DummyClass {
#[method_id(new)]
Expand All @@ -275,6 +288,12 @@ mod tests {
fn impl_traits() {
assert_impl_all!(NSObject: NSObjectProtocol);
assert_impl_all!(ProtocolObject<dyn NSObjectProtocol>: NSObjectProtocol);
assert_not_impl_any!(ProtocolObject<dyn NSObjectProtocol>: Send, Sync);
assert_impl_all!(ProtocolObject<dyn NSObjectProtocol + Send>: NSObjectProtocol, Send);
assert_not_impl_any!(ProtocolObject<dyn NSObjectProtocol + Send>: Sync);
assert_impl_all!(ProtocolObject<dyn NSObjectProtocol + Sync>: NSObjectProtocol, Sync);
assert_not_impl_any!(ProtocolObject<dyn NSObjectProtocol + Sync>: Send);
assert_impl_all!(ProtocolObject<dyn NSObjectProtocol + Send + Sync>: NSObjectProtocol, Send, Sync);
assert_not_impl_any!(ProtocolObject<dyn Foo>: NSObjectProtocol);
assert_impl_all!(ProtocolObject<dyn Bar>: NSObjectProtocol);
assert_impl_all!(ProtocolObject<dyn FooBar>: NSObjectProtocol);
Expand Down Expand Up @@ -332,6 +351,10 @@ mod tests {
let _nsobject: &ProtocolObject<dyn NSObjectProtocol> = ProtocolObject::from_ref(bar);
let nsobject: &ProtocolObject<dyn NSObjectProtocol> = ProtocolObject::from_ref(&*obj);
let _nsobject: &ProtocolObject<dyn NSObjectProtocol> = ProtocolObject::from_ref(nsobject);
let _: &ProtocolObject<dyn NSObjectProtocol + Send> = ProtocolObject::from_ref(&*obj);
let _: &ProtocolObject<dyn NSObjectProtocol + Sync> = ProtocolObject::from_ref(&*obj);
let _: &ProtocolObject<dyn NSObjectProtocol + Send + Sync> =
ProtocolObject::from_ref(&*obj);

let _foobar: &mut ProtocolObject<dyn FooBar> = ProtocolObject::from_mut(&mut *obj);
let _foobar: Id<ProtocolObject<dyn FooBar>> = ProtocolObject::from_id(obj);
Expand Down
Loading

0 comments on commit 1af01ab

Please sign in to comment.