From 5882f01186ee9aa2fe3c0421132f477a6300d92d Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 7 Jul 2022 14:59:49 +0200 Subject: [PATCH] Consistently implement `MethodImplementation` wrt. lifetimes 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 https://github.com/SSheldon/rust-objc/issues/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? --- README.md | 3 +- .../examples/class_with_lifetime.rs | 3 +- objc2-foundation/examples/custom_class.rs | 4 +- objc2/CHANGELOG.md | 28 ++++++++++++ objc2/src/declare.rs | 42 +++++++++--------- objc2/src/rc/id.rs | 2 +- objc2/src/rc/test_object.rs | 29 +++--------- objc2/src/test_utils.rs | 24 ++++++---- tests/ui/fn_ptr_reference_method.rs | 4 +- tests/ui/fn_ptr_reference_method.stderr | 44 +++++-------------- tests/ui/fn_ptr_reference_method2.rs | 1 + tests/ui/fn_ptr_reference_method2.stderr | 22 +++++++++- 12 files changed, 110 insertions(+), 96 deletions(-) diff --git a/README.md b/README.md index 2dbce84a7..6912e6e8e 100644 --- a/README.md +++ b/README.md @@ -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`: diff --git a/objc2-foundation/examples/class_with_lifetime.rs b/objc2-foundation/examples/class_with_lifetime.rs index 23c16561d..7d9c2df17 100644 --- a/objc2-foundation/examples/class_with_lifetime.rs +++ b/objc2-foundation/examples/class_with_lifetime.rs @@ -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); } diff --git a/objc2-foundation/examples/custom_class.rs b/objc2-foundation/examples/custom_class.rs index b808f3877..bb5319fdf 100644 --- a/objc2-foundation/examples/custom_class.rs +++ b/objc2-foundation/examples/custom_class.rs @@ -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); } diff --git a/objc2/CHANGELOG.md b/objc2/CHANGELOG.md index ee7a77e11..a1508b823 100644 --- a/objc2/CHANGELOG.md +++ b/objc2/CHANGELOG.md @@ -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`. diff --git a/objc2/src/declare.rs b/objc2/src/declare.rs index 096e08eb8..51b096044 100644 --- a/objc2/src/declare.rs +++ b/objc2/src/declare.rs @@ -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(_, _) -> _, //! ); //! } //! @@ -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,)*); @@ -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,)*); @@ -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),*); diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index 31ea36393..007ee3cd8 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -520,7 +520,7 @@ impl Id { /// unsafe { /// builder.add_class_method( /// sel!(get), - /// get as extern "C" fn(&Class, Sel) -> *mut Object, + /// get as extern "C" fn(_, _) -> _, /// ); /// } /// diff --git a/objc2/src/rc/test_object.rs b/objc2/src/rc/test_object.rs index 4f2ecf70d..c12a59b00 100644 --- a/objc2/src/rc/test_object.rs +++ b/objc2/src/rc/test_object.rs @@ -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(); diff --git a/objc2/src/test_utils.rs b/objc2/src/test_utils.rs index 2a462b877..b4600c7a5 100644 --- a/objc2/src/test_utils.rs +++ b/objc2/src/test_utils.rs @@ -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(); @@ -115,6 +115,10 @@ pub(crate) fn custom_class() -> &'static Class { unsafe { *this.ivar::("_foo") } } + extern "C" fn custom_obj_get_foo_reference(this: &Object, _cmd: Sel) -> &u32 { + unsafe { this.ivar::("_foo") } + } + extern "C" fn custom_obj_get_struct(_this: &Object, _cmd: Sel) -> CustomStruct { CustomStruct { a: 1, @@ -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); } @@ -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); } diff --git a/tests/ui/fn_ptr_reference_method.rs b/tests/ui/fn_ptr_reference_method.rs index 1eb6f2f74..051f52633 100644 --- a/tests/ui/fn_ptr_reference_method.rs +++ b/tests/ui/fn_ptr_reference_method.rs @@ -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)); } } diff --git a/tests/ui/fn_ptr_reference_method.stderr b/tests/ui/fn_ptr_reference_method.stderr index 4ab1584f1..5e752d583 100644 --- a/tests/ui/fn_ptr_reference_method.stderr +++ b/tests/ui/fn_ptr_reference_method.stderr @@ -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, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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 diff --git a/tests/ui/fn_ptr_reference_method2.rs b/tests/ui/fn_ptr_reference_method2.rs index c74f1c7b9..3f1e00f68 100644 --- a/tests/ui/fn_ptr_reference_method2.rs +++ b/tests/ui/fn_ptr_reference_method2.rs @@ -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)); } } diff --git a/tests/ui/fn_ptr_reference_method2.stderr b/tests/ui/fn_ptr_reference_method2.stderr index 6de5f54b8..dabad7e49 100644 --- a/tests/ui/fn_ptr_reference_method2.stderr +++ b/tests/ui/fn_ptr_reference_method2.stderr @@ -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`