Merge pull request #503 from madsmtm/small-fixes

Small fixes

Author: madsmtm

crates/block-sys/src/lib.rs

@@ -22,6 +22,7 @@
 #![warn(clippy::ptr_as_ptr)]
 #![warn(clippy::missing_errors_doc)]
 #![warn(clippy::missing_panics_doc)]
+#![allow(non_camel_case_types)]
 // Update in Cargo.toml as well.
 #![doc(html_root_url = "https://docs.rs/block-sys/0.2.0")]
 #![cfg_attr(feature = "unstable-docsrs", feature(doc_auto_cfg, doc_cfg_hide))]

crates/header-translator/README.md

@@ -14,7 +14,7 @@ Make sure you have the same XCode version installed as the one documented in [`c
 If you use a different operating system than macOS, or have multiple SDKs installed, you can specify the directory as the first argument:
 
 ```console
-cargo run --bin header-translator -- /Applications/Xcode_new.app/Contents/Developer
+cargo run --bin header-translator -- /Applications/Xcode.app/Contents/Developer
 ```
 
 Note that you will likely need to use a newer `libclang`, such as [the one distributed with Swift `5.7.2`](https://github.com/apple/llvm-project/tree/swift-5.7.2-RELEASE). You can use a different version as follows (details might vary between operating systems):

crates/icrate/CHANGELOG.md

@@ -305,7 +305,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   unsound before).
 
 ### Fixed
-* Fixed `NSZone` not being `#[repr(C)]`.
+* Fixed `NSZone` not specifying a `#[repr(...)]`.
 
 
 ## objc2::foundation 0.3.0-beta.3 - 2022-09-01
@@ -470,7 +470,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 * Soundness issue with `NSValue`, `NSDictionary`, `NSArray` and
-  `NSMutableArray` not being `#[repr(C)]`.
+  `NSMutableArray` not specifying a `#[repr(...)]`.
 * **BREAKING**: `NSObject` is no longer `Send` and `Sync` (because its
   subclasses may not be).
 

crates/objc-sys/src/object.rs

@@ -10,6 +10,10 @@ use crate::{objc_class, OpaqueData};
 #[repr(C)]
 pub struct objc_object {
     // `isa` field is deprecated, so we don't expose it here.
+    //
+    // Also, we need this to be a zero-sized, so that the compiler doesn't
+    // assume anything about the layout.
+    //
     // Use `object_getClass` instead.
     _priv: [u8; 0],
     _p: OpaqueData,

crates/objc2/src/declare/mod.rs

@@ -152,7 +152,9 @@ pub(crate) mod private {
 ///
 /// This is a sealed trait that is implemented for a lot of `extern "C"`
 /// function pointer types.
-pub trait MethodImplementation: private::Sealed {
+//
+// Note: `Sized` is intentionally added to make the trait not object safe.
+pub trait MethodImplementation: private::Sealed + Sized {
     /// The callee type of the method.
     type Callee: RefEncode + ?Sized;
     /// The return type of the method.

crates/objc2/src/encode.rs

@@ -196,6 +196,7 @@ pub unsafe trait RefEncode {
     /// [`Encoding::Class`], [`Encoding::Pointer`], [`Encoding::Sel`] or
     /// [`Encoding::Unknown`].
     ///
+    ///
     /// # Examples
     ///
     /// This is usually implemented either as an object pointer:

crates/objc2/src/mutability.rs

@@ -15,7 +15,9 @@
 //! (can be done inside [`extern_class!`] and [`declare_class!`]).
 //!
 //! Note that precious little of Objective-C follows Rust's usual shared xor
-//! unique ownership model, most often objects assume interior mutability.
+//! unique ownership model, most often objects assume interior mutability, so
+//! a safe default is often [`InteriorMutable`], or of you're working with GUI
+//! code, [`MainThreadOnly`].
 //!
 //! [`UnsafeCell`]: core::cell::UnsafeCell
 //! [`ClassType::Mutability`]: crate::ClassType::Mutability
@@ -248,6 +250,9 @@ mod private {
     impl MutabilityIsMutable for Mutable {}
     impl<IS: ?Sized> MutabilityIsMutable for MutableWithImmutableSuperclass<IS> {}
 
+    pub trait MutabilityIsMainThreadOnly: Mutability {}
+    impl MutabilityIsMainThreadOnly for MainThreadOnly {}
+
     // TODO: Trait for objects whose `hash` is guaranteed to never change,
     // which allows it to be used as a key in `NSDictionary`.
 }
@@ -256,6 +261,8 @@ mod private {
 ///
 /// This is a sealed trait, and should not need to be implemented. Open an
 /// issue if you know a use-case where this restrition should be lifted!
+//
+// Note: `Sized` is intentionally added to make the trait not object safe.
 pub trait Mutability: private::Sealed + Sized {}
 impl Mutability for Root {}
 impl Mutability for Immutable {}
@@ -337,7 +344,10 @@ impl<T: ?Sized + ClassType> IsMutable for T where T::Mutability: private::Mutabi
 //
 // Note: MainThreadMarker::from relies on this.
 pub trait IsMainThreadOnly: ClassType {}
-impl<T: ?Sized + ClassType<Mutability = MainThreadOnly>> IsMainThreadOnly for T {}
+impl<T: ?Sized + ClassType> IsMainThreadOnly for T where
+    T::Mutability: private::MutabilityIsMainThreadOnly
+{
+}
 
 #[cfg(test)]
 mod tests {

crates/objc2/src/rc/autorelease.rs

@@ -551,6 +551,9 @@ mod tests {
         pool
     }
 
+    #[allow(unused)]
+    fn assert_object_safe(_: &dyn AutoreleaseSafe) {}
+
     #[cfg_attr(
         not(feature = "unstable-autoreleasesafe"),
         ignore = "only stably ZST when `unstable-autoreleasesafe` is enabled"

crates/objc2/src/rc/weak_id.rs

@@ -27,7 +27,7 @@ use crate::{ffi, Message};
 ///
 /// [`sync::Weak`]: std::sync::Weak
 /// [`sync::Arc`]: std::sync::Arc
-#[repr(transparent)]
+#[repr(transparent)] // This is not a public guarantee
 pub struct WeakId<T: ?Sized> {
     /// We give the runtime the address to this box, so that it can modify it
     /// even if the `WeakId` is moved.

crates/objc2/src/runtime/mod.rs

@@ -1281,6 +1281,7 @@ impl fmt::Debug for AnyObject {
 mod tests {
     use alloc::format;
     use alloc::string::ToString;
+    use core::mem::size_of;
 
     use super::*;
     use crate::test_utils;
@@ -1562,4 +1563,18 @@ mod tests {
         };
         assert_eq!(res, 24);
     }
+
+    #[test]
+    fn test_sizes() {
+        assert_eq!(size_of::<Sel>(), size_of::<*const ()>());
+        assert_eq!(size_of::<Sel>(), size_of::<Option<Sel>>());
+
+        // These must be zero-sized until we get extern types, otherwise the
+        // optimizer may invalidly assume something about their layout.
+        assert_eq!(size_of::<AnyClass>(), 0);
+        assert_eq!(size_of::<AnyObject>(), 0);
+        assert_eq!(size_of::<AnyProtocol>(), 0);
+        assert_eq!(size_of::<Ivar>(), 0);
+        assert_eq!(size_of::<Method>(), 0);
+    }
 }