Skip to content

Commit aabe354

Browse files
madsmtmmadsmtm
committed
Add IsAllowedMutable
1 parent 76e0386 commit aabe354

13 files changed

+260
-36
lines changed

‎crates/objc2/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
1010
* Added the following traits to the `mutability` module (see the documentation
1111
for motivation and usage info):
1212
- `HasStableHash`.
13+
- `IsAllowedMutable`.
1314
- `IsMainThreadOnly`.
1415
- `CounterpartOrSelf`.
1516
* Added new `encode` traits `EncodeReturn`, `EncodeArgument` and
@@ -57,6 +58,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
5758
### Fixed
5859
* Fixed the name of the protocol that `NSObjectProtocol` references.
5960
* Allow cloning `Id<AnyObject>`.
61+
* **BREAKING**: Restrict message sending to `&mut` references to things that
62+
implement `IsAllowedMutable`.
6063

6164
### Removed
6265
* **BREAKING**: Removed `ProtocolType` implementation for `NSObject`.

‎crates/objc2/src/macros/declare_class.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,9 @@
8383
/// On instance methods, you can freely choose between different types of
8484
/// receivers, e.g. `&self`, `this: *const Self`, `&mut self`, and so on. Note
8585
/// though that using raw pointers requires the function to be `unsafe`, and
86-
/// using `&mut self` requires the class' mutability to be [`IsMutable`].
87-
/// If you require mutating your class' instance variables, consider using
86+
/// using `&mut self` requires the class' mutability to be
87+
/// [`IsAllowedMutable`].
88+
/// If you require mutation of your class' instance variables, consider using
8889
/// [`Cell`] or similar instead.
8990
///
9091
/// The desired selector can be specified using the `#[method(my:selector:)]`
@@ -114,7 +115,7 @@
114115
///
115116
/// ["associated functions"]: https://doc.rust-lang.org/reference/items/associated-items.html#methods
116117
/// ["methods"]: https://doc.rust-lang.org/reference/items/associated-items.html#methods
117-
/// [`IsMutable`]: crate::mutability::IsMutable
118+
/// [`IsAllowedMutable`]: crate::mutability::IsAllowedMutable
118119
/// [`Cell`]: core::cell::Cell
119120
/// [open an issue]: https://github.com/madsmtm/objc2/issues/new
120121
/// [`msg_send!`]: crate::msg_send

‎crates/objc2/src/mutability.rs

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ enum Never {}
8181
/// Functionality that is provided with this:
8282
/// - [`IsIdCloneable`] -> [`Id::clone`][crate::rc::Id#impl-Clone-for-Id<T>].
8383
/// - [`IsAllocableAnyThread`] -> [`ClassType::alloc`].
84+
/// - [`IsAllowedMutable`].
8485
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
8586
pub struct Root {
8687
inner: Never,
@@ -110,6 +111,7 @@ pub struct Immutable {
110111
///
111112
/// Functionality that is provided with this:
112113
/// - [`IsAllocableAnyThread`] -> [`ClassType::alloc`].
114+
/// - [`IsAllowedMutable`].
113115
/// - [`IsMutable`] -> [`impl DerefMut for Id`][crate::rc::Id#impl-DerefMut-for-Id<T>].
114116
/// - You are allowed to hand out pointers / references to an instance's
115117
/// internal data, since you know such data will never be mutated without
@@ -143,6 +145,7 @@ pub struct Mutable {
143145
/// Functionality that is provided with this:
144146
/// - [`IsIdCloneable`].
145147
/// - [`IsAllocableAnyThread`].
148+
/// - [`IsAllowedMutable`].
146149
/// - You are allowed to hand out pointers / references to an instance's
147150
/// internal data, since you know such data will never be mutated.
148151
///
@@ -176,6 +179,7 @@ pub struct ImmutableWithMutableSubclass<MS: ?Sized> {
176179
///
177180
/// Functionality that is provided with this:
178181
/// - [`IsAllocableAnyThread`] -> [`ClassType::alloc`].
182+
/// - [`IsAllowedMutable`].
179183
/// - [`IsMutable`] -> [`impl DerefMut for Id`][crate::rc::Id#impl-DerefMut-for-Id<T>].
180184
/// - You are allowed to hand out pointers / references to an instance's
181185
/// internal data, since you know such data will never be mutated without
@@ -324,7 +328,7 @@ unsafe impl IsIdCloneable for AnyObject {}
324328
///
325329
/// This is a sealed trait, and should not need to be implemented. Open an
326330
/// issue if you know a use-case where this restrition should be lifted!
327-
pub unsafe trait IsRetainable: IsIdCloneable {}
331+
pub unsafe trait IsRetainable: private_traits::Sealed + IsIdCloneable {}
328332

329333
trait MutabilityIsRetainable: MutabilityIsIdCloneable {}
330334
impl MutabilityIsRetainable for Immutable {}
@@ -368,6 +372,39 @@ unsafe impl<P: ?Sized + ProtocolType + IsAllocableAnyThread> IsAllocableAnyThrea
368372
{
369373
}
370374

375+
/// Marker trait for classes that may feasibly be used behind a mutable
376+
/// reference.
377+
///
378+
/// This trait exist mostly to disallow using `&mut self` when declaring
379+
/// classes, since that would be a huge footgun.
380+
///
381+
/// This is implemented for classes whose [`ClassType::Mutability`] is one of:
382+
/// - [`Root`]
383+
/// - [`Mutable`]
384+
/// - [`ImmutableWithMutableSubclass`]
385+
/// - [`MutableWithImmutableSuperclass`]
386+
///
387+
///
388+
/// # Safety
389+
///
390+
/// This is a sealed trait, and should not need to be implemented. Open an
391+
/// issue if you know a use-case where this restrition should be lifted!
392+
pub unsafe trait IsAllowedMutable: private_traits::Sealed {}
393+
394+
trait MutabilityIsAllowedMutable: Mutability {}
395+
impl MutabilityIsAllowedMutable for Root {}
396+
impl MutabilityIsAllowedMutable for Mutable {}
397+
impl<MS: ?Sized> MutabilityIsAllowedMutable for ImmutableWithMutableSubclass<MS> {}
398+
impl<IS: ?Sized> MutabilityIsAllowedMutable for MutableWithImmutableSuperclass<IS> {}
399+
400+
unsafe impl<T: ?Sized + ClassType> IsAllowedMutable for T where
401+
T::Mutability: MutabilityIsAllowedMutable
402+
{
403+
}
404+
unsafe impl<P: ?Sized + ProtocolType + IsAllowedMutable> IsAllowedMutable for ProtocolObject<P> {}
405+
// SAFETY: Same as for root classes.
406+
unsafe impl IsAllowedMutable for AnyObject {}
407+
371408
/// Marker trait for classes that are only mutable through `&mut`.
372409
///
373410
/// This is implemented for classes whose [`ClassType::Mutability`] is one of:
@@ -378,14 +415,17 @@ unsafe impl<P: ?Sized + ProtocolType + IsAllocableAnyThread> IsAllocableAnyThrea
378415
/// technically mutable), since it is allowed to mutate through shared
379416
/// references.
380417
///
418+
/// This trait inherits [`IsAllowedMutable`], so if a function is bound by
419+
/// this, functionality given with that trait is available.
420+
///
381421
///
382422
/// # Safety
383423
///
384424
/// This is a sealed trait, and should not need to be implemented. Open an
385425
/// issue if you know a use-case where this restrition should be lifted!
386-
pub unsafe trait IsMutable: private_traits::Sealed {}
426+
pub unsafe trait IsMutable: private_traits::Sealed + IsAllowedMutable {}
387427

388-
trait MutabilityIsMutable: Mutability {}
428+
trait MutabilityIsMutable: MutabilityIsAllowedMutable {}
389429
impl MutabilityIsMutable for Mutable {}
390430
impl<IS: ?Sized> MutabilityIsMutable for MutableWithImmutableSuperclass<IS> {}
391431

@@ -616,11 +656,12 @@ mod tests {
616656
);
617657
}
618658

619-
#[allow(unused)]
659+
#[allow(unused, clippy::too_many_arguments)]
620660
fn object_safe(
621661
_: &dyn IsIdCloneable,
622662
_: &dyn IsRetainable,
623663
_: &dyn IsAllocableAnyThread,
664+
_: &dyn IsAllowedMutable,
624665
_: &dyn IsMutable,
625666
_: &dyn IsMainThreadOnly,
626667
_: &dyn HasStableHash,

‎crates/objc2/src/runtime/message.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use core::ptr::{self, NonNull};
33

44
use crate::__macro_helpers::{ConvertArgument, ConvertReturn};
55
use crate::encode::{Encode, EncodeArguments, EncodeReturn};
6-
use crate::mutability::IsMutable;
6+
use crate::mutability::{IsAllowedMutable, IsMutable};
77
use crate::rc::Id;
88
use crate::runtime::{AnyClass, AnyObject, Imp, Sel};
99
use crate::{ClassType, Message};
@@ -573,10 +573,8 @@ unsafe impl<'a, T: ?Sized + Message> MessageReceiver for &'a T {
573573
}
574574
}
575575

576-
// TODO: Use `T: IsMutable + Root` here once we can handle `init` methods
577-
// better in `declare_class!`.
578-
impl<'a, T: ?Sized + Message> private::Sealed for &'a mut T {}
579-
unsafe impl<'a, T: ?Sized + Message> MessageReceiver for &'a mut T {
576+
impl<'a, T: ?Sized + Message + IsAllowedMutable> private::Sealed for &'a mut T {}
577+
unsafe impl<'a, T: ?Sized + Message + IsAllowedMutable> MessageReceiver for &'a mut T {
580578
type __Inner = T;
581579

582580
#[inline]

‎crates/objc2/src/runtime/method_implementation.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ use core::mem;
22

33
use crate::__macro_helpers::IdReturnValue;
44
use crate::encode::{EncodeArgument, EncodeArguments, EncodeReturn, RefEncode};
5-
use crate::mutability::IsMutable;
5+
use crate::mutability::IsAllowedMutable;
66
use crate::rc::Allocated;
7-
use crate::runtime::{AnyClass, AnyObject, Imp, Sel};
7+
use crate::runtime::{AnyClass, Imp, Sel};
88
use crate::Message;
99

1010
mod private {
@@ -116,14 +116,11 @@ macro_rules! method_impl_allocated {
116116
macro_rules! method_impl_abi {
117117
($abi:literal; $($t:ident),*) => {
118118
method_impl_generic!(<'a> T: Message, R, extern $abi fn(&'a T, Sel $(, $t)*) -> R, $($t),*);
119-
method_impl_generic!(<'a> T: Message + IsMutable, R, extern $abi fn(&'a mut T, Sel $(, $t)*) -> R, $($t),*);
119+
method_impl_generic!(<'a> T: Message + IsAllowedMutable, R, extern $abi fn(&'a mut T, Sel $(, $t)*) -> R, $($t),*);
120120
method_impl_generic!(<> T: Message, R, unsafe extern $abi fn(*const T, Sel $(, $t)*) -> R, $($t),*);
121121
method_impl_generic!(<> T: Message, R, unsafe extern $abi fn(*mut T, Sel $(, $t)*) -> R, $($t),*);
122122
method_impl_generic!(<'a> T: Message, R, unsafe extern $abi fn(&'a T, Sel $(, $t)*) -> R, $($t),*);
123-
method_impl_generic!(<'a> T: Message + IsMutable, R, unsafe extern $abi fn(&'a mut T, Sel $(, $t)*) -> R, $($t),*);
124-
125-
method_impl_concrete!(<'a> AnyObject, R, extern $abi fn(&'a mut AnyObject, Sel $(, $t)*) -> R, $($t),*);
126-
method_impl_concrete!(<'a> AnyObject, R, unsafe extern $abi fn(&'a mut AnyObject, Sel $(, $t)*) -> R, $($t),*);
123+
method_impl_generic!(<'a> T: Message + IsAllowedMutable, R, unsafe extern $abi fn(&'a mut T, Sel $(, $t)*) -> R, $($t),*);
127124

128125
method_impl_concrete!(<'a> AnyClass, R, extern $abi fn(&'a AnyClass, Sel $(, $t)*) -> R, $($t),*);
129126
method_impl_concrete!(<> AnyClass, R, unsafe extern $abi fn(*const AnyClass, Sel $(, $t)*) -> R, $($t),*);

‎crates/test-ui/ui/declare_class_mut_self_not_mutable.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ declare_class!(
1313

1414
unsafe impl CustomObject {
1515
#[method(initTest)]
16-
fn initTest(&mut self) -> &mut Self {
16+
fn init_test(&mut self) -> &mut Self {
1717
unimplemented!()
1818
}
1919

‎crates/test-ui/ui/declare_class_mut_self_not_mutable.stderr

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error[E0277]: the trait bound `InteriorMutable: mutability::MutabilityIsMutable` is not satisfied
1+
error[E0277]: the trait bound `InteriorMutable: mutability::MutabilityIsAllowedMutable` is not satisfied
22
--> ui/declare_class_mut_self_not_mutable.rs
33
|
44
| / declare_class!(
@@ -10,13 +10,15 @@ error[E0277]: the trait bound `InteriorMutable: mutability::MutabilityIsMutable`
1010
| | );
1111
| | ^
1212
| | |
13-
| |_the trait `mutability::MutabilityIsMutable` is not implemented for `InteriorMutable`
13+
| |_the trait `mutability::MutabilityIsAllowedMutable` is not implemented for `InteriorMutable`
1414
| required by a bound introduced by this call
1515
|
16-
= help: the following other types implement trait `mutability::MutabilityIsMutable`:
16+
= help: the following other types implement trait `mutability::MutabilityIsAllowedMutable`:
17+
Root
1718
Mutable
19+
ImmutableWithMutableSubclass<MS>
1820
MutableWithImmutableSuperclass<IS>
19-
= note: required for `CustomObject` to implement `IsMutable`
21+
= note: required for `CustomObject` to implement `IsAllowedMutable`
2022
= note: required for `extern "C" fn(&mut CustomObject, objc2::runtime::Sel) -> &mut CustomObject` to implement `MethodImplementation`
2123
note: required by a bound in `ClassBuilder::add_method`
2224
--> $WORKSPACE/crates/objc2/src/declare/mod.rs
@@ -28,7 +30,7 @@ note: required by a bound in `ClassBuilder::add_method`
2830
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ClassBuilder::add_method`
2931
= note: this error originates in the macro `$crate::__declare_class_register_out` which comes from the expansion of the macro `declare_class` (in Nightly builds, run with -Z macro-backtrace for more info)
3032

31-
error[E0277]: the trait bound `InteriorMutable: mutability::MutabilityIsMutable` is not satisfied
33+
error[E0277]: the trait bound `InteriorMutable: mutability::MutabilityIsAllowedMutable` is not satisfied
3234
--> ui/declare_class_mut_self_not_mutable.rs
3335
|
3436
| / declare_class!(
@@ -40,13 +42,15 @@ error[E0277]: the trait bound `InteriorMutable: mutability::MutabilityIsMutable`
4042
| | );
4143
| | ^
4244
| | |
43-
| |_the trait `mutability::MutabilityIsMutable` is not implemented for `InteriorMutable`
45+
| |_the trait `mutability::MutabilityIsAllowedMutable` is not implemented for `InteriorMutable`
4446
| required by a bound introduced by this call
4547
|
46-
= help: the following other types implement trait `mutability::MutabilityIsMutable`:
48+
= help: the following other types implement trait `mutability::MutabilityIsAllowedMutable`:
49+
Root
4750
Mutable
51+
ImmutableWithMutableSubclass<MS>
4852
MutableWithImmutableSuperclass<IS>
49-
= note: required for `CustomObject` to implement `IsMutable`
53+
= note: required for `CustomObject` to implement `IsAllowedMutable`
5054
= note: required for `extern "C" fn(&mut CustomObject, objc2::runtime::Sel)` to implement `MethodImplementation`
5155
note: required by a bound in `ClassBuilder::add_method`
5256
--> $WORKSPACE/crates/objc2/src/declare/mod.rs
@@ -58,7 +62,7 @@ note: required by a bound in `ClassBuilder::add_method`
5862
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ClassBuilder::add_method`
5963
= note: this error originates in the macro `$crate::__declare_class_register_out` which comes from the expansion of the macro `declare_class` (in Nightly builds, run with -Z macro-backtrace for more info)
6064

61-
error[E0277]: the trait bound `InteriorMutable: mutability::MutabilityIsMutable` is not satisfied
65+
error[E0277]: the trait bound `InteriorMutable: mutability::MutabilityIsAllowedMutable` is not satisfied
6266
--> ui/declare_class_mut_self_not_mutable.rs
6367
|
6468
| / declare_class!(
@@ -70,13 +74,15 @@ error[E0277]: the trait bound `InteriorMutable: mutability::MutabilityIsMutable`
7074
| | );
7175
| | ^
7276
| | |
73-
| |_the trait `mutability::MutabilityIsMutable` is not implemented for `InteriorMutable`
77+
| |_the trait `mutability::MutabilityIsAllowedMutable` is not implemented for `InteriorMutable`
7478
| required by a bound introduced by this call
7579
|
76-
= help: the following other types implement trait `mutability::MutabilityIsMutable`:
80+
= help: the following other types implement trait `mutability::MutabilityIsAllowedMutable`:
81+
Root
7782
Mutable
83+
ImmutableWithMutableSubclass<MS>
7884
MutableWithImmutableSuperclass<IS>
79-
= note: required for `CustomObject` to implement `IsMutable`
85+
= note: required for `CustomObject` to implement `IsAllowedMutable`
8086
= note: required for `extern "C" fn(&mut CustomObject, objc2::runtime::Sel) -> IdReturnValue` to implement `MethodImplementation`
8187
note: required by a bound in `ClassBuilder::add_method`
8288
--> $WORKSPACE/crates/objc2/src/declare/mod.rs
@@ -87,3 +93,25 @@ note: required by a bound in `ClassBuilder::add_method`
8793
| F: MethodImplementation<Callee = T>,
8894
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ClassBuilder::add_method`
8995
= note: this error originates in the macro `$crate::__declare_class_register_out` which comes from the expansion of the macro `declare_class` (in Nightly builds, run with -Z macro-backtrace for more info)
96+
97+
error[E0277]: the trait bound `InteriorMutable: mutability::MutabilityIsAllowedMutable` is not satisfied
98+
--> ui/declare_class_mut_self_not_mutable.rs
99+
|
100+
| / declare_class!(
101+
| | struct CustomObject;
102+
| |
103+
| | unsafe impl ClassType for CustomObject {
104+
... |
105+
| | }
106+
| | );
107+
| |_^ the trait `mutability::MutabilityIsAllowedMutable` is not implemented for `InteriorMutable`
108+
|
109+
= help: the following other types implement trait `mutability::MutabilityIsAllowedMutable`:
110+
Root
111+
Mutable
112+
ImmutableWithMutableSubclass<MS>
113+
MutableWithImmutableSuperclass<IS>
114+
= note: required for `CustomObject` to implement `IsAllowedMutable`
115+
= note: required for `&mut CustomObject` to implement `MessageReceiver`
116+
= note: required for `RetainSemantics<5>` to implement `MessageRecieveId<&mut CustomObject, Id<CustomObject>>`
117+
= note: this error originates in the macro `$crate::__rewrite_self_param_inner` which comes from the expansion of the macro `declare_class` (in Nightly builds, run with -Z macro-backtrace for more info)

‎crates/test-ui/ui/extern_methods_not_allowed_mutable.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//! Test extern_methods! with mutable receivers that are not IsAllowedMutable.
2+
use objc2::rc::Id;
3+
use objc2::runtime::NSObject;
4+
use objc2::{extern_class, extern_methods, mutability, ClassType};
5+
6+
extern_class!(
7+
pub struct MyObject;
8+
9+
unsafe impl ClassType for MyObject {
10+
type Super = NSObject;
11+
type Mutability = mutability::InteriorMutable;
12+
}
13+
);
14+
15+
extern_methods!(
16+
unsafe impl MyObject {
17+
#[method(test)]
18+
fn test(&mut self);
19+
}
20+
);
21+
22+
extern_methods!(
23+
unsafe impl MyObject {
24+
#[method_id(testId)]
25+
fn test_id(&mut self) -> Id<Self>;
26+
}
27+
);
28+
29+
fn main() {}

0 commit comments

Comments
 (0)