Skip to content

Commit bb96605

Browse files
madsmtmmadsmtm
committed
Disallow &mut self receivers in declare_class! when not mutable
These are clearly unsound, and even though the error messages are pretty terrible, it is still better to disallow them.
1 parent 053603b commit bb96605

File tree

5 files changed

+147
-18
lines changed

5 files changed

+147
-18
lines changed

‎crates/objc2/src/declare/mod.rs

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
//!
2121
//! use objc2::declare::ClassBuilder;
2222
//! use objc2::rc::Id;
23-
//! use objc2::runtime::{Class, NSObject, Sel};
23+
//! use objc2::runtime::{Class, Object, NSObject, Sel};
2424
//! use objc2::{sel, msg_send, msg_send_id, ClassType};
2525
//!
2626
//! fn register_class() -> &'static Class {
@@ -32,12 +32,16 @@
3232
//! builder.add_ivar::<Cell<u32>>("_number");
3333
//!
3434
//! // Add an Objective-C method for initializing an instance with a number
35+
//! //
36+
//! // We "cheat" a bit here, and use `Object` instead of `NSObject`,
37+
//! // since only the former is allowed to be a mutable receiver (which is
38+
//! // always safe in `init` methods, but not in others).
3539
//! unsafe extern "C" fn init_with_number(
36-
//! this: &mut NSObject,
40+
//! this: &mut Object,
3741
//! _cmd: Sel,
3842
//! number: u32,
39-
//! ) -> Option<&mut NSObject> {
40-
//! let this: Option<&mut NSObject> = msg_send![super(this, NSObject::class()), init];
43+
//! ) -> Option<&mut Object> {
44+
//! let this: Option<&mut Object> = msg_send![super(this, NSObject::class()), init];
4145
//! this.map(|this| {
4246
//! // SAFETY: The ivar is added with the same type above
4347
//! this.set_ivar::<Cell<u32>>("_number", Cell::new(number));
@@ -130,6 +134,7 @@ use std::ffi::CString;
130134
use crate::encode::__unstable::{EncodeArguments, EncodeReturn};
131135
use crate::encode::{Encode, Encoding, RefEncode};
132136
use crate::ffi;
137+
use crate::mutability::IsMutable;
133138
use crate::rc::Allocated;
134139
use crate::runtime::{Bool, Class, Imp, Object, Protocol, Sel};
135140
use crate::sel;
@@ -161,17 +166,17 @@ pub trait MethodImplementation: private::Sealed {
161166
}
162167

163168
macro_rules! method_decl_impl {
164-
(@<$($l:lifetime),*> T, $r:ident, $f:ty, $($t:ident),*) => {
169+
(@<$($l:lifetime),*> T: $t_bound:ident, $r:ident, $f:ty, $($t:ident),*) => {
165170
impl<$($l,)* T, $r, $($t),*> private::Sealed for $f
166171
where
167-
T: Message + ?Sized,
172+
T: ?Sized + $t_bound,
168173
$r: EncodeReturn,
169174
$($t: Encode,)*
170175
{}
171176

172177
impl<$($l,)* T, $r, $($t),*> MethodImplementation for $f
173178
where
174-
T: Message + ?Sized,
179+
T: ?Sized + $t_bound,
175180
$r: EncodeReturn,
176181
$($t: Encode,)*
177182
{
@@ -184,7 +189,7 @@ macro_rules! method_decl_impl {
184189
}
185190
}
186191
};
187-
(@<$($l:lifetime),*> Class, $r:ident, $f:ty, $($t:ident),*) => {
192+
(@<$($l:lifetime),*> $callee:ident, $r:ident, $f:ty, $($t:ident),*) => {
188193
impl<$($l,)* $r, $($t),*> private::Sealed for $f
189194
where
190195
$r: EncodeReturn,
@@ -196,7 +201,7 @@ macro_rules! method_decl_impl {
196201
$r: EncodeReturn,
197202
$($t: Encode,)*
198203
{
199-
type Callee = Class;
204+
type Callee = $callee;
200205
type Ret = $r;
201206
type Args = ($($t,)*);
202207

@@ -209,14 +214,14 @@ macro_rules! method_decl_impl {
209214
#[doc(hidden)]
210215
impl<T, $($t),*> private::Sealed for $f
211216
where
212-
T: Message + ?Sized,
217+
T: ?Sized + Message,
213218
$($t: Encode,)*
214219
{}
215220

216221
#[doc(hidden)]
217222
impl<T, $($t),*> MethodImplementation for $f
218223
where
219-
T: Message + ?Sized,
224+
T: ?Sized + Message,
220225
$($t: Encode,)*
221226
{
222227
type Callee = T;
@@ -237,12 +242,15 @@ macro_rules! method_decl_impl {
237242
}
238243
};
239244
(# $abi:literal; $($t:ident),*) => {
240-
method_decl_impl!(@<'a> T, R, extern $abi fn(&'a T, Sel $(, $t)*) -> R, $($t),*);
241-
method_decl_impl!(@<'a> T, R, extern $abi fn(&'a mut T, Sel $(, $t)*) -> R, $($t),*);
242-
method_decl_impl!(@<> T, R, unsafe extern $abi fn(*const T, Sel $(, $t)*) -> R, $($t),*);
243-
method_decl_impl!(@<> T, R, unsafe extern $abi fn(*mut T, Sel $(, $t)*) -> R, $($t),*);
244-
method_decl_impl!(@<'a> T, R, unsafe extern $abi fn(&'a T, Sel $(, $t)*) -> R, $($t),*);
245-
method_decl_impl!(@<'a> T, R, unsafe extern $abi fn(&'a mut T, Sel $(, $t)*) -> R, $($t),*);
245+
method_decl_impl!(@<'a> T: Message, R, extern $abi fn(&'a T, Sel $(, $t)*) -> R, $($t),*);
246+
method_decl_impl!(@<'a> T: IsMutable, R, extern $abi fn(&'a mut T, Sel $(, $t)*) -> R, $($t),*);
247+
method_decl_impl!(@<> T: Message, R, unsafe extern $abi fn(*const T, Sel $(, $t)*) -> R, $($t),*);
248+
method_decl_impl!(@<> T: Message, R, unsafe extern $abi fn(*mut T, Sel $(, $t)*) -> R, $($t),*);
249+
method_decl_impl!(@<'a> T: Message, R, unsafe extern $abi fn(&'a T, Sel $(, $t)*) -> R, $($t),*);
250+
method_decl_impl!(@<'a> T: IsMutable, R, unsafe extern $abi fn(&'a mut T, Sel $(, $t)*) -> R, $($t),*);
251+
252+
method_decl_impl!(@<'a> Object, R, extern $abi fn(&'a mut Object, Sel $(, $t)*) -> R, $($t),*);
253+
method_decl_impl!(@<'a> Object, R, unsafe extern $abi fn(&'a mut Object, Sel $(, $t)*) -> R, $($t),*);
246254

247255
method_decl_impl!(@<'a> Class, R, extern $abi fn(&'a Class, Sel $(, $t)*) -> R, $($t),*);
248256
method_decl_impl!(@<> Class, R, unsafe extern $abi fn(*const Class, Sel $(, $t)*) -> R, $($t),*);

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@
8080
/// your method will be registered as an instance method, and if you don't it
8181
/// will be registered as a class method.
8282
///
83+
/// On instance methods, you can freely choose between different types of
84+
/// receivers, e.g. `&self`, `this: *const Self`, `&mut self`, and so on. Note
85+
/// 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
88+
/// [`Cell`] or similar instead.
89+
///
8390
/// The desired selector can be specified using the `#[method(my:selector:)]`
8491
/// or `#[method_id(my:selector:)]` attribute, similar to the
8592
/// [`extern_methods!`] macro.
@@ -107,6 +114,8 @@
107114
///
108115
/// ["associated functions"]: https://doc.rust-lang.org/reference/items/associated-items.html#methods
109116
/// ["methods"]: https://doc.rust-lang.org/reference/items/associated-items.html#methods
117+
/// [`IsMutable`]: crate::mutability::IsMutable
118+
/// [`Cell`]: core::cell::Cell
110119
/// [open an issue]: https://github.com/madsmtm/objc2/issues/new
111120
/// [`msg_send!`]: crate::msg_send
112121
/// [`runtime::Bool`]: crate::runtime::Bool

‎crates/objc2/src/rc/test_object.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ declare_class!(
9999
}
100100

101101
#[method(init)]
102-
fn init(this: &mut Self) -> *mut Self {
102+
unsafe fn init(this: *mut Self) -> *mut Self {
103103
TEST_DATA.with(|data| data.borrow_mut().init += 1);
104104
unsafe { msg_send![super(this), init] }
105105
}

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
use objc2::rc::Id;
2+
use objc2::runtime::NSObject;
3+
use objc2::{declare_class, mutability, ClassType};
4+
5+
declare_class!(
6+
struct CustomObject;
7+
8+
unsafe impl ClassType for CustomObject {
9+
type Super = NSObject;
10+
type Mutability = mutability::InteriorMutable;
11+
const NAME: &'static str = "CustomObject";
12+
}
13+
14+
unsafe impl CustomObject {
15+
#[method(initTest)]
16+
fn initTest(&mut self) -> &mut Self {
17+
unimplemented!()
18+
}
19+
20+
#[method(testMethod)]
21+
fn test_method(&mut self) {
22+
unimplemented!()
23+
}
24+
25+
#[method_id(testMethodId)]
26+
fn test_method_id(&mut self) -> Id<Self> {
27+
unimplemented!()
28+
}
29+
}
30+
);
31+
32+
fn main() {}

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

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
error[E0277]: the trait bound `InteriorMutable: mutability::private::MutabilityIsMutable` is not satisfied
2+
--> ui/declare_class_mut_self_not_mutable.rs
3+
|
4+
| / declare_class!(
5+
| | struct CustomObject;
6+
| |
7+
| | unsafe impl ClassType for CustomObject {
8+
... |
9+
| | }
10+
| | );
11+
| | ^
12+
| | |
13+
| |_the trait `mutability::private::MutabilityIsMutable` is not implemented for `InteriorMutable`
14+
| required by a bound introduced by this call
15+
|
16+
= help: the following other types implement trait `mutability::private::MutabilityIsMutable`:
17+
Mutable
18+
MutableWithImmutableSuperclass<IS>
19+
= note: required for `CustomObject` to implement `IsMutable`
20+
= note: required for `extern "C" fn(&mut CustomObject, objc2::runtime::Sel) -> &mut CustomObject` to implement `MethodImplementation`
21+
note: required by a bound in `ClassBuilder::add_method`
22+
--> $WORKSPACE/crates/objc2/src/declare/mod.rs
23+
|
24+
| F: MethodImplementation<Callee = T>,
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ClassBuilder::add_method`
26+
= 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)
27+
28+
error[E0277]: the trait bound `InteriorMutable: mutability::private::MutabilityIsMutable` is not satisfied
29+
--> ui/declare_class_mut_self_not_mutable.rs
30+
|
31+
| / declare_class!(
32+
| | struct CustomObject;
33+
| |
34+
| | unsafe impl ClassType for CustomObject {
35+
... |
36+
| | }
37+
| | );
38+
| | ^
39+
| | |
40+
| |_the trait `mutability::private::MutabilityIsMutable` is not implemented for `InteriorMutable`
41+
| required by a bound introduced by this call
42+
|
43+
= help: the following other types implement trait `mutability::private::MutabilityIsMutable`:
44+
Mutable
45+
MutableWithImmutableSuperclass<IS>
46+
= note: required for `CustomObject` to implement `IsMutable`
47+
= note: required for `extern "C" fn(&mut CustomObject, objc2::runtime::Sel)` to implement `MethodImplementation`
48+
note: required by a bound in `ClassBuilder::add_method`
49+
--> $WORKSPACE/crates/objc2/src/declare/mod.rs
50+
|
51+
| F: MethodImplementation<Callee = T>,
52+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ClassBuilder::add_method`
53+
= 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)
54+
55+
error[E0277]: the trait bound `InteriorMutable: mutability::private::MutabilityIsMutable` is not satisfied
56+
--> ui/declare_class_mut_self_not_mutable.rs
57+
|
58+
| / declare_class!(
59+
| | struct CustomObject;
60+
| |
61+
| | unsafe impl ClassType for CustomObject {
62+
... |
63+
| | }
64+
| | );
65+
| | ^
66+
| | |
67+
| |_the trait `mutability::private::MutabilityIsMutable` is not implemented for `InteriorMutable`
68+
| required by a bound introduced by this call
69+
|
70+
= help: the following other types implement trait `mutability::private::MutabilityIsMutable`:
71+
Mutable
72+
MutableWithImmutableSuperclass<IS>
73+
= note: required for `CustomObject` to implement `IsMutable`
74+
= note: required for `extern "C" fn(&mut CustomObject, objc2::runtime::Sel) -> __IdReturnValue` to implement `MethodImplementation`
75+
note: required by a bound in `ClassBuilder::add_method`
76+
--> $WORKSPACE/crates/objc2/src/declare/mod.rs
77+
|
78+
| F: MethodImplementation<Callee = T>,
79+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ClassBuilder::add_method`
80+
= 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)

0 commit comments

Comments
 (0)