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

Remove non-interior mutability from non-generic types #633

Merged
merged 5 commits into from
Jun 20, 2024
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
4 changes: 2 additions & 2 deletions crates/header-translator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ specific API!
3. If the method can throw an exception if provided with invalid inputs, it is
not safe. Consider declaring a helper method that checks the preconditions
first!
4. Beware of `Mutable` classes (e.g. `NSMutableString`); these usually need to
be passed as `&mut T`, or operate on `&mut self`.
4. Beware of `Mutable` classes; these usually need to be passed as `&mut T`, or
operate on `&mut self`.

Note: It is _not_ considered a breaking change for a method to be marked safe,
so such an improvement can be made in a minor version!
18 changes: 18 additions & 0 deletions crates/header-translator/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,24 @@ impl<'de> Deserialize<'de> for Mutability {
return Ok(Mutability::MutableWithImmutableSuperclass(item));
}

if let Some(value) = value.strip_prefix("InteriorMutableWithSubclass(") {
let value = value
.strip_suffix(')')
.ok_or(de::Error::custom("end parenthesis"))?;
let item =
parse_itemidentifier(value).ok_or(de::Error::custom("requires ::"))?;
return Ok(Mutability::InteriorMutableWithSubclass(item));
}

if let Some(value) = value.strip_prefix("InteriorMutableWithSuperclass(") {
let value = value
.strip_suffix(')')
.ok_or(de::Error::custom("end parenthesis"))?;
let item =
parse_itemidentifier(value).ok_or(de::Error::custom("requires ::"))?;
return Ok(Mutability::InteriorMutableWithSuperclass(item));
}

match value {
"Immutable" => Ok(Mutability::Immutable),
"Mutable" => Ok(Mutability::Mutable),
Expand Down
17 changes: 14 additions & 3 deletions crates/header-translator/src/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,10 @@ pub(crate) fn items_required_by_decl(
for (superclass, _, _) in parse_superclasses(entity, context) {
items.push(superclass);
}
if let Some(Mutability::ImmutableWithMutableSubclass(subclass)) =
data.map(|data| &data.mutability)
if let Some(
Mutability::ImmutableWithMutableSubclass(subclass)
| Mutability::InteriorMutableWithSubclass(subclass),
) = data.map(|data| &data.mutability)
{
items.push(subclass.clone());
}
Expand Down Expand Up @@ -391,6 +393,8 @@ pub enum Mutability {
MutableWithImmutableSuperclass(ItemIdentifier),
#[default]
InteriorMutable,
InteriorMutableWithSubclass(ItemIdentifier),
InteriorMutableWithSuperclass(ItemIdentifier),
MainThreadOnly,
}

Expand All @@ -415,6 +419,12 @@ impl fmt::Display for Mutability {
write!(f, "MutableWithImmutableSuperclass<{}>", superclass.path())
}
Self::InteriorMutable => write!(f, "InteriorMutable"),
Self::InteriorMutableWithSubclass(subclass) => {
write!(f, "InteriorMutableWithSubclass<{}>", subclass.path())
}
Self::InteriorMutableWithSuperclass(superclass) => {
write!(f, "InteriorMutableWithSuperclass<{}>", superclass.path())
}
Self::MainThreadOnly => write!(f, "MainThreadOnly"),
}
}
Expand Down Expand Up @@ -861,7 +871,8 @@ impl Stmt {
);
}

let extra_methods = if let Mutability::ImmutableWithMutableSubclass(subclass) =
let extra_methods = if let Mutability::ImmutableWithMutableSubclass(subclass)
| Mutability::InteriorMutableWithSubclass(subclass) =
data.map(|data| data.mutability.clone()).unwrap_or_default()
{
let subclass_data = context
Expand Down
2 changes: 2 additions & 0 deletions crates/objc2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
* Added the feature flag `"relax-sign-encoding"`, which when enabled, allows
using e.g. `NSInteger` in places where you would otherwise have to use
`NSUInteger`.
* Added new mutability options `InteriorMutableWithSubclass` and
`InteriorMutableWithSuperclass`.

### Changed
* Renamed `Id` to `Retained`, to better reflect what it represents.
Expand Down
5 changes: 3 additions & 2 deletions crates/objc2/src/__framework_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ pub use std::os::raw::{
pub use crate::encode::{Encode, Encoding, RefEncode};
pub use crate::ffi::{NSInteger, NSIntegerMax, NSUInteger, NSUIntegerMax, IMP};
pub use crate::mutability::{
Immutable, ImmutableWithMutableSubclass, InteriorMutable, IsIdCloneable, IsMainThreadOnly,
IsRetainable, MainThreadOnly, Mutable, MutableWithImmutableSuperclass,
Immutable, ImmutableWithMutableSubclass, InteriorMutable, InteriorMutableWithSubclass,
InteriorMutableWithSuperclass, IsIdCloneable, IsMainThreadOnly, IsRetainable, MainThreadOnly,
Mutable, MutableWithImmutableSuperclass,
};
pub use crate::rc::{Allocated, DefaultId, DefaultRetained, Id, Retained};
pub use crate::runtime::{
Expand Down
26 changes: 26 additions & 0 deletions crates/objc2/src/__macro_helpers/declare_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,13 @@ where
{
}
impl ValidSubclassMutability<mutability::InteriorMutable> for mutability::Root {}
impl<MS, IS> ValidSubclassMutability<mutability::InteriorMutableWithSubclass<MS>>
for mutability::Root
where
MS: ?Sized + ClassType<Mutability = mutability::InteriorMutableWithSuperclass<IS>>,
IS: ?Sized + ClassType<Mutability = mutability::InteriorMutableWithSubclass<MS>>,
{
}
impl ValidSubclassMutability<mutability::MainThreadOnly> for mutability::Root {}

// Immutable
Expand Down Expand Up @@ -182,6 +189,25 @@ impl<IS: ?Sized + ClassType> ValidSubclassMutability<mutability::Mutable>
impl ValidSubclassMutability<mutability::InteriorMutable> for mutability::InteriorMutable {}
impl ValidSubclassMutability<mutability::MainThreadOnly> for mutability::InteriorMutable {}

// InteriorMutableWithSubclass
impl<MS, IS> ValidSubclassMutability<mutability::InteriorMutableWithSuperclass<IS>>
for mutability::InteriorMutableWithSubclass<MS>
where
MS: ?Sized + ClassType<Mutability = mutability::InteriorMutableWithSuperclass<IS>>,
IS: ?Sized + ClassType<Mutability = mutability::InteriorMutableWithSubclass<MS>>,
{
}
impl<S: ?Sized + ClassType> ValidSubclassMutability<mutability::InteriorMutable>
for mutability::InteriorMutableWithSubclass<S>
{
}

// InteriorMutableWithSuperclass
impl<S: ?Sized + ClassType> ValidSubclassMutability<mutability::InteriorMutable>
for mutability::InteriorMutableWithSuperclass<S>
{
}

// MainThreadOnly
impl ValidSubclassMutability<mutability::MainThreadOnly> for mutability::MainThreadOnly {}

Expand Down
14 changes: 7 additions & 7 deletions crates/objc2/src/macros/extern_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,14 +456,14 @@ macro_rules! __extern_class_impl_traits {
// consideration, the lifetime of `&mut Self::Target` is still tied to
// `&mut self`.
//
// Usually we don't want to allow `&mut` of immutable objects like
// `NSString`, because their `NSCopying` implementation returns the
// same object, and would violate aliasing rules.
// Usually we don't want to allow `&mut` of immutable objects, because
// their `NSCopying` implementation returns the same object, and that
// would violate aliasing rules.
//
// But `&mut NSMutableString` -> `&mut NSString` safe, since the
// `NSCopying` implementation of `NSMutableString` is still used on
// the `&mut NSString`, and that is guaranteed to return a different
// object.
// But even then, `&mut MyMutableObject` -> `&mut MyObject` is still
// safe, as it's the `NSCopying` implementation of `MyMutableObject`
// that is used on the `&mut MyObject`, and that is guaranteed to
// return a different object.
$(#[$impl_m])*
impl<$($t)*> $crate::__macro_helpers::DerefMut for $for {
#[inline]
Expand Down
17 changes: 9 additions & 8 deletions crates/objc2/src/macros/extern_protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,22 @@
///
/// # Examples
///
/// Create a trait to represent the `NSItemProviderWriting` protocol.
/// Create a trait to represent the `NSItemProviderWriting` protocol (in
/// practice, you would import this from `objc2-foundation`, this is just for
/// demonstration purposes).
///
/// ```
/// use std::ffi::c_void;
/// use objc2::ffi::NSInteger;
/// use objc2::rc::Retained;
/// use objc2::runtime::{NSObject, NSObjectProtocol};
/// use objc2::{extern_protocol, ProtocolType};
///
/// // Assume these were correctly defined, as if they came from
/// // `objc2-foundation`
/// type NSArray<T> = T;
/// type NSString = NSObject;
/// type NSProgress = NSObject;
/// type NSItemProviderRepresentationVisibility = NSInteger;
/// # type NSArray<T> = T;
/// # type NSString = NSObject;
/// # type NSProgress = NSObject;
/// # type NSItemProviderRepresentationVisibility = NSInteger;
/// # #[cfg(defined_in_foundation)]
/// use objc2_foundation::{NSArray, NSString, NSProgress, NSItemProviderRepresentationVisibility};
///
/// extern_protocol!(
/// /// This comment will appear on the trait as expected.
Expand Down
128 changes: 94 additions & 34 deletions crates/objc2/src/mutability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ impl<IS: ?Sized> Mutability for MutableWithImmutableSuperclass<IS> {}
impl private_mutability::Sealed for InteriorMutable {}
impl Mutability for InteriorMutable {}

impl<S: ?Sized> private_mutability::Sealed for InteriorMutableWithSubclass<S> {}
impl<S: ?Sized> Mutability for InteriorMutableWithSubclass<S> {}

impl<S: ?Sized> private_mutability::Sealed for InteriorMutableWithSuperclass<S> {}
impl<S: ?Sized> Mutability for InteriorMutableWithSuperclass<S> {}

impl private_mutability::Sealed for MainThreadOnly {}
impl Mutability for MainThreadOnly {}

Expand Down Expand Up @@ -154,23 +160,6 @@ pub struct Mutable {
/// - [`IsAllowedMutable`].
/// - You are allowed to hand out pointers / references to an instance's
/// internal data, since you know such data will never be mutated.
///
///
/// # Example
///
/// ```ignore
/// unsafe impl ClassType for NSString {
/// type Super = NSObject;
/// type Mutability = ImmutableWithMutableSubclass<NSMutableString>;
/// // ...
/// }
///
/// unsafe impl ClassType for NSMutableString {
/// type Super = NSString;
/// type Mutability = MutableWithImmutableSubclass<NSString>;
/// // ...
/// }
/// ```
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub struct ImmutableWithMutableSubclass<MS: ?Sized> {
inner: Never,
Expand All @@ -190,23 +179,6 @@ pub struct ImmutableWithMutableSubclass<MS: ?Sized> {
/// - You are allowed to hand out pointers / references to an instance's
/// internal data, since you know such data will never be mutated without
/// the borrowchecker catching it.
///
///
/// # Example
///
/// ```ignore
/// unsafe impl ClassType for NSData {
/// type Super = NSObject;
/// type Mutability = ImmutableWithMutableSubclass<NSMutableData>;
/// // ...
/// }
///
/// unsafe impl ClassType for NSMutableData {
/// type Super = NSData;
/// type Mutability = MutableWithImmutableSubclass<NSData>;
/// // ...
/// }
/// ```
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub struct MutableWithImmutableSuperclass<IS: ?Sized> {
inner: Never,
Expand Down Expand Up @@ -241,6 +213,70 @@ pub struct InteriorMutable {
inner: Never,
}

/// Marker type for classes that have a mutable counterpart.
///
/// This is effectively the same as [`InteriorMutable`], except that the type
/// returned by `NSMutableCopying::mutableCopy` is the mutable counterpart.
///
/// Functionality that is provided with this:
/// - [`IsRetainable`] -> [`ClassType::retain`].
/// - [`IsIdCloneable`] -> [`Retained::clone`][crate::rc::Retained#impl-Clone-for-Retained<T>].
/// - [`IsAllocableAnyThread`] -> [`ClassType::alloc`].
///
///
/// # Example
///
/// ```ignore
/// unsafe impl ClassType for NSString {
/// type Super = NSObject;
/// type Mutability = InteriorMutableWithSubclass<NSMutableString>;
/// // ...
/// }
///
/// unsafe impl ClassType for NSMutableString {
/// type Super = NSString;
/// type Mutability = InteriorMutableWithSuperclass<NSString>;
/// // ...
/// }
/// ```
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub struct InteriorMutableWithSubclass<Subclass: ?Sized> {
inner: Never,
subclass: PhantomData<Subclass>,
}

/// Marker type for classes that have an immutable counterpart.
///
/// This is effectively the same as [`InteriorMutable`], except that the type
/// returned by `NSCopying::copy` is the immutable counterpart.
///
/// Functionality that is provided with this:
/// - [`IsRetainable`] -> [`ClassType::retain`].
/// - [`IsIdCloneable`] -> [`Retained::clone`][crate::rc::Retained#impl-Clone-for-Retained<T>].
/// - [`IsAllocableAnyThread`] -> [`ClassType::alloc`].

///
/// # Example
///
/// ```ignore
/// unsafe impl ClassType for NSData {
/// type Super = NSObject;
/// type Mutability = InteriorMutableWithSubclass<NSMutableData>;
/// // ...
/// }
///
/// unsafe impl ClassType for NSMutableData {
/// type Super = NSData;
/// type Mutability = InteriorMutableWithSuperclass<NSData>;
/// // ...
/// }
/// ```
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub struct InteriorMutableWithSuperclass<Superclass: ?Sized> {
inner: Never,
superclass: PhantomData<Superclass>,
}

/// Marker type for classes that are only safe to use from the main thread.
///
/// This is effectively the same as [`InteriorMutable`], except that classes
Expand Down Expand Up @@ -307,6 +343,8 @@ impl MutabilityIsIdCloneable for Root {}
impl MutabilityIsIdCloneable for Immutable {}
impl<MS: ?Sized> MutabilityIsIdCloneable for ImmutableWithMutableSubclass<MS> {}
impl MutabilityIsIdCloneable for InteriorMutable {}
impl<S: ?Sized> MutabilityIsIdCloneable for InteriorMutableWithSubclass<S> {}
impl<S: ?Sized> MutabilityIsIdCloneable for InteriorMutableWithSuperclass<S> {}
impl MutabilityIsIdCloneable for MainThreadOnly {}

unsafe impl<T: ?Sized + ClassType> IsIdCloneable for T where T::Mutability: MutabilityIsIdCloneable {}
Expand Down Expand Up @@ -340,6 +378,8 @@ pub unsafe trait IsRetainable: private_traits::Sealed + IsIdCloneable {}
trait MutabilityIsRetainable: MutabilityIsIdCloneable {}
impl MutabilityIsRetainable for Immutable {}
impl MutabilityIsRetainable for InteriorMutable {}
impl<S: ?Sized> MutabilityIsRetainable for InteriorMutableWithSubclass<S> {}
impl<S: ?Sized> MutabilityIsRetainable for InteriorMutableWithSuperclass<S> {}
impl MutabilityIsRetainable for MainThreadOnly {}

unsafe impl<T: ?Sized + ClassType> IsRetainable for T where T::Mutability: MutabilityIsRetainable {}
Expand Down Expand Up @@ -369,6 +409,8 @@ impl MutabilityIsAllocableAnyThread for Mutable {}
impl<MS: ?Sized> MutabilityIsAllocableAnyThread for ImmutableWithMutableSubclass<MS> {}
impl<IS: ?Sized> MutabilityIsAllocableAnyThread for MutableWithImmutableSuperclass<IS> {}
impl MutabilityIsAllocableAnyThread for InteriorMutable {}
impl<S: ?Sized> MutabilityIsAllocableAnyThread for InteriorMutableWithSubclass<S> {}
impl<S: ?Sized> MutabilityIsAllocableAnyThread for InteriorMutableWithSuperclass<S> {}

unsafe impl<T: ?Sized + ClassType> IsAllocableAnyThread for T where
T::Mutability: MutabilityIsAllocableAnyThread
Expand Down Expand Up @@ -600,6 +642,22 @@ mod private_counterpart {
type Immutable = T;
type Mutable = T;
}
impl<T, S> MutabilityCounterpartOrSelf<T> for InteriorMutableWithSubclass<S>
where
T: ClassType<Mutability = InteriorMutableWithSubclass<S>>,
S: ClassType<Mutability = InteriorMutableWithSuperclass<T>>,
{
type Immutable = T;
type Mutable = S;
}
impl<T, S> MutabilityCounterpartOrSelf<T> for InteriorMutableWithSuperclass<S>
where
T: ClassType<Mutability = InteriorMutableWithSuperclass<S>>,
S: ClassType<Mutability = InteriorMutableWithSubclass<T>>,
{
type Immutable = S;
type Mutable = T;
}
impl<T: ClassType<Mutability = MainThreadOnly>> MutabilityCounterpartOrSelf<T> for MainThreadOnly {
type Immutable = T;
type Mutable = T;
Expand Down Expand Up @@ -661,6 +719,8 @@ mod tests {
assert_traits::<ImmutableWithMutableSubclass<()>>();
assert_traits::<MutableWithImmutableSuperclass<()>>();
assert_traits::<InteriorMutable>();
assert_traits::<InteriorMutableWithSubclass<()>>();
assert_traits::<InteriorMutableWithSuperclass<()>>();
assert_traits::<MainThreadOnly>();

#[allow(unused)]
Expand Down
Loading