Skip to content

Commit

Permalink
Merge pull request #276 from madsmtm/msg-send-error
Browse files Browse the repository at this point in the history 
Better error handling in `msg_send!` and `msg_send_id!`
  • Loading branch information
@madsmtm
madsmtm authored Oct 28, 2022
2 parents 14449d7 + 88d1b83 commit cf22f78
Show file tree
Hide file tree
Showing 42 changed files with 1,361 additions and 320 deletions.
8 changes: 7 additions & 1 deletion objc2/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
msg_send_id![NSObject::alloc(), init]
};
```
* Add `Class::class_method`.
* Added `Class::class_method`.
* Added the ability to specify `error: _`, `somethingReturningError: _` and
so on at the end of `msg_send!`/`msg_send_id!`, and have it automatically
return a `Result<..., Id<NSError, Shared>>`.
* Added the ability to specify an extra parameter at the end of the selector
in methods declared with `extern_methods!`, and let that be the `NSError**`
parameter.

### Changed
* Allow other types than `&Class` as the receiver in `msg_send_id!` methods
Expand Down
1 change: 1 addition & 0 deletions objc2/CHANGELOG_FOUNDATION.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
* Added `NSString::concat` and `NSString::join_path`.
* Added `CGSize`, `CGPoint` and `CGRect` (just aliases to equivalent
`NS`-types, but helps readability).
* Added `NSString::write_to_file`.

### Changed
* **BREAKING**: `NSSize::new` no longer requires it's arguments to be
Expand Down
270 changes: 172 additions & 98 deletions objc2/src/__macro_helpers.rs
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
#[cfg(feature = "verify_message")]
use alloc::vec::Vec;
use core::ptr;
#[cfg(feature = "verify_message")]
use std::collections::HashSet;

use crate::__sel_inner;
use crate::declare::ClassBuilder;
#[cfg(feature = "verify_message")]
use crate::declare::MethodImplementation;
use crate::rc::{Allocated, Id, Ownership};
use crate::encode::Encode;
use crate::message::__TupleExtender;
use crate::rc::{Allocated, Id, Ownership, Shared};
#[cfg(feature = "verify_message")]
use crate::runtime::MethodDescription;
use crate::runtime::{Class, Object, Protocol, Sel};
use crate::{Message, MessageArguments, MessageReceiver};
use crate::{__sel_data, __sel_inner};

pub use crate::cache::CachedClass;
pub use crate::cache::CachedSel;
Expand All @@ -38,19 +41,16 @@ pub use std::sync::Once;
// actual assembly is as one would expect.

#[inline]
pub fn alloc() -> Sel {
// SAFETY: Must have NUL byte
__sel_inner!("alloc\0", "alloc")
pub fn alloc_sel() -> Sel {
__sel_inner!(__sel_data!(alloc), "alloc")
}
#[inline]
pub fn init() -> Sel {
// SAFETY: Must have NUL byte
__sel_inner!("init\0", "init")
pub fn init_sel() -> Sel {
__sel_inner!(__sel_data!(init), "init")
}
#[inline]
pub fn new() -> Sel {
// SAFETY: Must have NUL byte
__sel_inner!("new\0", "new")
pub fn new_sel() -> Sel {
__sel_inner!(__sel_data!(new), "new")
}

/// Helper for specifying the retain semantics for a given selector family.
Expand All @@ -72,29 +72,90 @@ pub fn new() -> Sel {
/// means it can't be a class method!
///
/// <https://clang.llvm.org/docs/AutomaticReferenceCounting.html#retainable-object-pointers-as-operands-and-arguments>
pub struct RetainSemantics<
// `new` family
const NEW: bool,
// `alloc` family
const ALLOC: bool,
// `init` family
const INIT: bool,
// `copy` or `mutableCopy` family
const COPY_OR_MUT_COPY: bool,
> {}

type New = RetainSemantics<true, false, false, false>;
type Alloc = RetainSemantics<false, true, false, false>;
type Init = RetainSemantics<false, false, true, false>;
type CopyOrMutCopy = RetainSemantics<false, false, false, true>;
type Other = RetainSemantics<false, false, false, false>;
// TODO: Use an enum instead of u8 here when stable
pub struct RetainSemantics<const INNER: u8> {}

pub type New = RetainSemantics<1>;
pub type Alloc = RetainSemantics<2>;
pub type Init = RetainSemantics<3>;
pub type CopyOrMutCopy = RetainSemantics<4>;
pub type Other = RetainSemantics<5>;

pub const fn retain_semantics(selector: &str) -> u8 {
let selector = selector.as_bytes();
match (
in_selector_family(selector, b"new"),
in_selector_family(selector, b"alloc"),
in_selector_family(selector, b"init"),
in_selector_family(selector, b"copy"),
in_selector_family(selector, b"mutableCopy"),
) {
(true, false, false, false, false) => 1,
(false, true, false, false, false) => 2,
(false, false, true, false, false) => 3,
(false, false, false, true, false) => 4,
(false, false, false, false, true) => 4,
(false, false, false, false, false) => 5,
_ => unreachable!(),
}
}

pub trait MsgSendId<T, U> {
unsafe fn send_message_id<A: MessageArguments, R: MaybeUnwrap<Input = U>>(
obj: T,
sel: Sel,
args: A,
) -> R;

/// Add an extra error argument to the argument list, call
/// `send_message_id` with that, and return an error if one occurred.
#[inline]
#[track_caller]
unsafe fn send_message_id_error<A, E>(obj: T, sel: Sel, args: A) -> Result<U, Id<E, Shared>>
where
*mut *mut E: Encode,
A: __TupleExtender<*mut *mut E>,
<A as __TupleExtender<*mut *mut E>>::PlusOneArgument: MessageArguments,
E: Message,
Option<U>: MaybeUnwrap<Input = U>,
{
let mut err: *mut E = ptr::null_mut();
let args = args.add_argument(&mut err);
let res: Option<U> = unsafe { Self::send_message_id(obj, sel, args) };
// As per the Cocoa documentation:
// > Success or failure is indicated by the return value of the
// > method. Although Cocoa methods that indirectly return error
// > objects in the Cocoa error domain are guaranteed to return such
// > objects if the method indicates failure by directly returning
// > `nil` or `NO`, you should always check that the return value is
// > `nil` or `NO` before attempting to do anything with the `NSError`
// > object.
if let Some(res) = res {
// In this case, the error is likely not created. If it is, it is
// autoreleased anyhow, so it would be a waste to retain and
// release it.
Ok(res)
} else {
// In this case, the error has very likely been created, but has
// been autoreleased (as is common for "out parameters"). Hence we
// need to retain it if we want it to live across autorelease
// pools.
//
// SAFETY: The closure `f` is guaranteed to populate the error
// object, or leave it as NULL. The error is shared, and all
// holders of the error know this, so is safe to retain.
Err(unsafe { encountered_error(err) })
}
}
}

// Marked `cold` to tell the optimizer that errors are comparatively rare.
// And intentionally not inlined, for much the same reason.
#[cold]
#[track_caller]
unsafe fn encountered_error<E: Message>(err: *mut E) -> Id<E, Shared> {
// SAFETY: Ensured by caller
unsafe { Id::retain(err) }.expect("error parameter should be set if the method returns NULL")
}

impl<T: MessageReceiver, U: ?Sized + Message, O: Ownership> MsgSendId<T, Id<U, O>> for New {
Expand Down Expand Up @@ -266,7 +327,7 @@ impl<'a> MsgSendIdFailed<'a> for New {
if let Some(obj) = obj {
let cls = obj.class();
if cls.is_metaclass() {
if sel == new() {
if sel == new_sel() {
panic!("failed creating new instance of {:?}", cls)
} else {
panic!("failed creating new instance using +[{:?} {:?}]", cls, sel)
Expand All @@ -286,7 +347,7 @@ impl<'a> MsgSendIdFailed<'a> for Alloc {
#[cold]
#[track_caller]
fn failed((cls, sel): Self::Args) -> ! {
if sel == alloc() {
if sel == alloc_sel() {
panic!("failed allocating {:?}", cls)
} else {
panic!("failed allocating with +[{:?} {:?}]", cls, sel)
Expand All @@ -305,7 +366,7 @@ impl MsgSendIdFailed<'_> for Init {
} else {
// We can't really display a more descriptive message here since the
// object is consumed by `init` and may not be valid any more.
if sel == init() {
if sel == init_sel() {
panic!("failed initializing object")
} else {
panic!("failed initializing object with -{:?}", sel)
Expand Down Expand Up @@ -347,7 +408,7 @@ impl<'a> MsgSendIdFailed<'a> for Other {
/// Checks whether a given selector is said to be in a given selector family.
///
/// <https://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-method-families>
pub const fn in_selector_family(mut selector: &[u8], mut family: &[u8]) -> bool {
const fn in_selector_family(mut selector: &[u8], mut family: &[u8]) -> bool {
// Skip leading underscores from selector
loop {
selector = match selector {
Expand Down Expand Up @@ -547,6 +608,7 @@ impl Drop for ClassProtocolMethodsBuilder<'_, '_> {
mod tests {
use super::*;

use alloc::string::ToString;
use alloc::vec;
use core::ptr;

Expand Down Expand Up @@ -769,80 +831,92 @@ mod tests {

#[test]
fn test_in_selector_family() {
fn assert_in_family(selector: &str, family: &str) {
assert!(in_selector_family(selector.as_bytes(), family.as_bytes()));
let selector = selector.to_string() + "\0";
assert!(in_selector_family(selector.as_bytes(), family.as_bytes()));
}

fn assert_not_in_family(selector: &str, family: &str) {
assert!(!in_selector_family(selector.as_bytes(), family.as_bytes()));
let selector = selector.to_string() + "\0";
assert!(!in_selector_family(selector.as_bytes(), family.as_bytes()));
}

// Common cases

assert!(in_selector_family(b"alloc", b"alloc"));
assert!(in_selector_family(b"allocWithZone:", b"alloc"));
assert!(!in_selector_family(b"dealloc", b"alloc"));
assert!(!in_selector_family(b"initialize", b"init"));
assert!(!in_selector_family(b"decimalNumberWithDecimal:", b"init"));
assert!(in_selector_family(b"initWithCapacity:", b"init"));
assert!(in_selector_family(b"_initButPrivate:withParam:", b"init"));
assert!(!in_selector_family(b"description", b"init"));
assert!(!in_selector_family(b"inIT", b"init"));

assert!(!in_selector_family(b"init", b"copy"));
assert!(!in_selector_family(b"copyingStuff:", b"copy"));
assert!(in_selector_family(b"copyWithZone:", b"copy"));
assert!(!in_selector_family(b"initWithArray:copyItems:", b"copy"));
assert!(in_selector_family(b"copyItemAtURL:toURL:error:", b"copy"));

assert!(!in_selector_family(b"mutableCopying", b"mutableCopy"));
assert!(in_selector_family(b"mutableCopyWithZone:", b"mutableCopy"));
assert!(in_selector_family(b"mutableCopyWithZone:", b"mutableCopy"));

assert!(in_selector_family(
b"newScriptingObjectOfClass:forValueForKey:withContentsValue:properties:",
b"new"
));
assert!(in_selector_family(
b"newScriptingObjectOfClass:forValueForKey:withContentsValue:properties:",
b"new"
));
assert!(!in_selector_family(b"newsstandAssetDownload", b"new"));
assert_in_family("alloc", "alloc");
assert_in_family("allocWithZone:", "alloc");
assert_not_in_family("dealloc", "alloc");
assert_not_in_family("initialize", "init");
assert_not_in_family("decimalNumberWithDecimal:", "init");
assert_in_family("initWithCapacity:", "init");
assert_in_family("_initButPrivate:withParam:", "init");
assert_not_in_family("description", "init");
assert_not_in_family("inIT", "init");

assert_not_in_family("init", "copy");
assert_not_in_family("copyingStuff:", "copy");
assert_in_family("copyWithZone:", "copy");
assert_not_in_family("initWithArray:copyItems:", "copy");
assert_in_family("copyItemAtURL:toURL:error:", "copy");

assert_not_in_family("mutableCopying", "mutableCopy");
assert_in_family("mutableCopyWithZone:", "mutableCopy");
assert_in_family("mutableCopyWithZone:", "mutableCopy");

assert_in_family(
"newScriptingObjectOfClass:forValueForKey:withContentsValue:properties:",
"new",
);
assert_in_family(
"newScriptingObjectOfClass:forValueForKey:withContentsValue:properties:",
"new",
);
assert_not_in_family("newsstandAssetDownload", "new");

// Trying to weed out edge-cases:

assert!(in_selector_family(b"__abcDef", b"abc"));
assert!(in_selector_family(b"_abcDef", b"abc"));
assert!(in_selector_family(b"abcDef", b"abc"));
assert!(in_selector_family(b"___a", b"a"));
assert!(in_selector_family(b"__a", b"a"));
assert!(in_selector_family(b"_a", b"a"));
assert!(in_selector_family(b"a", b"a"));

assert!(!in_selector_family(b"_abcdef", b"abc"));
assert!(!in_selector_family(b"_abcdef", b"def"));
assert!(!in_selector_family(b"_bcdef", b"abc"));
assert!(!in_selector_family(b"a_bc", b"abc"));
assert!(!in_selector_family(b"abcdef", b"abc"));
assert!(!in_selector_family(b"abcdef", b"def"));
assert!(!in_selector_family(b"abcdef", b"abb"));
assert!(!in_selector_family(b"___", b"a"));
assert!(!in_selector_family(b"_", b"a"));
assert!(!in_selector_family(b"", b"a"));

assert!(in_selector_family(b"copy", b"copy"));
assert!(in_selector_family(b"copy:", b"copy"));
assert!(in_selector_family(b"copyMe", b"copy"));
assert!(in_selector_family(b"_copy", b"copy"));
assert!(in_selector_family(b"_copy:", b"copy"));
assert!(in_selector_family(b"_copyMe", b"copy"));
assert!(!in_selector_family(b"copying", b"copy"));
assert!(!in_selector_family(b"copying:", b"copy"));
assert!(!in_selector_family(b"_copying", b"copy"));
assert!(!in_selector_family(b"Copy", b"copy"));
assert!(!in_selector_family(b"COPY", b"copy"));
assert_in_family("__abcDef", "abc");
assert_in_family("_abcDef", "abc");
assert_in_family("abcDef", "abc");
assert_in_family("___a", "a");
assert_in_family("__a", "a");
assert_in_family("_a", "a");
assert_in_family("a", "a");

assert_not_in_family("_abcdef", "abc");
assert_not_in_family("_abcdef", "def");
assert_not_in_family("_bcdef", "abc");
assert_not_in_family("a_bc", "abc");
assert_not_in_family("abcdef", "abc");
assert_not_in_family("abcdef", "def");
assert_not_in_family("abcdef", "abb");
assert_not_in_family("___", "a");
assert_not_in_family("_", "a");
assert_not_in_family("", "a");

assert_in_family("copy", "copy");
assert_in_family("copy:", "copy");
assert_in_family("copyMe", "copy");
assert_in_family("_copy", "copy");
assert_in_family("_copy:", "copy");
assert_in_family("_copyMe", "copy");
assert_not_in_family("copying", "copy");
assert_not_in_family("copying:", "copy");
assert_not_in_family("_copying", "copy");
assert_not_in_family("Copy", "copy");
assert_not_in_family("COPY", "copy");

// Empty family (not supported)
assert!(in_selector_family(b"___", b""));
assert!(in_selector_family(b"__", b""));
assert!(in_selector_family(b"_", b""));
assert!(in_selector_family(b"", b""));
assert!(!in_selector_family(b"_a", b""));
assert!(!in_selector_family(b"a", b""));
assert!(in_selector_family(b"_A", b""));
assert!(in_selector_family(b"A", b""));
assert_in_family("___", "");
assert_in_family("__", "");
assert_in_family("_", "");
assert_in_family("", "");
assert_not_in_family("_a", "");
assert_not_in_family("a", "");
assert_in_family("_A", "");
assert_in_family("A", "");
}

mod test_trait_disambugated {
Expand Down
Loading

0 comments on commit cf22f78

Please sign in to comment.