Merge pull request #110 from madsmtm/take-inspiration

Take inspiration from `objrs`

Author: madsmtm

objc2-encode/src/encode.rs

@@ -147,6 +147,8 @@ pub unsafe trait RefEncode {
     const ENCODING_REF: Encoding<'static>;
 }
 
+// TODO: Implement for `PhantomData` and `PhantomPinned`?
+
 /// Simple helper for implementing [`Encode`].
 macro_rules! encode_impls {
     ($($t:ty => $e:ident,)*) => ($(
@@ -169,6 +171,8 @@ encode_impls!(
     f64 => Double,
 );
 
+// TODO: Structs in core::arch?
+
 /// To allow usage as the return type of generic functions.
 ///
 /// You should not rely on this encoding to exist for any other purpose (since
@@ -304,6 +308,20 @@ unsafe impl<T: RefEncode + ?Sized> RefEncode for ManuallyDrop<T> {
     const ENCODING_REF: Encoding<'static> = T::ENCODING_REF;
 }
 
+// TODO: Consider UnsafeCell<T>
+// It is #[repr(transparent)], but might not be safe given that UnsafeCell
+// also has #[repr(no_niche)]
+
+// TODO: Consider Cell<T>
+// It is #[repr(transparent)] of UnsafeCell<T>, so figure that out first
+// `Cell` might also have other requirements that would disourage this impl?
+
+// TODO: Types that need to be made repr(transparent) first:
+// core::cell::Ref?
+// core::cell::RefCell?
+// core::cell::RefMut?
+// core::panic::AssertUnwindSafe<T>
+
 // SAFETY: `Pin` is `repr(transparent)`.
 unsafe impl<T: Encode> Encode for Pin<T> {
     const ENCODING: Encoding<'static> = T::ENCODING;
@@ -314,6 +332,8 @@ unsafe impl<T: RefEncode> RefEncode for Pin<T> {
     const ENCODING_REF: Encoding<'static> = T::ENCODING_REF;
 }
 
+// TODO: MaybeUninit<T>
+
 // SAFETY: `Wrapping` is `repr(transparent)`.
 unsafe impl<T: Encode> Encode for Wrapping<T> {
     const ENCODING: Encoding<'static> = T::ENCODING;
@@ -324,6 +344,10 @@ unsafe impl<T: RefEncode> RefEncode for Wrapping<T> {
     const ENCODING_REF: Encoding<'static> = T::ENCODING_REF;
 }
 
+// TODO: core::num::Saturating when that is stabilized
+
+// TODO: core::cmp::Reverse?
+
 /// Helper for implementing `Encode`/`RefEncode` for pointers to types that
 /// implement `RefEncode`.
 ///

objc2-encode/src/encoding.rs

@@ -42,6 +42,9 @@ pub enum Encoding<'a> {
     /// A C `int`. Corresponds to the `i` code.
     Int,
     /// A C `long`. Corresponds to the `l` code.
+    ///
+    /// This is treated as a 32-bit quantity in 64-bit programs.
+    // TODO: What does that mean??
     Long,
     /// A C `long long`. Corresponds to the `q` code.
     LongLong,
@@ -121,6 +124,13 @@ pub enum Encoding<'a> {
     // } x_t;
     // NSLog(@"Encoding: %s", @encode(_Atomic x_t)); // -> A{x}
     // NSLog(@"Encoding: %s", @encode(const int*)); // -> r^i
+    //
+    // Note that const only applies to the outermost pointer!
+    //
+    // And how does atomic objects work?
+    // core::sync::atomic::AtomicPtr<T: Message>?
+
+    // TODO: `t` and `T` codes for i128 and u128?
 }
 
 impl Encoding<'_> {

objc2/CHANGELOG.md

@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Added
 * Added deprecated `Object::get_ivar` and `Object::get_mut_ivar` to make
   upgrading easier.
+* Allow using `From`/`TryFrom` to convert between `rc::Id` and `rc::WeakId`.
 
 
 ## 0.3.0-alpha.6 - 2022-01-03

objc2/src/exception.rs

@@ -14,6 +14,9 @@
 //! - <https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Exceptions/Exceptions.html>
 //! - <https://llvm.org/docs/ExceptionHandling.html>
 
+// TODO: Test this with panic=abort, and ensure that the code-size is
+// reasonable in that case.
+
 use core::ffi::c_void;
 use core::mem;
 use core::ptr;
@@ -25,6 +28,21 @@ use crate::rc::{Id, Shared};
 use crate::runtime::Object;
 
 extern "C" {
+    /// Call the given function inside an Objective-C `@try/@catch` block.
+    ///
+    /// Defined in `extern/exception.m` and compiled in `build.rs`.
+    ///
+    /// Alternatively, we could manually write assembly for this function like
+    /// [`objrs` does][manual-asm] does, that would cut down on a build stage
+    /// (and would probably give us a bit better performance), but it gets
+    /// unwieldy _very_ quickly, so I chose the much more stable option.
+    ///
+    /// Another thing to remember: While Rust's and Objective-C's unwinding
+    /// mechanisms are similar now, Rust's is explicitly unspecified, and they
+    /// may diverge significantly in the future; so handling this in pure Rust
+    /// (using mechanisms like core::intrinsics::r#try) is not an option!
+    ///
+    /// [manual-asm]: https://gitlab.com/objrs/objrs/-/blob/b4f6598696b3fa622e6fddce7aff281770b0a8c2/src/exception.rs
     fn rust_objc_try_catch_exception(
         f: extern "C" fn(*mut c_void),
         context: *mut c_void,

objc2/src/rc/autorelease.rs

@@ -18,6 +18,8 @@ use crate::ffi;
 /// pool on the current thread.
 #[derive(Debug)]
 pub struct AutoreleasePool {
+    /// This is an opaque handle, and is not guaranteed to be neither a valid
+    /// nor aligned pointer.
     context: *mut c_void,
     // May pointer to data that is mutated (even though we hold shared access)
     p: PhantomData<*mut UnsafeCell<c_void>>,
@@ -140,9 +142,13 @@ impl Drop for AutoreleasePool {
     /// > Not draining the pool during an unwind is apparently required by the
     /// > Objective-C exceptions implementation.
     ///
-    /// This was true in the past, but since [revision `371`] of objc4 (ships
-    /// with MacOS 10.5) the exception is now retained when `@throw` is
-    /// encountered.
+    /// However, we would like to do this anyway whenever possible, since the
+    /// unwind is probably caused by Rust, and forgetting to pop the pool will
+    /// likely leak memory.
+    ///
+    /// Fortunately, the above statement was true in the past, but since
+    /// [revision `371`] of objc4 (ships with MacOS 10.5) the exception is now
+    /// retained when `@throw` is encountered.
     ///
     /// Hence it is safe to drain the pool when unwinding.
     ///

objc2/src/rc/weak_id.rs

@@ -1,5 +1,6 @@
 use alloc::boxed::Box;
 use core::cell::UnsafeCell;
+use core::convert::TryFrom;
 use core::fmt;
 use core::marker::PhantomData;
 use core::ptr;
@@ -24,6 +25,7 @@ pub struct WeakId<T: ?Sized> {
     /// an UnsafeCell to get a *mut without self being mutable.
     ///
     /// TODO: Verify the need for UnsafeCell?
+    /// TODO: Investigate if we can avoid some allocations using `Pin`.
     inner: Box<UnsafeCell<*mut T>>,
     /// WeakId inherits variance, dropck and various marker traits from
     /// `Id<T, Shared>` because it can be upgraded to a shared Id.
@@ -59,6 +61,7 @@ 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")]
     #[inline]
@@ -67,6 +70,8 @@ impl<T: Message> WeakId<T> {
         let obj = unsafe { ffi::objc_loadWeakRetained(ptr) } as *mut T;
         NonNull::new(obj).map(|obj| unsafe { Id::new(obj) })
     }
+
+    // TODO: Add `autorelease(&self) -> Option<&T>` using `objc_loadWeak`?
 }
 
 impl<T: ?Sized> Drop for WeakId<T> {
@@ -124,6 +129,19 @@ impl<T: RefUnwindSafe + ?Sized> RefUnwindSafe for WeakId<T> {}
 // Same as `Id<T, Shared>`.
 impl<T: RefUnwindSafe + ?Sized> UnwindSafe for WeakId<T> {}
 
+impl<T: Message> From<Id<T, Shared>> for WeakId<T> {
+    fn from(obj: Id<T, Shared>) -> Self {
+        WeakId::new(&obj)
+    }
+}
+
+impl<T: Message> TryFrom<WeakId<T>> for Id<T, Shared> {
+    type Error = ();
+    fn try_from(weak: WeakId<T>) -> Result<Self, ()> {
+        return weak.load().ok_or(());
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use core::ptr::NonNull;

objc2/src/runtime.rs

@@ -649,6 +649,29 @@ mod tests {
     use crate::test_utils;
     use crate::Encode;
 
+    #[test]
+    fn test_selector() {
+        macro_rules! test_sel {
+            ($s:literal, $($tt:tt)+) => {{
+                let sel = sel!($($tt)*);
+                let expected = Sel::register($s);
+                assert_eq!(sel, expected);
+                assert_eq!(sel.name(), $s);
+            }}
+        }
+        test_sel!("abc", abc);
+        test_sel!("abc:", abc:);
+        test_sel!("abc:def:", abc:def:);
+    }
+
+    #[test]
+    fn test_empty_selector() {
+        let sel = Sel::register("");
+        assert_eq!(sel.name(), "");
+        let sel = Sel::register(":");
+        assert_eq!(sel.name(), ":");
+    }
+
     #[test]
     fn test_ivar() {
         let cls = test_utils::custom_class();