Skip to content

Commit

Permalink
Avoid NSString feature requirement on Display and Debug impls
Browse files Browse the repository at this point in the history
We would like objects to be more self-contained, and less reliant on
other features being enabled.
  • Loading branch information
madsmtm committed Sep 23, 2024
1 parent 27ab005 commit 1e0b9b4
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 55 deletions.
30 changes: 17 additions & 13 deletions framework-crates/objc2-foundation/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#[cfg(feature = "NSString")]
use core::fmt;
use core::panic::{RefUnwindSafe, UnwindSafe};
use objc2::msg_send_id;
use objc2::rc::Retained;
use objc2::runtime::NSObject;

use crate::NSError;
use crate::{util, NSError};

impl UnwindSafe for NSError {}
impl RefUnwindSafe for NSError {}
Expand Down Expand Up @@ -32,25 +34,27 @@ impl NSError {
}
}

#[cfg(feature = "NSString")]
#[cfg(feature = "NSDictionary")]
impl std::error::Error for NSError {}

#[cfg(feature = "NSString")]
#[cfg(feature = "NSDictionary")]
impl fmt::Debug for NSError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("NSError")
.field("domain", &self.domain())
.field("code", &self.code())
.field("user_info", &self.userInfo())
.finish()
let mut debug = f.debug_struct("NSError");
debug.field("code", &self.code());

#[cfg(feature = "NSString")]
debug.field("domain", &self.domain());

#[cfg(all(feature = "NSDictionary", feature = "NSString"))]
debug.field("userInfo", &self.userInfo());

debug.finish_non_exhaustive()
}
}

#[cfg(feature = "NSString")]
impl fmt::Display for NSError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.localizedDescription())
let desc: Retained<NSObject> = unsafe { msg_send_id![self, localizedDescription] };
// SAFETY: `localizedDescription` returns `NSString`.
unsafe { util::display_string(&desc, f) }
}
}
28 changes: 18 additions & 10 deletions framework-crates/objc2-foundation/src/exception.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
#[cfg(all(feature = "NSObjCRuntime", feature = "NSString"))]
use core::fmt;
use core::hint::unreachable_unchecked;
use core::panic::{RefUnwindSafe, UnwindSafe};

use objc2::exception::Exception;
use objc2::rc::Retained;
use objc2::runtime::{NSObject, NSObjectProtocol};
use objc2::{extern_methods, sel, ClassType};
use objc2::runtime::{AnyObject, NSObject, NSObjectProtocol};
use objc2::{extern_methods, msg_send_id, sel, ClassType};

use crate::NSException;
use crate::{util, NSException};

// SAFETY: Exception objects are immutable data containers, and documented as
// thread safe.
Expand Down Expand Up @@ -93,15 +92,24 @@ impl NSException {
}
}

#[cfg(all(feature = "NSObjCRuntime", feature = "NSString"))]
impl fmt::Debug for NSException {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let obj: &objc2::runtime::AnyObject = self.as_ref();
write!(f, "{obj:?} '{}'", self.name())?;
if let Some(reason) = self.reason() {
write!(f, " reason: {reason}")?;
let obj: &AnyObject = self.as_ref();
write!(f, "{obj:?}")?;

write!(f, " '")?;
let name: Retained<NSObject> = unsafe { msg_send_id![self, name] };
// SAFETY: `name` returns `NSExceptionName`, which is `NSString`.
unsafe { util::display_string(&name, f)? };
write!(f, "'")?;

write!(f, " reason: ")?;
let reason: Option<Retained<NSObject>> = unsafe { msg_send_id![self, reason] };
if let Some(reason) = reason {
// SAFETY: `reason` returns `NSString`.
unsafe { util::display_string(&reason, f)? };
} else {
write!(f, " reason: (NULL)")?;
write!(f, "(NULL)")?;
}
Ok(())
}
Expand Down
8 changes: 6 additions & 2 deletions framework-crates/objc2-foundation/src/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ use core::hash;
use core::panic::{RefUnwindSafe, UnwindSafe};

use objc2::encode::Encoding;
use objc2::msg_send_id;
use objc2::rc::Retained;
use objc2::runtime::NSObject;

use crate::util;
use crate::NSNumber;

impl UnwindSafe for NSNumber {}
Expand Down Expand Up @@ -257,10 +260,11 @@ impl Ord for NSNumber {
}
}

#[cfg(feature = "NSString")]
impl fmt::Display for NSNumber {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(&self.stringValue(), f)
let string: Retained<NSObject> = unsafe { msg_send_id![self, stringValue] };
// SAFETY: `stringValue` returns `NSString`.
unsafe { util::display_string(&string, f) }
}
}

Expand Down
11 changes: 6 additions & 5 deletions framework-crates/objc2-foundation/src/process_info.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#[cfg(feature = "NSString")]
use core::fmt;
use core::panic::{RefUnwindSafe, UnwindSafe};

Expand All @@ -7,11 +6,13 @@ use crate::NSProcessInfo;
impl UnwindSafe for NSProcessInfo {}
impl RefUnwindSafe for NSProcessInfo {}

#[cfg(feature = "NSString")]
impl fmt::Debug for NSProcessInfo {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("NSProcessInfo")
.field("processName", &self.processName())
.finish_non_exhaustive()
let mut debug = f.debug_struct("NSProcessInfo");

#[cfg(feature = "NSString")]
debug.field("processName", &self.processName());

debug.finish_non_exhaustive()
}
}
21 changes: 7 additions & 14 deletions framework-crates/objc2-foundation/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use objc2::rc::{autoreleasepool_leaking, Allocated, AutoreleasePool, Retained};
use objc2::runtime::__nsstring::{nsstring_len, nsstring_to_str, UTF8_ENCODING};
use objc2::{AllocAnyThread, Message};

use crate::util;
use crate::{NSMutableString, NSString};

// Even if an exception occurs inside a string method, the state of the string
Expand Down Expand Up @@ -282,25 +283,17 @@ impl AddAssign<&NSString> for &NSMutableString {

impl fmt::Display for NSString {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// SAFETY:
// - The object is an instance of `NSString`.
// - We control the scope in which the string is alive, so we know
// it is not moved outside the current autorelease pool.
//
// TODO: Use more performant APIs, maybe by copying bytes into a
// temporary stack buffer so that we avoid allocating?
//
// Beware though that the string may be mutable internally, and that
// mutation may happen on every call to the formatter `f` (so
// `CFStringGetCharactersPtr` is probably out of the question, unless
// we somehow check that the string is immutable?).
autoreleasepool_leaking(|pool| fmt::Display::fmt(unsafe { nsstring_to_str(self, pool) }, f))
// SAFETY: The object is an instance of `NSString`.
unsafe { util::display_string(self, f) }
}
}

impl fmt::Debug for NSString {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// SAFETY: Same as for `Display` above.
// SAFETY: Same as for `display_string`:
// - The object is an instance of `NSString`.
// - We control the scope in which the string is alive, so we know
// it is not moved outside the current autorelease pool.
autoreleasepool_leaking(|pool| fmt::Debug::fmt(unsafe { nsstring_to_str(self, pool) }, f))
}
}
Expand Down
7 changes: 6 additions & 1 deletion framework-crates/objc2-foundation/src/tests/process_info.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#![cfg(feature = "NSString")]
#![cfg(feature = "NSProcessInfo")]
use alloc::format;

Expand All @@ -7,10 +6,16 @@ use crate::NSProcessInfo;
#[test]
fn debug() {
let info = NSProcessInfo::processInfo();

#[cfg(feature = "NSString")]
let expected = format!(
"NSProcessInfo {{ processName: {:?}, .. }}",
info.processName()
);

#[cfg(not(feature = "NSString"))]
let expected = "NSProcessInfo { .. }";

assert_eq!(format!("{info:?}"), expected);
}

Expand Down
31 changes: 29 additions & 2 deletions framework-crates/objc2-foundation/src/util.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
#![allow(dead_code)]
use core::ptr::NonNull;
use core::{fmt, ptr::NonNull};

use objc2::rc::Retained;
use objc2::runtime::__nsstring::nsstring_to_str;
use objc2::{
rc::{autoreleasepool_leaking, Retained},
runtime::NSObject,
};

pub(crate) fn retained_ptr_cast<T: ?Sized>(objects: *mut Retained<T>) -> *mut NonNull<T> {
// SAFETY: `Retained<T>` has the same memory layout as `NonNull<T>`, and
Expand All @@ -18,3 +22,26 @@ pub(crate) fn ref_ptr_cast_const<T: ?Sized>(objects: *const &T) -> *mut NonNull<
pub(crate) fn retained_ptr_cast_const<T: ?Sized>(objects: *const Retained<T>) -> *mut NonNull<T> {
retained_ptr_cast(objects as *mut Retained<T>)
}

/// Display the string.
///
/// Put here to allow using it without the `"NSString"` feature being active.
///
/// # Safety
///
/// The string must be an instance of `NSString`.
pub(crate) unsafe fn display_string(string: &NSObject, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// SAFETY:
// - The caller upholds that the object is a `NSString`.
// - We control the scope in which the string is alive, so we know
// it is not moved outside the current autorelease pool.
//
// TODO: Use more performant APIs, maybe by copying bytes into a
// temporary stack buffer so that we avoid allocating?
//
// Beware though that the string may be mutable internally, and that
// mutation may happen on every call to the formatter `f` (so
// `CFStringGetCharactersPtr` is probably out of the question, unless we
// somehow check that the string is immutable?).
autoreleasepool_leaking(|pool| fmt::Display::fmt(unsafe { nsstring_to_str(string, pool) }, f))
}
19 changes: 11 additions & 8 deletions framework-crates/objc2-foundation/src/uuid.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#[cfg(feature = "NSString")]
use core::fmt;
use core::panic::{RefUnwindSafe, UnwindSafe};

use objc2::encode::{Encode, Encoding, RefEncode};
use objc2::rc::{Allocated, Retained};
use objc2::{extern_methods, AllocAnyThread};
use objc2::runtime::NSObject;
use objc2::{extern_methods, msg_send_id, AllocAnyThread};

use crate::NSUUID;
use crate::{util, NSUUID};

impl UnwindSafe for NSUUID {}
impl RefUnwindSafe for NSUUID {}
Expand Down Expand Up @@ -77,18 +77,21 @@ impl NSUUID {
}
}

#[cfg(feature = "NSString")]
impl fmt::Display for NSUUID {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(&self.UUIDString(), f)
let string: Retained<NSObject> = unsafe { msg_send_id![self, UUIDString] };
// SAFETY: `UUIDString` returns `NSString`.
unsafe { util::display_string(&string, f) }
}
}

#[cfg(feature = "NSString")]
impl fmt::Debug for NSUUID {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// The `uuid` crate does `Debug` and `Display` equally, and so do we
fmt::Display::fmt(&self.UUIDString(), f)
// The `uuid` crate does `Debug` and `Display` equally, and so do we.

let string: Retained<NSObject> = unsafe { msg_send_id![self, UUIDString] };
// SAFETY: `UUIDString` returns `NSString`.
unsafe { util::display_string(&string, f) }
}
}

Expand Down

0 comments on commit 1e0b9b4

Please sign in to comment.