Merge pull request #477 from madsmtm/declare-class-tests

Improve `declare_class!` tests

.github/workflows/ci.yml

@@ -254,6 +254,11 @@ jobs:
     - name: Run all assembly tests
       if: ${{ env.FULL }}
       run: ./helper-scripts/run-assembly-tests.sh
+      env:
+        TEST_OVERWRITE: 1
+
+    - name: Check diff
+      run: git diff --exit-code
 
   header-translator:
     name: Verify header translator output

crates/objc2/src/__macro_helpers/declare_class.rs

@@ -1,6 +1,3 @@
-use core::mem::ManuallyDrop;
-use core::ptr;
-
 use crate::declare::__IdReturnValue;
 use crate::rc::{Allocated, Id};
 use crate::{ClassType, Message, MessageReceiver};
@@ -85,7 +82,7 @@ pub trait MaybeOptionId: MaybeUnwrap {
 impl<T: Message> MaybeOptionId for Id<T> {
     #[inline]
     fn consumed_return(self) -> __IdReturnValue {
-        let ptr: *mut T = Id::consume_as_ptr(ManuallyDrop::new(self));
+        let ptr: *mut T = Id::consume_as_ptr(self);
         __IdReturnValue(ptr.cast())
     }
 
@@ -99,9 +96,7 @@ impl<T: Message> MaybeOptionId for Id<T> {
 impl<T: Message> MaybeOptionId for Option<Id<T>> {
     #[inline]
     fn consumed_return(self) -> __IdReturnValue {
-        let ptr: *mut T = self
-            .map(|this| Id::consume_as_ptr(ManuallyDrop::new(this)))
-            .unwrap_or_else(ptr::null_mut);
+        let ptr: *mut T = Id::consume_as_ptr_option(self);
         __IdReturnValue(ptr.cast())
     }
 

crates/objc2/src/declare/mod.rs

@@ -832,10 +832,7 @@ mod tests {
     }
 
     #[test]
-    #[cfg_attr(
-        debug_assertions,
-        should_panic = "selector xyz: accepts 1 arguments, but function accepts 0"
-    )]
+    #[should_panic = "selector xyz: accepts 1 arguments, but function accepts 0"]
     fn wrong_arguments() {
         let cls = test_utils::custom_class();
         let mut builder = ClassBuilder::new("TestClassBuilderWrongArguments", cls).unwrap();

crates/objc2/src/encode/__unstable.rs

@@ -30,21 +30,26 @@ mod return_private {
 ///
 /// We currently don't need a similar `EncodeArgument` trait, but we might in
 /// the future.
+///
+///
+/// # Safety
+///
+/// Similar to [`Encode`].
 //
 // Note: While this is not public, it is still a breaking change to change,
 // since `block2` relies on it.
-pub trait EncodeReturn: return_private::Sealed {
+pub unsafe trait EncodeReturn: return_private::Sealed {
     /// The Objective-C type-encoding for this type.
     const ENCODING_RETURN: Encoding;
 }
 
 impl return_private::Sealed for () {}
-impl EncodeReturn for () {
+unsafe impl EncodeReturn for () {
     const ENCODING_RETURN: Encoding = Encoding::Void;
 }
 
 impl<T: Encode> return_private::Sealed for T {}
-impl<T: Encode> EncodeReturn for T {
+unsafe impl<T: Encode> EncodeReturn for T {
     const ENCODING_RETURN: Encoding = T::ENCODING;
 }
 

crates/objc2/src/message/mod.rs

@@ -454,7 +454,7 @@ unsafe impl<T: ?Sized + Message> MessageReceiver for ManuallyDrop<Id<T>> {
 
     #[inline]
     fn __as_raw_receiver(self) -> *mut AnyObject {
-        Id::consume_as_ptr(self).cast()
+        Id::consume_as_ptr(ManuallyDrop::into_inner(self)).cast()
     }
 }
 

crates/objc2/src/rc/id.rs

@@ -239,8 +239,17 @@ impl<T: ?Sized + Message> Id<T> {
     }
 
     #[inline]
-    pub(crate) fn consume_as_ptr(this: ManuallyDrop<Self>) -> *mut T {
-        this.ptr.as_ptr()
+    pub(crate) fn consume_as_ptr(this: Self) -> *mut T {
+        ManuallyDrop::new(this).ptr.as_ptr()
+    }
+
+    #[inline]
+    pub(crate) fn consume_as_ptr_option(this: Option<Self>) -> *mut T
+    where
+        T: Sized,
+    {
+        this.map(|this| Id::consume_as_ptr(this))
+            .unwrap_or_else(ptr::null_mut)
     }
 }
 

crates/objc2/src/rc/mod.rs

@@ -63,5 +63,6 @@ pub use self::autorelease::{
 };
 pub use self::id::Id;
 pub use self::id_traits::{DefaultId, IdFromIterator, IdIntoIterator};
+#[doc(hidden)]
 pub use self::test_object::{__RcTestObject, __ThreadTestData};
 pub use self::weak_id::WeakId;

crates/objc2/tests/declare_class_self.rs

@@ -1,7 +1,6 @@
 //! To remind myself that `Self` needs to work in methods in `declare_class!`,
-//! and hence we _must_ implement things by changing the generated method, we
-//! can't just create an internal helper function (since we can't name the
-//! types of such a function)!
+//! and hence whenever we name any of the types involved in this, we need to
+//! do it in a context where `Self` works.
 use objc2::rc::{Allocated, Id};
 use objc2::runtime::NSObject;
 use objc2::{declare_class, mutability, ClassType};
@@ -14,6 +13,14 @@ impl<T: ?Sized> GetSameType for T {
     type SameType = T;
 }
 
+trait GetId {
+    type IdType;
+}
+
+impl<T> GetId for T {
+    type IdType = Id<T>;
+}
+
 macro_rules! get_self {
     () => {
         Self
@@ -31,16 +38,16 @@ declare_class!(
     }
 
     unsafe impl MyTestObject {
-        #[method_id(init)]
+        #[method_id(initWith:)]
         fn init(
             _this: Allocated<<Self as GetSameType>::SameType>,
             _param: <*const Self as GetSameType>::SameType,
         ) -> Id<<Self as GetSameType>::SameType> {
             unimplemented!()
         }
 
-        #[method(compare:)]
-        fn compare(&self, _other: &Self) -> bool {
+        #[method(isEqual:)]
+        fn is_equal(&self, _other: &Self) -> bool {
             unimplemented!()
         }
 
@@ -49,7 +56,15 @@ declare_class!(
         fn test4(_this: &<(Self) as GetSameType>::SameType) -> Id<get_self!()> {
             unimplemented!()
         }
+
+        #[method_id(test5)]
+        fn test5(&self) -> <Self as GetId>::IdType {
+            unimplemented!()
+        }
     }
 );
 
-fn main() {}
+#[test]
+fn create_class() {
+    let _ = MyTestObject::class();
+}

crates/objc2/tests/no_prelude.rs

@@ -143,11 +143,13 @@ new_objc2::extern_protocol!(
     unsafe impl ProtocolType for dyn CustomProtocol {}
 );
 
+#[test]
 pub fn test_selector() {
     let _sel = new_objc2::sel!(abc);
     let _sel = new_objc2::sel!(abc:def:);
 }
 
+#[test]
 pub fn test_class() {
     let _class = new_objc2::class!(NSObject);
 }

crates/objc2/tests/use_macros.rs

@@ -3,7 +3,7 @@ use objc2::runtime::{AnyClass, NSObject};
 use objc2::{class, declare_class, msg_send, sel, ClassType};
 
 declare_class!(
-    struct MyObject;
+    pub struct MyObject;
 
     unsafe impl ClassType for MyObject {
         type Super = NSObject;
@@ -28,8 +28,7 @@ fn use_sel() {
     let _sel = sel!(setObject:forKey:);
 }
 
-#[allow(unused)]
-fn test_msg_send_comma_handling(obj: &MyObject, superclass: &AnyClass) {
+pub fn test_msg_send_comma_handling(obj: &MyObject, superclass: &AnyClass) {
     unsafe {
         let _: () = msg_send![obj, a];
         let _: () = msg_send![obj, a,];

crates/test-assembly/crates/test_declare_class/expected/apple-aarch64.s

@@ -783,7 +783,7 @@ l_anon.[ID].2:
 	.ascii	"called `Option::unwrap()` on a `None` value"
 
 l_anon.[ID].3:
-	.ascii	"/Users/madsmarquart/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sync/once.rs"
+	.ascii	"$RUSTC/library/std/src/sync/once.rs"
 
 	.section	__DATA,__const
 	.p2align	3, 0x0

crates/test-assembly/crates/test_declare_class/expected/apple-armv7.s

@@ -711,7 +711,7 @@ l_anon.[ID].2:
 	.ascii	"called `Option::unwrap()` on a `None` value"
 
 l_anon.[ID].3:
-	.ascii	"/Users/madsmarquart/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sync/once.rs"
+	.ascii	"$RUSTC/library/std/src/sync/once.rs"
 
 	.section	__DATA,__const
 	.p2align	2, 0x0

crates/test-assembly/crates/test_declare_class/expected/apple-armv7s.s

@@ -714,7 +714,7 @@ l_anon.[ID].2:
 	.ascii	"called `Option::unwrap()` on a `None` value"
 
 l_anon.[ID].3:
-	.ascii	"/Users/madsmarquart/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sync/once.rs"
+	.ascii	"$RUSTC/library/std/src/sync/once.rs"
 
 	.section	__DATA,__const
 	.p2align	2, 0x0

crates/test-assembly/crates/test_declare_class/expected/apple-old-x86.s

@@ -736,7 +736,7 @@ l_anon.[ID].2:
 	.ascii	"called `Option::unwrap()` on a `None` value"
 
 l_anon.[ID].3:
-	.ascii	"/Users/madsmarquart/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sync/once.rs"
+	.ascii	"$RUSTC/library/std/src/sync/once.rs"
 
 	.section	__DATA,__const
 	.p2align	2, 0x0

crates/test-assembly/crates/test_declare_class/expected/apple-x86.s

@@ -736,7 +736,7 @@ l_anon.[ID].2:
 	.ascii	"called `Option::unwrap()` on a `None` value"
 
 l_anon.[ID].3:
-	.ascii	"/Users/madsmarquart/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sync/once.rs"
+	.ascii	"$RUSTC/library/std/src/sync/once.rs"
 
 	.section	__DATA,__const
 	.p2align	2, 0x0

crates/test-assembly/src/lib.rs

@@ -64,6 +64,8 @@ pub fn read_assembly<P: AsRef<Path>>(path: P, package_path: &Path) -> io::Result
         .as_os_str()
         .to_str()
         .unwrap();
+
+    // Replace paths
     let s = s.replace(workspace_dir, "$WORKSPACE");
     let s = s.replace(
         package_path
@@ -79,6 +81,14 @@ pub fn read_assembly<P: AsRef<Path>>(path: P, package_path: &Path) -> io::Result
     let s = regex::Regex::new(r"/rustc/[0-9a-f]*/")
         .unwrap()
         .replace_all(&s, |_: &regex::Captures| "$RUSTC/");
+    let s = regex::Regex::new(r"/.*/rustlib/src/rust/")
+        .unwrap()
+        .replace_all(&s, |_: &regex::Captures| "$RUSTC/");
+
+    // HACK: Make location data the same no matter which platform generated
+    // the data.
+    let s = s.replace(".asciz\t\"}", ".asciz\t\"t");
+
     // HACK: Replace Objective-C image info for simulator targets
     let s = s.replace(
         ".asciz\t\"\\000\\000\\000\\000`\\000\\000\"",

crates/test-ui/ui/declare_class_invalid_receiver.rs

@@ -1,5 +1,5 @@
 use objc2::rc::{Allocated, Id};
-use objc2::runtime::NSObject;
+use objc2::runtime::{AnyClass, NSObject};
 use objc2::{declare_class, mutability, ClassType};
 
 declare_class!(
@@ -12,47 +12,52 @@ declare_class!(
     }
 
     unsafe impl CustomObject {
-        #[method(test1)]
-        fn test1(self: Box<Self>) {
+        #[method(testBox)]
+        fn test_box(self: Box<Self>) {
             unimplemented!()
         }
 
-        #[method(test2)]
-        fn test2(this: Id<Self>) {
+        #[method(testIdSelf)]
+        fn test_id_self(this: Id<Self>) {
             unimplemented!()
         }
 
-        #[method(test3)]
-        fn test3(this: Self) {
+        #[method(testSelf)]
+        fn test_self(this: Self) {
+            unimplemented!()
+        }
+
+        #[method(testClass)]
+        fn test_class(this: &AnyClass) {
             unimplemented!()
         }
     }
 
     unsafe impl CustomObject {
-        #[method_id(test4)]
-        fn test4(self: Box<Self>) -> Id<Self> {
+        #[method_id(testBoxId)]
+        fn test_box_id(self: Box<Self>) -> Id<Self> {
             unimplemented!()
         }
 
-        #[method_id(test5)]
-        fn test5(this: Id<Self>) -> Id<Self> {
+        #[method_id(testIdSelfId)]
+        fn test_id_self_id(this: Id<Self>) -> Id<Self> {
             unimplemented!()
         }
 
-        #[method_id(test6)]
-        fn test6(this: Self) -> Id<Self> {
+        #[method_id(testSelfId)]
+        fn test_self_id(this: Self) -> Id<Self> {
             unimplemented!()
         }
     }
 
     unsafe impl CustomObject {
-        #[method_id(test7)]
-        fn test7(this: Allocated<Self>) -> Id<Self> {
+        #[method_id(testAlloc)]
+        fn test_alloc(this: Allocated<Self>) -> Id<Self> {
             unimplemented!()
         }
 
-        #[method_id(initTest8)]
-        fn test8(&self) -> Id<Self> {
+        #[method_id(initTestNotAlloc)]
+        fn test_init_not_alloc(&self) -> Id<Self> {
             unimplemented!()
         }
     }

crates/test-ui/ui/declare_class_invalid_receiver.stderr

@@ -103,6 +103,36 @@ note: required by a bound in `ClassBuilder::add_method`
   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `ClassBuilder::add_method`
   = 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)
 
+error[E0277]: the trait bound `AnyClass: Message` is not satisfied
+ --> ui/declare_class_invalid_receiver.rs
+  |
+  | / declare_class!(
+  | |     struct CustomObject;
+  | |
+  | |     unsafe impl ClassType for CustomObject {
+... |
+  | |     }
+  | | );
+  | |_^ the trait `Message` is not implemented for `AnyClass`
+  |
+  = help: the following other types implement trait `Message`:
+            AnyObject
+            CustomObject
+            Exception
+            NSObject
+            ProtocolObject<P>
+            __NSProxy
+            __RcTestObject
+note: required by a bound in `ClassBuilder::add_method`
+ --> $WORKSPACE/crates/objc2/src/declare/mod.rs
+  |
+  |     pub unsafe fn add_method<T, F>(&mut self, sel: Sel, func: F)
+  |                   ---------- required by a bound in this associated function
+  |     where
+  |         T: Message + ?Sized,
+  |            ^^^^^^^ required by this bound in `ClassBuilder::add_method`
+  = note: this error originates in the macro `$crate::__rewrite_self_arg_inner` which comes from the expansion of the macro `declare_class` (in Nightly builds, run with -Z macro-backtrace for more info)
+
 error[E0277]: the trait bound `extern "C" fn(Box<CustomObject>, objc2::runtime::Sel) -> __IdReturnValue: MethodImplementation` is not satisfied
  --> ui/declare_class_invalid_receiver.rs
   |