Skip to content

Commit

Permalink
Merge pull request #633 from madsmtm/remove-generic-non-mut
Browse files Browse the repository at this point in the history 
Remove non-interior mutability from non-generic types.

`NSString`/`NSAttributedString`/`NSData`.
  • Loading branch information
@madsmtm
madsmtm authored Jun 20, 2024
2 parents 47358ba + f7226f7 commit 47abe0b
Show file tree
Hide file tree
Showing 48 changed files with 680 additions and 573 deletions.
4 changes: 2 additions & 2 deletions crates/header-translator/README.md
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 47abe0b

Please sign in to comment.