Merge pull request #199 from madsmtm/small-fixes

Smaller fixes

block2/src/global.rs

@@ -16,7 +16,7 @@ const GLOBAL_DESCRIPTOR: ffi::Block_descriptor_header = ffi::Block_descriptor_he
     size: mem::size_of::<ffi::Block_layout>() as c_ulong,
 };
 
-/// An Objective-C block that does not capture it's environment.
+/// An Objective-C block that does not capture its environment.
 ///
 /// This is effectively just a glorified function pointer, and can created and
 /// stored in static memory using the [`global_block!`] macro.
@@ -93,8 +93,8 @@ where
 /// Construct a static [`GlobalBlock`].
 ///
 /// The syntax is similar to a static closure (except that all types have to
-/// be specified). Note that the block cannot capture it's environment, and
-/// it's argument types and return type must be [`Encode`].
+/// be specified). Note that the block cannot capture its environment, and
+/// its argument types and return type must be [`Encode`].
 ///
 /// # Examples
 ///
@@ -139,7 +139,7 @@ where
 /// }
 /// ```
 ///
-/// There is also no way to get a block function that's generic over it's
+/// There is also no way to get a block function that's generic over its
 /// arguments. One could imagine the following syntax would work, but it can't
 /// due to implementation limitations:
 ///

objc-sys/build.rs

@@ -176,7 +176,7 @@ fn main() {
                 (MacOS(_), _) => "macosx",
                 (IOS(_), _) => "ios",
                 (WatchOS(_), _) => "watchos",
-                // tvOS doesn't have it's own -fobjc-runtime string
+                // tvOS doesn't have its own -fobjc-runtime string
                 (TvOS(_), _) => "ios",
             };
             match runtime {

objc2-foundation/src/macros.rs

@@ -25,7 +25,7 @@
 /// Finally, [`AsRef`], [`AsMut`], [`Borrow`] and [`BorrowMut`] are
 /// implemented to allow conversion to an arbitary superclasses in the
 /// inheritance chain (since an instance of a class can always be interpreted
-/// as it's superclasses).
+/// as its superclasses).
 ///
 /// [`Deref`]: core::ops::Deref
 /// [`DerefMut`]: core::ops::DerefMut
@@ -35,9 +35,12 @@
 ///
 /// # Safety
 ///
-/// The specified inheritance chain must be correct (including in the correct
-/// order), and the types in said chain must be valid as Objective-C objects -
-/// this is easiest to ensure by also creating those using this macro.
+/// The specified inheritance chain must be correct, including in the correct
+/// order, and the types in said chain must be valid as Objective-C objects
+/// (this is easy to ensure by also creating those using this macro).
+///
+/// The object must respond to standard memory management messages (this is
+/// upheld if `NSObject` is part of its inheritance chain).
 ///
 ///
 /// # Example
@@ -208,15 +211,27 @@ macro_rules! __inner_extern_class {
             $($p_v $p: $pty),*
         }
 
-        unsafe impl<$($t $(: $b)?),*> $crate::objc2::Message for $name<$($t),*> { }
-
+        // SAFETY:
+        // - The item is FFI-safe with `#[repr(C)]`.
+        // - The encoding is taken from the inner item, and caller verifies
+        //   that it actually inherits said object.
+        // - The rest of the struct's fields are ZSTs, so they don't influence
+        //   the layout.
         unsafe impl<$($t $(: $b)?),*> $crate::objc2::RefEncode for $name<$($t),*> {
             const ENCODING_REF: $crate::objc2::Encoding<'static>
                 = <$inherits as $crate::objc2::RefEncode>::ENCODING_REF;
         }
 
+        // SAFETY: This is essentially just a newtype wrapper over `Object`
+        // (we even ensure that `Object` is always last in our inheritance
+        // tree), so it is always safe to reinterpret as that.
+        //
+        // That the object must work with standard memory management is upheld
+        // by the caller.
+        unsafe impl<$($t $(: $b)?),*> $crate::objc2::Message for $name<$($t),*> {}
+
         // SAFETY: An instance can always be _used_ in exactly the same way as
-        // it's superclasses (though not necessarily _constructed_ in the same
+        // its superclasses (though not necessarily _constructed_ in the same
         // way, but `Deref` doesn't allow this).
         //
         // Remember; while we (the Rust side) may intentionally be forgetting

objc2/CHANGELOG.md

@@ -15,14 +15,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   ```rust
   // Before
   let obj: Id<Object, Shared> = unsafe {
-      let obj: *mut Self = msg_send![Self::class(), alloc];
-      let obj: *mut Self = msg_send![obj, init];
+      let obj: *mut Object = msg_send![class!(MyObject), alloc];
+      let obj: *mut Object = msg_send![obj, init];
       Id::new(obj).unwrap()
   };
 
   // After
   let obj: Id<Object, Shared> = unsafe {
-      msg_send_id![msg_send_id![Self::class(), alloc], new].unwrap()
+      msg_send_id![msg_send_id![class!(MyObject), alloc], new].unwrap()
   };
   ```
 * Added the `"unstable-static-sel"` and `"unstable-static-sel-inlined"`
@@ -41,6 +41,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   exceptions.
 * Added `declare::Ivar<T>` helper struct. This is useful for building safe
   abstractions that access instance variables.
+* Added `Id::from_owned` helper function.
 
 ### Changed
 * **BREAKING**: `Sel` is now required to be non-null, which means that you
@@ -88,7 +89,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * **BREAKING**: Disallow throwing `nil` exceptions in `exception::throw`.
 
 ### Removed
-* **BREAKING**: Removed the `Sel::from_ptr` and `Sel::as_ptr` methods.
+* **BREAKING**: Removed the `Sel::from_ptr` method.
 * **BREAKING**: Removed `MessageError`.
 
 
@@ -149,7 +150,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 * **BREAKING**: `Class` no longer implements `Message` (but it can still be
   used as the receiver in `msg_send!`, so this is unlikely to break anything
   in practice).
-* **BREAKING**: Sealed the `MethodImplementation` trait, and made it's `imp`
+* **BREAKING**: Sealed the `MethodImplementation` trait, and made its `imp`
   method privat.
 * **BREAKING**: Updated `ffi` module to `objc-sys v0.2.0-beta.0`.
 * **BREAKING**: Updated `objc2-encode` (`Encoding`, `Encode`, `RefEncode` and

objc2/Cargo.toml

@@ -44,7 +44,7 @@ malloc = ["malloc_buf"]
 # Make the `sel!` macro look up the selector statically.
 #
 # The plan is to enable this by default, but right now we are uncertain of
-# it's stability, and it might need significant changes before being fully
+# its stability, and it might need significant changes before being fully
 # ready!
 #
 # Please test it, and report any issues you may find:

objc2/src/declare.rs

@@ -485,6 +485,49 @@ mod tests {
         assert!(ClassBuilder::new("TestClassBuilderDuplicate", cls).is_none());
     }
 
+    #[test]
+    #[should_panic = "Failed to add ivar xyz"]
+    fn duplicate_ivar() {
+        let cls = test_utils::custom_class();
+        let builder = ClassBuilder::new("TestClassBuilderDuplicateIvar", cls).unwrap();
+        // ManuallyDrop to work around GNUStep issue
+        let mut builder = ManuallyDrop::new(builder);
+
+        builder.add_ivar::<i32>("xyz");
+        // Should panic:
+        builder.add_ivar::<i32>("xyz");
+    }
+
+    #[test]
+    #[should_panic = "Failed to add method xyz"]
+    fn duplicate_method() {
+        let cls = test_utils::custom_class();
+        let builder = ClassBuilder::new("TestClassBuilderDuplicateMethod", cls).unwrap();
+        let mut builder = ManuallyDrop::new(builder);
+
+        extern "C" fn xyz(_this: &Object, _cmd: Sel) {}
+
+        unsafe {
+            builder.add_method(sel!(xyz), xyz as extern "C" fn(_, _));
+            // Should panic:
+            builder.add_method(sel!(xyz), xyz as extern "C" fn(_, _));
+        }
+    }
+
+    #[test]
+    #[should_panic = "Failed to add protocol NSObject"]
+    fn duplicate_protocol() {
+        let cls = test_utils::custom_class();
+        let builder = ClassBuilder::new("TestClassBuilderDuplicateProtocol", cls).unwrap();
+        let mut builder = ManuallyDrop::new(builder);
+
+        let protocol = Protocol::get("NSObject").unwrap();
+
+        builder.add_protocol(protocol);
+        // Should panic:
+        builder.add_protocol(protocol);
+    }
+
     #[test]
     #[cfg_attr(
         feature = "gnustep-1-7",

objc2/src/lib.rs

@@ -2,7 +2,7 @@
 //!
 //! Objective-C is[^note] the standard programming language on Apple platforms
 //! like macOS, iOS, iPadOS, tvOS and watchOS. It is an object-oriented
-//! language centered around "sending messages" to it's instances - this can
+//! language centered around "sending messages" to its instances - this can
 //! for the most part be viewed as a simple method call.
 //!
 //! Most of the core libraries and frameworks that are in use on Apple systems

objc2/src/message/apple/mod.rs

@@ -17,7 +17,7 @@ mod arch;
 mod arch;
 
 /// On the above architectures we can statically find the correct method to
-/// call from the return type, by looking at it's `Encode` implementation.
+/// call from the return type, by looking at its `Encode` implementation.
 #[allow(clippy::missing_safety_doc)]
 unsafe trait MsgSendFn: Encode {
     const MSG_SEND: Imp;

objc2/src/rc/autorelease.rs

@@ -294,7 +294,7 @@ impl !AutoreleaseSafe for AutoreleasePool {}
 ///         let obj = needs_lifetime_from_pool(outer_pool);
 ///         obj
 ///     });
-///     // `obj` could wrongly be used here because it's lifetime was
+///     // `obj` could wrongly be used here because its lifetime was
 ///     // assigned to the outer pool, even though it was released by the
 ///     // inner pool already.
 /// });

objc2/src/rc/id.rs

@@ -27,13 +27,14 @@ use crate::Message;
 /// may be other references to the object, since a shared [`Id`] can be cloned
 /// to provide exactly that.
 ///
-/// An [`Id<T, Owned>`] can be safely "downgraded", that is, turned into to a
-/// `Id<T, Shared>` using `From`/`Into`. The opposite is not safely possible,
+/// An [`Id<T, Owned>`] can be safely converted to a [`Id<T, Shared>`] using
+/// [`Id::from_owned`] or `From`/`Into`. The opposite is not safely possible,
 /// but the unsafe option [`Id::from_shared`] is provided.
 ///
 /// `Option<Id<T, O>>` is guaranteed to have the same size as a pointer to the
 /// object.
 ///
+///
 /// # Comparison to `std` types
 ///
 /// `Id<T, Owned>` can be thought of as the Objective-C equivalent of [`Box`]
@@ -267,8 +268,8 @@ impl<T: Message, O: Ownership> Id<T, O> {
     /// a specific class (e.g. casting an instance of `NSString` to `NSObject`
     /// is safe because `NSString` is a subclass of `NSObject`).
     ///
-    /// All objects can safely be cast to [`Object`], since that assumes no
-    /// specific class.
+    /// All `'static` objects can safely be cast to [`Object`], since that
+    /// assumes no specific class.
     ///
     /// [`Object`]: crate::runtime::Object
     ///
@@ -278,6 +279,9 @@ impl<T: Message, O: Ownership> Id<T, O> {
     /// You must ensure that the object can be reinterpreted as the given
     /// type.
     ///
+    /// If `T` is not `'static`, you must ensure that `U` ensures that the
+    /// data contained by `T` is kept alive for as long as `U` lives.
+    ///
     /// Additionally, you must ensure that any safety invariants that the new
     /// type has are upheld.
     #[inline]
@@ -608,16 +612,29 @@ impl<T: Message> Id<T, Shared> {
     }
 }
 
-impl<T: Message + ?Sized> From<Id<T, Owned>> for Id<T, Shared> {
-    /// Downgrade from an owned to a shared [`Id`], allowing it to be cloned.
+impl<T: Message + ?Sized> Id<T, Shared> {
+    /// Convert an owned to a shared [`Id`], allowing it to be cloned.
+    ///
+    /// This is also implemented as a `From` conversion, but this name is more
+    /// explicit, which may be useful in some cases.
     #[inline]
-    fn from(obj: Id<T, Owned>) -> Self {
+    pub fn from_owned(obj: Id<T, Owned>) -> Self {
         let ptr = ManuallyDrop::new(obj).ptr;
         // SAFETY: The pointer is valid, and ownership is simply decreased
         unsafe { <Id<T, Shared>>::new_nonnull(ptr) }
     }
 }
 
+impl<T: Message + ?Sized> From<Id<T, Owned>> for Id<T, Shared> {
+    /// Convert an owned to a shared [`Id`], allowing it to be cloned.
+    ///
+    /// Same as [`Id::from_owned`].
+    #[inline]
+    fn from(obj: Id<T, Owned>) -> Self {
+        Self::from_owned(obj)
+    }
+}
+
 // TODO: Add ?Sized bound
 impl<T: Message> Clone for Id<T, Shared> {
     /// Makes a clone of the shared object.

objc2/src/rc/mod.rs

@@ -8,12 +8,12 @@
 //!
 //! To enforce aliasing rules, an `Id` can be either owned or shared; if it is
 //! owned, meaning the `Id` is the only reference to the object, it can be
-//! mutably dereferenced. An owned `Id` can be downgraded to a shared `Id`
+//! mutably dereferenced. An owned `Id` can be converted to a shared `Id`,
 //! which can be cloned to allow multiple references.
 //!
 //! Weak references may be created using the [`WeakId`] struct; these will not
-//! retain the object, but they can upgraded to an `Id` in a manner that
-//! safely fails if the object has been deallocated.
+//! retain the object, but one can attempt to load them and obtain an `Id`, or
+//! safely fail if the object has been deallocated.
 //!
 //! See [the clang documentation][clang-arc] and [the Apple article on memory
 //! management][mem-mgmt] (similar document exists [for Core Foundation][cf])

objc2/src/rc/weak_id.rs

@@ -30,7 +30,7 @@ pub struct WeakId<T: ?Sized> {
     /// TODO: Investigate if we can avoid some allocations using `Pin`.
     inner: Box<UnsafeCell<*mut ffi::objc_object>>,
     /// WeakId inherits variance, dropck and various marker traits from
-    /// `Id<T, Shared>` because it can be upgraded to a shared Id.
+    /// `Id<T, Shared>` because it can be loaded as a shared Id.
     item: PhantomData<Id<T, Shared>>,
 }
 
@@ -63,7 +63,6 @@ impl<T: Message> WeakId<T> {
     ///
     /// Returns [`None`] if the object has been deallocated or was created
     /// with [`Default::default`].
-    #[doc(alias = "upgrade")]
     #[doc(alias = "retain")]
     #[doc(alias = "objc_loadWeak")]
     #[doc(alias = "objc_loadWeakRetained")]

objc2/src/runtime.rs

@@ -125,8 +125,11 @@ impl Sel {
         NonNull::new(ptr as *mut ffi::objc_selector).map(|ptr| Self { ptr })
     }
 
+    /// Get a pointer to the raw selector.
+    ///
+    /// Useful when working with raw FFI methods.
     #[inline]
-    pub(crate) const fn as_ptr(&self) -> *const ffi::objc_selector {
+    pub const fn as_ptr(&self) -> *const ffi::objc_selector {
         self.ptr.as_ptr()
     }
 

tests/extern/encode_utils.m

@@ -83,6 +83,19 @@
 };
 ENCODING(STRUCT_WITH_BLOCK, struct with_block);
 
+struct with_atomic_inner {
+    _Atomic int a;
+    _Atomic int* b;
+};
+struct with_atomic {
+    _Atomic int a;
+    _Atomic const int* b;
+    struct with_atomic_inner c;
+    struct with_atomic_inner* d;
+    _Atomic struct with_atomic_inner* e;
+};
+ENCODING(STRUCT_WITH_ATOMIC, struct with_atomic);
+
 // Bit field
 
 struct bitfield {

tests/src/test_encode_utils.rs

@@ -241,6 +241,10 @@ assert_inner!(str ENCODING_STRUCT_WITH_BLOCK => WITH_BLOCK);
 assert_inner!(str ENCODING_STRUCT_WITH_BLOCK_POINTER => Encoding::Pointer(&WITH_BLOCK));
 assert_inner!(str ENCODING_STRUCT_WITH_BLOCK_ATOMIC => "A{with_block}");
 
+assert_inner!(str ENCODING_STRUCT_WITH_ATOMIC => "{with_atomic=Ai^Ai{with_atomic_inner=Ai^Ai}^{with_atomic_inner}^A{with_atomic_inner}}");
+assert_inner!(str ENCODING_STRUCT_WITH_ATOMIC_POINTER => "^{with_atomic=Ai^Ai{with_atomic_inner=Ai^Ai}^{with_atomic_inner}^A{with_atomic_inner}}");
+assert_inner!(str ENCODING_STRUCT_WITH_ATOMIC_ATOMIC => "A{with_atomic}");
+
 // Bitfields
 
 #[cfg(feature = "apple")]