Skip to content

Commit

Permalink
Completely remove the ability to create mutable objects
Browse files Browse the repository at this point in the history
The ability to specify whether an object is thread-safe or not has been
kept, `ClassType::Mutability` has been renamed to `ThreadKind` and used
in that way instead. It can be omitted from `extern_class!` most of the
time, since we can make it default to that of the superclass.

Finally fixes #563
  • Loading branch information
madsmtm committed Sep 9, 2024
1 parent 25a6dd6 commit 7678c56
Show file tree
Hide file tree
Showing 118 changed files with 1,089 additions and 976 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ However, that just pushes the burden onto you, the user, and then we're not
much better off!

As such, we'll try to be as safe and idiomatic as possible; using references
instead of pointers to represent objects and their mutability, `Option`
instead of pointers to represent objects and their (interior) mutability, `Option`
instead of `null`, doing memory management automatically instead of manually,
and so on (see again [these notes on "Layered Safety"][layered-safety]). These
abstractions should ideally be zero-cost, but this is of course a balancing
Expand Down
1 change: 0 additions & 1 deletion crates/block2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@
//! #
//! # unsafe impl ClassType for MyClass {
//! # type Super = objc2::runtime::NSObject;
//! # type Mutability = objc2::mutability::InteriorMutable;
//! # const NAME: &'static str = "NSObject";
//! # }
//! # );
Expand Down
2 changes: 1 addition & 1 deletion crates/header-translator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Feel free to open a half-finished PR if you need assistance.

The `translation-config.toml` file describes various tweaks that we need to do because our header translation is incomplete in some areas.

However, even if our header translation was perfect, we still need a way to enrich the generated data, since C headers have no way to describe which methods are safe, or mutable, and which are not!
However, even if our header translation was perfect, we still need a way to enrich the generated data, since C headers have no way to describe which methods are safe and which are not!


### What is required for a method to be safe?
Expand Down
8 changes: 4 additions & 4 deletions crates/header-translator/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,13 +335,13 @@ impl<'de> Deserialize<'de> for Counterpart {
))
}

struct MutabilityVisitor;
struct CounterpartVisitor;

impl<'de> de::Visitor<'de> for MutabilityVisitor {
impl<'de> de::Visitor<'de> for CounterpartVisitor {
type Value = Counterpart;

fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
formatter.write_str("mutability")
formatter.write_str("counterpart")
}

fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
Expand Down Expand Up @@ -370,6 +370,6 @@ impl<'de> Deserialize<'de> for Counterpart {
}
}

deserializer.deserialize_str(MutabilityVisitor)
deserializer.deserialize_str(CounterpartVisitor)
}
}
2 changes: 1 addition & 1 deletion crates/header-translator/src/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ fn mainthreadonly_override<'a>(
}

if any_argument_provides_mainthreadmarker {
// MainThreadMarker can be retrieved via. `IsMainThreadOnly::mtm`
// MainThreadMarker can be retrieved via. `MainThreadOnly::mtm`
// inside these methods, and hence passing it is redundant.
false
} else if result_type_requires_mainthreadmarker {
Expand Down
8 changes: 3 additions & 5 deletions crates/header-translator/src/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ impl Stmt {
superclasses,
designated_initializers,
derives: data.map(|data| data.derives.clone()).unwrap_or_default(),
main_thread_only: thread_safety.inferred_mainthreadonly(),
main_thread_only: thread_safety.explicit_mainthreadonly(),
skipped: data.map(|data| data.definition_skipped).unwrap_or_default(),
// Ignore sendability on superclasses; since it's an auto
// trait, it's propagated to subclasses anyhow!
Expand Down Expand Up @@ -1724,9 +1724,7 @@ impl Stmt {
GenericTyHelper(superclass_generics),
)?;
if *main_thread_only {
writeln!(f, " type Mutability = MainThreadOnly;")?;
} else {
writeln!(f, " type Mutability = InteriorMutable;")?;
writeln!(f, " type ThreadKind = dyn MainThreadOnly;")?;
}
if !generics.is_empty() {
writeln!(f)?;
Expand Down Expand Up @@ -2054,7 +2052,7 @@ impl Stmt {
} else {
write!(f, "+ ")?;
}
write!(f, "IsMainThreadOnly")?;
write!(f, "MainThreadOnly")?;
}
writeln!(f, " {{")?;

Expand Down
64 changes: 58 additions & 6 deletions crates/objc2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,62 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
* Added `IsMainThreadOnly::mtm`.

### Changed
* **BREAKING**: Changed how you specify a class to only be available on the
main thread. It is now automatically inferred, and you only need to
overwrite it if your class is doing something different than its superclass.

```rust
// Before
declare_class!(
struct MyObject;

unsafe impl ClassType for MyObject {
type Super = NSObject;
type Mutability = InteriorMutable;
const NAME: &'static str = "MyObject";
}

impl DeclaredClass for MyObject {
type Ivars = ...;
}

// ...
);

// After
declare_class!(
struct MyObject;

unsafe impl ClassType for MyObject {
type Super = NSObject;
// No need to specify mutability any more
//
// But if you need it (e.g. when implementing delegates), you can add:
// type ThreadKind = dyn MainThreadOnly;
const NAME: &'static str = "MyObject";
}

impl DeclaredClass for MyObject {
type Ivars = ...;
}

// ...
);
```
* **BREAKING**: Moved the common `retain` and `alloc` methods from `ClassType`
to `Message` and `AllocAnyThread`/`MainThreadOnly`, respectively.

```rust
// Before
use objc2::ClassType;
let my_obj = MyObject::init(MyObject::alloc());
let retained = my_obj.retain();

// After
use objc2::{Message, AllocAnyThread}; // Need different trait imports
let my_obj = MyObject::init(MyObject::alloc());
let retained = my_obj.retain();
```
* Print backtrace when catching exceptions with the `"catch-all"` feature.
* Changed the return value of `ClassBuilder::add_protocol` to indicate whether
the protocol was already present on the class or not.
Expand Down Expand Up @@ -54,14 +110,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
`objc`).
* **BREAKING**: Removed `AsMut`, `BorrowMut` and `DerefMut` implementations in
`extern_class!` and `declare_class!`.
* **BREAKING**: Removed `mutability` options `Immutable`, `Mutable`,
`ImmutableWithMutableSubclass`, `MutableWithImmutableSuperclass`,
`InteriorMutableWithSubclass` and `InteriorMutableWithSuperclass`, along
with the related `IsAllowedMutable`, `CounterpartOrSelf`, `IsIdCloneable`,
`IsRetainable` and `IsMutable` traits.
* **BREAKING**: Removed the `mutability` module, and everything within.
Classes now always use interior mutability.
* **BREAKING**: Removed `DerefMut` implementation for `Retained<T>` when the
`Retained` was mutable.
* **BREAKING**: Moved `retain` from `ClassType` to `Message`.

### Fixed
* Remove an incorrect assertion when adding protocols to classes in an unexpected
Expand Down
1 change: 1 addition & 0 deletions crates/objc2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ memoffset = "0.9.0"
block2 = { path = "../block2", default-features = false }
objc2-foundation = { path = "../../framework-crates/objc2-foundation", default-features = false, features = [
"NSDictionary",
"NSGeometry",
"NSKeyValueObserving",
"NSObject",
"NSString",
Expand Down
3 changes: 1 addition & 2 deletions crates/objc2/examples/class_with_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use std::marker::PhantomData;
use std::sync::Once;

use objc2::msg_send_id;
use objc2::mutability::InteriorMutable;
use objc2::rc::Retained;
use objc2::runtime::{AnyClass, ClassBuilder, NSObject};
use objc2::{ClassType, Encoding, Message, RefEncode};
Expand Down Expand Up @@ -60,7 +59,7 @@ impl<'a> MyObject<'a> {

unsafe impl<'a> ClassType for MyObject<'a> {
type Super = NSObject;
type Mutability = InteriorMutable;
type ThreadKind = <Self::Super as ClassType>::ThreadKind;
const NAME: &'static str = "MyObject";

fn class() -> &'static AnyClass {
Expand Down
5 changes: 2 additions & 3 deletions crates/objc2/src/__framework_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,13 @@ pub use std::os::raw::{

pub use crate::encode::{Encode, Encoding, RefEncode};
pub use crate::ffi::{NSInteger, NSIntegerMax, NSUInteger, NSUIntegerMax};
pub use crate::mutability::{InteriorMutable, IsMainThreadOnly, MainThreadOnly};
pub use crate::rc::{Allocated, DefaultId, DefaultRetained, Id, Retained};
pub use crate::runtime::{
AnyClass, AnyObject, Bool, NSObject, NSObjectProtocol, ProtocolObject, Sel,
};
pub use crate::{
__inner_extern_class, extern_category, extern_class, extern_methods, extern_protocol,
ClassType, MainThreadMarker, Message, ProtocolType,
MainThreadOnly, __inner_extern_class, extern_category, extern_class, extern_methods,
extern_protocol, ClassType, MainThreadMarker, Message, ProtocolType,
};

// TODO
Expand Down
107 changes: 107 additions & 0 deletions crates/objc2/src/__macro_helpers/class.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
use crate::{AllocAnyThread, ClassType, MainThreadOnly, ThreadKind};

/// Helper for ensuring that `ClassType::ThreadKind`, if specified, is set
/// correctly.
pub trait ValidThreadKind<Requested: ?Sized + ThreadKind>
where
Self: ClassType<ThreadKind = Requested>,
// Ensure the user did not attempt to declare or define a root class.
Self::Super: ClassType,
{
// Required to reference the trait.
fn check() {}
}

/// Always allow setting `MainThreadOnly`.
impl<'a, Cls> ValidThreadKind<dyn MainThreadOnly + 'a> for Cls
where
Self: ClassType<ThreadKind = dyn MainThreadOnly + 'a>,
Self::Super: ClassType,
{
}

/// But restrict `AllocAnyThread` to only if the superclass also sets it.
impl<'a, 'b, Cls> ValidThreadKind<dyn AllocAnyThread + 'a> for Cls
where
Self: ClassType<ThreadKind = dyn AllocAnyThread + 'a>,
Self::Super: ClassType<ThreadKind = dyn AllocAnyThread + 'b>,
{
}

/// Check that `MainThreadOnly` types do not implement `Send` and `Sync`.
///
/// Check implemented using type inference:
/// let _ = <MyType as MainThreadOnlyDoesNotImplSendSync<_>>::check
pub trait MainThreadOnlyDoesNotImplSendSync<Inferred> {
// Required to reference the trait.
fn check() {}
}

// Type inference will find this blanket impl...
impl<Cls: ?Sized> MainThreadOnlyDoesNotImplSendSync<()> for Cls {}

// ... unless one of these impls also apply, then type inference fails.
struct ImplsSend;
impl<Cls: ?Sized + MainThreadOnly + Send> MainThreadOnlyDoesNotImplSendSync<ImplsSend> for Cls {}

struct ImplsSync;
impl<Cls: ?Sized + MainThreadOnly + Sync> MainThreadOnlyDoesNotImplSendSync<ImplsSync> for Cls {}

#[cfg(test)]
mod tests {
use super::*;
use crate::extern_class;
use crate::runtime::NSObject;

extern_class!(
struct SetAnyThread;

unsafe impl ClassType for SetAnyThread {
type Super = NSObject;
type ThreadKind = dyn AllocAnyThread;
const NAME: &'static str = "NSObject";
}
);

extern_class!(
struct SendSync;

unsafe impl ClassType for SendSync {
type Super = NSObject;
type ThreadKind = dyn AllocAnyThread;
const NAME: &'static str = "NSObject";
}
);

unsafe impl Send for SendSync {}
unsafe impl Sync for SendSync {}

extern_class!(
struct OnlyMain;

unsafe impl ClassType for OnlyMain {
type Super = NSObject;
type ThreadKind = dyn MainThreadOnly;
const NAME: &'static str = "NSObject";
}
);

extern_class!(
struct OnlyMainSubDefault;

unsafe impl ClassType for OnlyMainSubDefault {
type Super = OnlyMain;
const NAME: &'static str = "NSObject";
}
);

extern_class!(
struct OnlyMainSubExplicit;

unsafe impl ClassType for OnlyMainSubExplicit {
type Super = OnlyMain;
type ThreadKind = dyn MainThreadOnly;
const NAME: &'static str = "NSObject";
}
);
}
32 changes: 0 additions & 32 deletions crates/objc2/src/__macro_helpers/declare_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use crate::{ClassType, DeclaredClass, Message, ProtocolType};

use super::declared_ivars::{register_with_ivars, setup_dealloc};
use super::{CopyOrMutCopy, Init, MaybeUnwrap, New, Other};
use crate::mutability;

/// Helper type for implementing `MethodImplementation` with a receiver of
/// `Allocated<T>`, without exposing that implementation to users.
Expand Down Expand Up @@ -134,37 +133,6 @@ impl<T: Message> MaybeOptionId for Option<Retained<T>> {
}
}

/// Helper for ensuring that `ClassType::Mutability` is implemented correctly
/// for subclasses.
pub trait ValidSubclassMutability<T: mutability::Mutability> {}

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

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

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

/// Ensure that:
/// 1. The type is not a root class (it's superclass implements `ClassType`,
/// and it's mutability is not `Root`), and therefore also implements basic
/// memory management methods, as required by `unsafe impl Message`.
/// 2. The mutability is valid according to the superclass' mutability.
#[inline]
pub fn assert_mutability_matches_superclass_mutability<T>()
where
T: ?Sized + ClassType,
T::Super: ClassType,
T::Mutability: mutability::Mutability,
<T::Super as ClassType>::Mutability: ValidSubclassMutability<T::Mutability>,
{
// Noop
}

#[derive(Debug)]
pub struct ClassBuilderHelper<T: ?Sized> {
builder: ClassBuilder,
Expand Down
Loading

0 comments on commit 7678c56

Please sign in to comment.