Merge pull request #424 from madsmtm/redo-autorelease

Redo autorelease pools

Author: madsmtm

crates/icrate/src/Foundation/additions/string.rs

@@ -1,5 +1,4 @@
 #![cfg(feature = "Foundation_NSString")]
-use alloc::borrow::ToOwned;
 use core::cmp;
 use core::ffi::c_void;
 use core::fmt;
@@ -14,7 +13,7 @@ use core::str;
 use std::os::raw::c_char;
 
 use objc2::msg_send;
-use objc2::rc::{autoreleasepool, AutoreleasePool, DefaultId, Id, Shared};
+use objc2::rc::{autoreleasepool_leaking, AutoreleasePool, DefaultId, Id, Shared};
 use objc2::runtime::__nsstring::{nsstring_len, nsstring_to_str, UTF8_ENCODING};
 
 use crate::common::*;
@@ -110,7 +109,7 @@ impl NSString {
     ///
     /// TODO: Further explain this.
     #[doc(alias = "UTF8String")]
-    pub fn as_str<'r, 's: 'r, 'p: 'r>(&'s self, pool: &'p AutoreleasePool) -> &'r str {
+    pub fn as_str<'r, 's: 'r, 'p: 'r>(&'s self, pool: AutoreleasePool<'p>) -> &'r str {
         // SAFETY: This is an instance of `NSString`
         unsafe { nsstring_to_str(self, pool) }
     }
@@ -180,20 +179,12 @@ impl DefaultId for NSString {
 
 impl fmt::Display for NSString {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        // The call to `to_owned` is unfortunate, but is required to work
-        // around `f` not being AutoreleaseSafe.
-        // TODO: Fix this!
-        let s = autoreleasepool(|pool| self.as_str(pool).to_owned());
-        fmt::Display::fmt(&s, f)
+        autoreleasepool_leaking(|pool| fmt::Display::fmt(self.as_str(pool), f))
     }
 }
 
 impl fmt::Debug for NSString {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        // The call to `to_owned` is unfortunate, but is required to work
-        // around `f` not being AutoreleaseSafe.
-        // TODO: Fix this!
-        let s = autoreleasepool(|pool| self.as_str(pool).to_owned());
-        fmt::Debug::fmt(&s, f)
+        autoreleasepool_leaking(|pool| fmt::Debug::fmt(self.as_str(pool), f))
     }
 }

crates/objc2/CHANGELOG.md

@@ -6,9 +6,24 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased - YYYY-MM-DD
 
+### Added
+* Added `objc2::rc::autoreleasepool_leaking`, and improve performance of
+  objects `Debug` impls.
+
 ### Changed
 * Made the default ownership in `Id` be `Shared`. This means that you can now
   write `Id<NSString>`, and it'll mean `Id<NSString, Shared>`.
+* **BREAKING**: `objc2::rc::AutoreleasePool` is now a zero-sized `Copy` type
+  with a lifetime parameter, instead of the lifetime parameter being the
+  reference it was behind.
+* **BREAKING**: Made `Id::autorelease` and `Id::autorelease_return` be
+  associated functions instead of methods. This means they now have to be
+  called as `Id::autorelease(obj, pool)` instead of `obj.autorelease(pool)`.
+
+  Additionally, rename the mutable version to `Id::autorelease_mut`.
+
+### Fixed
+* Fixed using autorelease pools on 32bit macOS and older macOS versions.
 
 
 ## 0.3.0-beta.5 - 2023-02-07

crates/objc2/Cargo.toml

@@ -54,7 +54,7 @@ unstable-static-sel-inlined = ["unstable-static-sel"]
 unstable-static-class = ["objc2-proc-macros"]
 unstable-static-class-inlined = ["unstable-static-class"]
 
-# Uses nightly features to make AutoreleasePool zero-cost even in debug mode
+# Uses nightly features to make autorelease pools fully sound
 unstable-autoreleasesafe = []
 
 # Uses the nightly c_unwind feature to make throwing safe

crates/objc2/src/declare.rs

@@ -60,7 +60,7 @@
 //!                 initWithNumber: number,
 //!             ]
 //!         };
-//!         obj.map(|obj| obj.autorelease_return()).unwrap_or(std::ptr::null_mut())
+//!         obj.map(Id::autorelease_return).unwrap_or(std::ptr::null_mut())
 //!     }
 //!     unsafe {
 //!         builder.add_class_method(

crates/objc2/src/exception.rs

@@ -19,7 +19,6 @@
 // TODO: Test this with panic=abort, and ensure that the code-size is
 // reasonable in that case.
 
-use crate::alloc::borrow::ToOwned;
 #[cfg(feature = "exception")]
 use core::ffi::c_void;
 use core::fmt;
@@ -35,7 +34,7 @@ use std::error::Error;
 use crate::encode::{Encoding, RefEncode};
 #[cfg(feature = "exception")]
 use crate::ffi;
-use crate::rc::{autoreleasepool, Id};
+use crate::rc::{autoreleasepool_leaking, Id};
 use crate::runtime::__nsstring::nsstring_to_str;
 use crate::runtime::{Class, NSObject, NSObjectProtocol, Object};
 use crate::{extern_methods, sel, Message};
@@ -110,28 +109,27 @@ impl fmt::Debug for Exception {
         // Attempt to present a somewhat usable error message if the exception
         // is an instance of NSException.
         if let Some(true) = self.is_nsexception() {
-            let (name, reason) = autoreleasepool(|pool| {
+            autoreleasepool_leaking(|pool| {
                 // SAFETY: Just checked that object is an NSException
                 let (name, reason) = unsafe { (self.name(), self.reason()) };
 
                 // SAFETY: `name` and `reason` are guaranteed to be NSString.
                 let name = name
                     .as_deref()
-                    .map(|name| unsafe { nsstring_to_str(name, pool).to_owned() });
+                    .map(|name| unsafe { nsstring_to_str(name, pool) });
                 let reason = reason
                     .as_deref()
-                    .map(|reason| unsafe { nsstring_to_str(reason, pool).to_owned() });
-                (name, reason)
-            });
-
-            let obj: &Object = self.as_ref();
-            write!(f, "{obj:?} '{}'", name.unwrap_or_default())?;
-            if let Some(reason) = reason {
-                write!(f, " reason:{reason}")?;
-            } else {
-                write!(f, " reason:(NULL)")?;
-            }
-            Ok(())
+                    .map(|reason| unsafe { nsstring_to_str(reason, pool) });
+
+                let obj: &Object = self.as_ref();
+                write!(f, "{obj:?} '{}'", name.unwrap_or_default())?;
+                if let Some(reason) = reason {
+                    write!(f, " reason:{reason}")?;
+                } else {
+                    write!(f, " reason:(NULL)")?;
+                }
+                Ok(())
+            })
         } else {
             // Fall back to `Object` Debug
             write!(f, "{:?}", self.0)
@@ -141,23 +139,20 @@ impl fmt::Debug for Exception {
 
 impl fmt::Display for Exception {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        if let Some(true) = self.is_nsexception() {
-            let reason = autoreleasepool(|pool| {
+        autoreleasepool_leaking(|pool| {
+            if let Some(true) = self.is_nsexception() {
                 // SAFETY: Just checked that object is an NSException
                 let reason = unsafe { self.reason() };
 
-                // SAFETY: `reason` is guaranteed to be NSString.
-                reason
-                    .as_deref()
-                    .map(|reason| unsafe { nsstring_to_str(reason, pool).to_owned() })
-            });
-
-            if let Some(reason) = reason {
-                return write!(f, "{reason}");
+                if let Some(reason) = &reason {
+                    // SAFETY: `reason` is guaranteed to be NSString.
+                    let reason = unsafe { nsstring_to_str(reason, pool) };
+                    return write!(f, "{reason}");
+                }
             }
-        }
 
-        write!(f, "unknown exception")
+            write!(f, "unknown exception")
+        })
     }
 }
 

crates/objc2/src/protocol.rs

@@ -1,11 +1,10 @@
-use alloc::borrow::ToOwned;
 use core::fmt;
 use core::hash;
 use core::marker::PhantomData;
 use core::ptr::NonNull;
 
 use crate::encode::{Encoding, RefEncode};
-use crate::rc::{autoreleasepool, Id, Ownership};
+use crate::rc::{autoreleasepool_leaking, Id, Ownership};
 use crate::runtime::__nsstring::nsstring_to_str;
 use crate::runtime::{NSObjectProtocol, Object, Protocol};
 use crate::Message;
@@ -212,16 +211,17 @@ impl<P: ?Sized + ProtocolType + NSObjectProtocol> fmt::Debug for ProtocolObject<
         match description {
             // Attempt to format description string
             Some(description) => {
-                let s = autoreleasepool(|pool| {
+                // We use a leaking autorelease pool since often the string
+                // will be UTF-8, and in that case the pool will be
+                // irrelevant. Also, it allows us to pass the formatter into
+                // the pool (since it may contain a pool internally that it
+                // assumes is current when writing).
+                autoreleasepool_leaking(|pool| {
                     // SAFETY: `description` selector is guaranteed to always
                     // return an instance of `NSString`.
                     let s = unsafe { nsstring_to_str(&description, pool) };
-                    // The call to `to_owned` is unfortunate, but is required
-                    // to work around `f` not being `AutoreleaseSafe`.
-                    // TODO: Fix this!
-                    s.to_owned()
-                });
-                fmt::Display::fmt(&s, f)
+                    fmt::Display::fmt(s, f)
+                })
             }
             // If description was `NULL`, use `Object`'s `Debug` impl instead
             None => {