Skip to content

Commit

Permalink
Consistently implement MethodImplementation wrt. lifetimes
Browse files Browse the repository at this point in the history 
Previously, the receiver's lifetime was higher-ranked, while the rest of the arguments weren't, which meant that:
- Their type could not be inferred like the other arguments
- Having a return type with a lifetime bound to the receiver was impossible

Fixes upstream SSheldon/rust-objc#12, at least as far as possible right now.

This is a breaking change, and is unfortunately difficult for users to know how to fix, not really sure how to improve that situation?
  • Loading branch information
@madsmtm
madsmtm committed Jul 7, 2022
1 parent dee66be commit 5882f01
Show file tree
Hide file tree
Showing 12 changed files with 110 additions and 96 deletions.
3 changes: 2 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ runtime libraries.
It is recommended that you upgrade in small steps, to decrease the chance of
making a mistake somewhere. This way, you can under each release review the
relevant changelogs and run your test suite, to ensure you haven't missed
something.
something. For the most common errors, the changelogs will include a helpful
example on how to upgrade.

As an example, if you want to migrate to `objc2`, you'd start by using it
instead of `objc` in your `Cargo.toml`:
Expand Down
3 changes: 1 addition & 2 deletions objc2-foundation/examples/class_with_lifetime.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ impl<'a> MyObject<'a> {
}

unsafe {
let init_with_ptr: unsafe extern "C" fn(*mut Object, Sel, *mut u8) -> *mut Object =
init_with_ptr;
let init_with_ptr: unsafe extern "C" fn(_, _, _) -> _ = init_with_ptr;
builder.add_method(sel!(initWithPtr:), init_with_ptr);
}

Expand Down
4 changes: 2 additions & 2 deletions objc2-foundation/examples/custom_class.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ impl MYObject {
}

unsafe {
let set_number: extern "C" fn(&mut MYObject, Sel, u32) = my_object_set_number;
let set_number: extern "C" fn(_, _, _) = my_object_set_number;
builder.add_method(sel!(setNumber:), set_number);
let get_number: extern "C" fn(&MYObject, Sel) -> u32 = my_object_get_number;
let get_number: extern "C" fn(_, _) -> _ = my_object_get_number;
builder.add_method(sel!(number), get_number);
}

Expand Down
28 changes: 28 additions & 0 deletions objc2/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,34 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
* **BREAKING**: Changed `MessageReceiver::send_message` to panic instead of
returning an error.
* **BREAKING**: Renamed `catch_all` feature to `catch-all`.
* **BREAKING**: Made passing the function pointer argument to
`ClassBuilder::add_method`, `ClassBuilder::add_class_method` and similar
more ergonomic.

Let's say you have the following code:
```rust
// Before
let init: extern "C" fn(&mut Object, Sel) -> *mut Object = init;
builder.add_method(sel!(init), init);
```

Unfortunately, you will now encounter a very confusing error:
```
|
2 | builder.add_method(sel!(init), init);
| ^^^^^^^^^^ implementation of `MethodImplementation` is not general enough
|
= note: `MethodImplementation` would have to be implemented for the type `for<'r> extern "C" fn(&'r mut Object, Sel) -> *mut Object`
= note: ...but `MethodImplementation` is actually implemented for the type `extern "C" fn(&'0 mut Object, Sel) -> *mut Object`, for some specific lifetime `'0`
```

Fret not, the fix is easy! Just let the compiler infer the argument and
return types:
```rust
// After
let init: extern "C" fn(_, _) -> _ = init;
builder.add_method(sel!(init), init);
```

### Fixed
* **BREAKING**: Disallow throwing `nil` exceptions in `exception::throw`.
Expand Down
42 changes: 21 additions & 21 deletions objc2/src/declare.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
//! unsafe {
//! decl.add_method(
//! sel!(number),
//! my_number_get as extern "C" fn(&Object, Sel) -> u32,
//! my_number_get as extern "C" fn(_, _) -> _,
//! );
//! }
//!
Expand Down Expand Up @@ -69,21 +69,21 @@ pub trait MethodImplementation: private::Sealed {
}

macro_rules! method_decl_impl {
(-$s:ident, $r:ident, $f:ty, $($t:ident),*) => {
impl<$s, $r, $($t),*> private::Sealed for $f
(@<$($l:lifetime),*> T, $r:ident, $f:ty, $($t:ident),*) => {
impl<$($l,)* T, $r, $($t),*> private::Sealed for $f
where
$s: Message + ?Sized,
T: Message + ?Sized,
$r: Encode,
$($t: Encode,)*
{}

impl<$s, $r, $($t),*> MethodImplementation for $f
impl<$($l,)* T, $r, $($t),*> MethodImplementation for $f
where
$s: Message + ?Sized,
T: Message + ?Sized,
$r: Encode,
$($t: Encode,)*
{
type Callee = $s;
type Callee = T;
type Ret = $r;
type Args = ($($t,)*);

Expand All @@ -92,19 +92,19 @@ macro_rules! method_decl_impl {
}
}
};
(@$s:ident, $r:ident, $f:ty, $($t:ident),*) => {
impl<$r, $($t),*> private::Sealed for $f
(@<$($l:lifetime),*> Class, $r:ident, $f:ty, $($t:ident),*) => {
impl<$($l,)* $r, $($t),*> private::Sealed for $f
where
$r: Encode,
$($t: Encode,)*
{}

impl<$r, $($t),*> MethodImplementation for $f
impl<$($l,)* $r, $($t),*> MethodImplementation for $f
where
$r: Encode,
$($t: Encode,)*
{
type Callee = $s;
type Callee = Class;
type Ret = $r;
type Args = ($($t,)*);

Expand All @@ -114,16 +114,16 @@ macro_rules! method_decl_impl {
}
};
(# $abi:literal; $($t:ident),*) => {
method_decl_impl!(-T, R, extern $abi fn(&T, Sel $(, $t)*) -> R, $($t),*);
method_decl_impl!(-T, R, extern $abi fn(&mut T, Sel $(, $t)*) -> R, $($t),*);
method_decl_impl!(-T, R, unsafe extern $abi fn(*const T, Sel $(, $t)*) -> R, $($t),*);
method_decl_impl!(-T, R, unsafe extern $abi fn(*mut T, Sel $(, $t)*) -> R, $($t),*);
method_decl_impl!(-T, R, unsafe extern $abi fn(&T, Sel $(, $t)*) -> R, $($t),*);
method_decl_impl!(-T, R, unsafe extern $abi fn(&mut T, Sel $(, $t)*) -> R, $($t),*);

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

method_decl_impl!(@<'a> Class, R, extern $abi fn(&'a Class, Sel $(, $t)*) -> R, $($t),*);
method_decl_impl!(@<> Class, R, unsafe extern $abi fn(*const Class, Sel $(, $t)*) -> R, $($t),*);
method_decl_impl!(@<'a> Class, R, unsafe extern $abi fn(&'a Class, Sel $(, $t)*) -> R, $($t),*);
};
($($t:ident),*) => {
method_decl_impl!(# "C"; $($t),*);
Expand Down
2 changes: 1 addition & 1 deletion objc2/src/rc/id.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ impl<T: Message, O: Ownership> Id<T, O> {
/// unsafe {
/// builder.add_class_method(
/// sel!(get),
/// get as extern "C" fn(&Class, Sel) -> *mut Object,
/// get as extern "C" fn(_, _) -> _,
/// );
/// }
///
Expand Down
29 changes: 7 additions & 22 deletions objc2/src/rc/test_object.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,31 +118,16 @@ impl RcTestObject {

let mut builder = ClassBuilder::new("RcTestObject", class!(NSObject)).unwrap();
unsafe {
builder.add_class_method(
sel!(alloc),
alloc as extern "C" fn(&Class, Sel) -> *mut RcTestObject,
);
builder.add_method(
sel!(init),
init as extern "C" fn(&mut RcTestObject, Sel) -> _,
);
builder.add_method(
sel!(retain),
retain as extern "C" fn(&RcTestObject, Sel) -> _,
);
builder.add_class_method(sel!(alloc), alloc as extern "C" fn(_, _) -> _);
builder.add_method(sel!(init), init as extern "C" fn(_, _) -> _);
builder.add_method(sel!(retain), retain as extern "C" fn(_, _) -> _);
builder.add_method(
sel!(_tryRetain),
try_retain as unsafe extern "C" fn(&RcTestObject, Sel) -> Bool,
);
builder.add_method(sel!(release), release as extern "C" fn(&RcTestObject, Sel));
builder.add_method(
sel!(autorelease),
autorelease as extern "C" fn(&RcTestObject, Sel) -> _,
);
builder.add_method(
sel!(dealloc),
dealloc as unsafe extern "C" fn(*mut RcTestObject, Sel),
try_retain as unsafe extern "C" fn(_, _) -> _,
);
builder.add_method(sel!(release), release as extern "C" fn(_, _));
builder.add_method(sel!(autorelease), autorelease as extern "C" fn(_, _) -> _);
builder.add_method(sel!(dealloc), dealloc as unsafe extern "C" fn(_, _));
}

builder.register();
Expand Down
24 changes: 15 additions & 9 deletions objc2/src/test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub(crate) fn custom_class() -> &'static Class {

let mut builder = ClassBuilder::root(
"CustomObject",
custom_obj_class_initialize as extern "C" fn(&Class, Sel),
custom_obj_class_initialize as extern "C" fn(_, _),
)
.unwrap();
let proto = custom_protocol();
Expand All @@ -115,6 +115,10 @@ pub(crate) fn custom_class() -> &'static Class {
unsafe { *this.ivar::<u32>("_foo") }
}

extern "C" fn custom_obj_get_foo_reference(this: &Object, _cmd: Sel) -> &u32 {
unsafe { this.ivar::<u32>("_foo") }
}

extern "C" fn custom_obj_get_struct(_this: &Object, _cmd: Sel) -> CustomStruct {
CustomStruct {
a: 1,
Expand Down Expand Up @@ -144,21 +148,23 @@ pub(crate) fn custom_class() -> &'static Class {
}

unsafe {
let release: unsafe extern "C" fn(*mut Object, Sel) = custom_obj_release;
let release: unsafe extern "C" fn(_, _) = custom_obj_release;
builder.add_method(sel!(release), release);

let set_foo: extern "C" fn(&mut Object, Sel, u32) = custom_obj_set_foo;
let set_foo: extern "C" fn(_, _, _) = custom_obj_set_foo;
builder.add_method(sel!(setFoo:), set_foo);
let get_foo: extern "C" fn(&Object, Sel) -> u32 = custom_obj_get_foo;
let get_foo: extern "C" fn(_, _) -> _ = custom_obj_get_foo;
builder.add_method(sel!(foo), get_foo);
let get_struct: extern "C" fn(&Object, Sel) -> CustomStruct = custom_obj_get_struct;
let get_foo_reference: extern "C" fn(_, _) -> _ = custom_obj_get_foo_reference;
builder.add_method(sel!(fooReference), get_foo_reference);
let get_struct: extern "C" fn(_, _) -> CustomStruct = custom_obj_get_struct;
builder.add_method(sel!(customStruct), get_struct);
let class_method: extern "C" fn(&Class, Sel) -> u32 = custom_obj_class_method;
let class_method: extern "C" fn(_, _) -> _ = custom_obj_class_method;
builder.add_class_method(sel!(classFoo), class_method);

let protocol_instance_method: extern "C" fn(&mut Object, Sel, u32) = custom_obj_set_bar;
let protocol_instance_method: extern "C" fn(_, _, _) = custom_obj_set_bar;
builder.add_method(sel!(setBar:), protocol_instance_method);
let protocol_class_method: extern "C" fn(&Class, Sel, i32, i32) -> i32 =
let protocol_class_method: extern "C" fn(_, _, _, _) -> _ =
custom_obj_add_number_to_number;
builder.add_class_method(sel!(addNumber:toNumber:), protocol_class_method);
}
Expand Down Expand Up @@ -218,7 +224,7 @@ pub(crate) fn custom_subclass() -> &'static Class {
}

unsafe {
let get_foo: extern "C" fn(&Object, Sel) -> u32 = custom_subclass_get_foo;
let get_foo: extern "C" fn(_, _) -> _ = custom_subclass_get_foo;
builder.add_method(sel!(foo), get_foo);
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/fn_ptr_reference_method.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ fn main() {
let mut builder = ClassBuilder::new("SomeTestClass", class!(NSObject)).unwrap();
unsafe {
// Works
builder.add_method(sel!(first:), my_fn as extern "C" fn(&Object, _, _));
builder.add_method(sel!(none:), my_fn as extern "C" fn(_, _, _));

// Fails
builder.add_method(sel!(none:), my_fn as extern "C" fn(_, _, _));
builder.add_method(sel!(third:), my_fn as extern "C" fn(_, _, &Object));

// Also fails, properly tested in `fn_ptr_reference_method2`
builder.add_method(sel!(first:), my_fn as extern "C" fn(&Object, _, _));
builder.add_method(sel!(both:), my_fn as extern "C" fn(&Object, _, &Object));
}
}
44 changes: 10 additions & 34 deletions tests/ui/fn_ptr_reference_method.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,44 +1,20 @@
error[E0277]: the trait bound `extern "C" fn(_, _, _): MethodImplementation` is not satisfied
--> ui/fn_ptr_reference_method.rs:21:41
|
21 | builder.add_method(sel!(none:), my_fn as extern "C" fn(_, _, _));
| ---------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `MethodImplementation` is not implemented for `extern "C" fn(_, _, _)`
| |
| required by a bound introduced by this call
|
= help: the following other types implement trait `MethodImplementation`:
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B, C) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B, C, D) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B, C, D, E) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B, C, D, E, F) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B, C, D, E, F, G) -> R
and 109 others
note: required by a bound in `ClassBuilder::add_method`
--> $WORKSPACE/objc2/src/declare.rs
|
| F: MethodImplementation<Callee = T>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ClassBuilder::add_method`

error[E0277]: the trait bound `for<'r> extern "C" fn(_, _, &'r objc2::runtime::Object): MethodImplementation` is not satisfied
--> ui/fn_ptr_reference_method.rs:22:42
--> ui/fn_ptr_reference_method.rs:21:42
|
22 | builder.add_method(sel!(third:), my_fn as extern "C" fn(_, _, &Object));
21 | builder.add_method(sel!(third:), my_fn as extern "C" fn(_, _, &Object));
| ---------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `MethodImplementation` is not implemented for `for<'r> extern "C" fn(_, _, &'r objc2::runtime::Object)`
| |
| required by a bound introduced by this call
|
= help: the following other types implement trait `MethodImplementation`:
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B, C) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B, C, D) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B, C, D, E) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B, C, D, E, F) -> R
for<'r> extern "C" fn(&'r T, objc2::runtime::Sel, A, B, C, D, E, F, G) -> R
extern "C" fn(&'a T, objc2::runtime::Sel) -> R
extern "C" fn(&'a T, objc2::runtime::Sel, A) -> R
extern "C" fn(&'a T, objc2::runtime::Sel, A, B) -> R
extern "C" fn(&'a T, objc2::runtime::Sel, A, B, C) -> R
extern "C" fn(&'a T, objc2::runtime::Sel, A, B, C, D) -> R
extern "C" fn(&'a T, objc2::runtime::Sel, A, B, C, D, E) -> R
extern "C" fn(&'a T, objc2::runtime::Sel, A, B, C, D, E, F) -> R
extern "C" fn(&'a T, objc2::runtime::Sel, A, B, C, D, E, F, G) -> R
and 109 others
note: required by a bound in `ClassBuilder::add_method`
--> $WORKSPACE/objc2/src/declare.rs
Expand Down
1 change: 1 addition & 0 deletions tests/ui/fn_ptr_reference_method2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ extern "C" fn my_fn(_this: &Object, _cmd: Sel, _x: &Object) {}
fn main() {
let mut builder = ClassBuilder::new("SomeTestClass", class!(NSObject)).unwrap();
unsafe {
builder.add_method(sel!(first:), my_fn as extern "C" fn(&Object, _, _));
builder.add_method(sel!(both:), my_fn as extern "C" fn(&Object, Sel, &Object));
}
}
22 changes: 20 additions & 2 deletions tests/ui/fn_ptr_reference_method2.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,26 @@
error: implementation of `MethodImplementation` is not general enough
--> ui/fn_ptr_reference_method2.rs:12:9
|
12 | builder.add_method(sel!(both:), my_fn as extern "C" fn(&Object, Sel, &Object));
12 | builder.add_method(sel!(first:), my_fn as extern "C" fn(&Object, _, _));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `MethodImplementation` is not general enough
|
= note: `MethodImplementation` would have to be implemented for the type `for<'r> extern "C" fn(&'r objc2::runtime::Object, objc2::runtime::Sel, &objc2::runtime::Object)`
= note: ...but `MethodImplementation` is actually implemented for the type `extern "C" fn(&'0 objc2::runtime::Object, objc2::runtime::Sel, &objc2::runtime::Object)`, for some specific lifetime `'0`

error: implementation of `MethodImplementation` is not general enough
--> ui/fn_ptr_reference_method2.rs:13:9
|
13 | builder.add_method(sel!(both:), my_fn as extern "C" fn(&Object, Sel, &Object));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `MethodImplementation` is not general enough
|
= note: `MethodImplementation` would have to be implemented for the type `for<'r, 's> extern "C" fn(&'r objc2::runtime::Object, objc2::runtime::Sel, &'s objc2::runtime::Object)`
= note: ...but `MethodImplementation` is actually implemented for the type `extern "C" fn(&'0 objc2::runtime::Object, objc2::runtime::Sel, &objc2::runtime::Object)`, for some specific lifetime `'0`

error: implementation of `MethodImplementation` is not general enough
--> ui/fn_ptr_reference_method2.rs:13:9
|
13 | builder.add_method(sel!(both:), my_fn as extern "C" fn(&Object, Sel, &Object));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `MethodImplementation` is not general enough
|
= note: `MethodImplementation` would have to be implemented for the type `for<'r, 's> extern "C" fn(&'r objc2::runtime::Object, objc2::runtime::Sel, &'s objc2::runtime::Object)`
= note: ...but `MethodImplementation` is actually implemented for the type `for<'r> extern "C" fn(&'r objc2::runtime::Object, objc2::runtime::Sel, &'0 objc2::runtime::Object)`, for some specific lifetime `'0`
= note: ...but `MethodImplementation` is actually implemented for the type `extern "C" fn(&objc2::runtime::Object, objc2::runtime::Sel, &'0 objc2::runtime::Object)`, for some specific lifetime `'0`

0 comments on commit 5882f01

Please sign in to comment.