Skip to content

Commit

Permalink
Remove INSObject::new
Browse files Browse the repository at this point in the history
Some classes (NSValue is the current example of this, but it's probably not the only case) don't want to handle cases where the users didn't supply a value, and implement `init` with throwing an exception or returning `nil`.

We could panic in those cases (though we can't currently catch GNUStep exceptions), but instead we should just simply not provide a way to construct these invalid instances.
  • Loading branch information
madsmtm committed Nov 1, 2021
1 parent 9b8ce34 commit 2fb4fb8
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 71 deletions.
3 changes: 1 addition & 2 deletions objc2_foundation/examples/basic_usage.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use objc2::rc::autoreleasepool;
use objc2_foundation::{
INSArray, INSCopying, INSDictionary, INSObject, INSString, NSArray, NSDictionary, NSObject,
NSString,
INSArray, INSCopying, INSDictionary, INSString, NSArray, NSDictionary, NSObject, NSString,
};

fn main() {
Expand Down
4 changes: 2 additions & 2 deletions objc2_foundation/examples/class_with_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ unsafe impl RefEncode for MyObject<'_> {
unsafe impl Message for MyObject<'_> {}

impl<'a> MyObject<'a> {
fn new_with_ptr(number_ptr: &'a mut u8) -> Id<Self, Owned> {
fn new(number_ptr: &'a mut u8) -> Id<Self, Owned> {
unsafe {
let obj: *mut Self = msg_send![Self::class(), alloc];
let obj: *mut Self = msg_send![obj, initWithPtr: number_ptr];
Expand Down Expand Up @@ -83,7 +83,7 @@ unsafe impl INSObject for MyObject<'_> {

fn main() {
let mut number = 54;
let mut obj = MyObject::new_with_ptr(&mut number);
let mut obj = MyObject::new(&mut number);

println!("Number: {}", obj.get().unwrap());

Expand Down
8 changes: 7 additions & 1 deletion objc2_foundation/examples/custom_class.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::ptr::NonNull;
use std::sync::Once;

use objc2::declare::ClassDecl;
use objc2::rc::Owned;
use objc2::rc::{Id, Owned};
use objc2::runtime::{Class, Object, Sel};
use objc2::{msg_send, sel};
use objc2::{Encoding, Message, RefEncode};
Expand All @@ -22,6 +23,11 @@ unsafe impl RefEncode for MYObject {
}

impl MYObject {
fn new() -> Id<Self, <Self as INSObject>::Ownership> {
let cls = Self::class();
unsafe { Id::new(NonNull::new_unchecked(msg_send![cls, new])) }
}

fn number(&self) -> u32 {
unsafe {
let obj = &*(self as *const _ as *const Object);
Expand Down
4 changes: 3 additions & 1 deletion objc2_foundation/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ pub unsafe trait INSArray: INSObject {
type Item: INSObject;
type ItemOwnership: Ownership;

unsafe_def_fn!(fn new);

#[doc(alias = "count")]
fn len(&self) -> usize {
unsafe { msg_send![self, count] }
Expand Down Expand Up @@ -387,7 +389,7 @@ mod tests {
assert_eq!(array.first(), array.get(0));
assert_eq!(array.last(), array.get(3));

let empty_array: Id<NSArray<NSObject, Owned>, _> = INSObject::new();
let empty_array = <NSArray<NSObject, Owned>>::new();
assert!(empty_array.first().is_none());
assert!(empty_array.last().is_none());
}
Expand Down
3 changes: 2 additions & 1 deletion objc2_foundation/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use objc2::msg_send;
use objc2::rc::{Id, Owned, Shared};

pub unsafe trait INSData: INSObject {
unsafe_def_fn!(fn new);

fn len(&self) -> usize {
unsafe { msg_send![self, length] }
}
Expand Down Expand Up @@ -179,7 +181,6 @@ impl DerefMut for NSMutableData {
#[cfg(test)]
mod tests {
use super::{INSData, INSMutableData, NSData, NSMutableData};
use crate::INSObject;
#[cfg(feature = "block")]
use alloc::vec;

Expand Down
4 changes: 3 additions & 1 deletion objc2_foundation/src/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub unsafe trait INSDictionary: INSObject {
type Value: INSObject;
type ValueOwnership: Ownership;

unsafe_def_fn!(fn new);

#[doc(alias = "count")]
fn len(&self) -> usize {
unsafe { msg_send![self, count] }
Expand Down Expand Up @@ -169,7 +171,7 @@ mod tests {
use objc2::rc::{autoreleasepool, Id, Shared};

use super::{INSDictionary, NSDictionary};
use crate::{INSArray, INSObject, INSString, NSObject, NSString};
use crate::{INSArray, INSString, NSObject, NSString};

fn sample_dict(key: &str) -> Id<NSDictionary<NSString, NSObject>, Shared> {
let string = NSString::from_str(key);
Expand Down
12 changes: 4 additions & 8 deletions objc2_foundation/src/enumerator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,29 +173,25 @@ mod tests {

#[test]
fn test_enumerator() {
let vec = (0u32..4).map(NSValue::from_value).collect();
let vec = (0u32..4).map(NSValue::new).collect();
let array = NSArray::from_vec(vec);

let enumerator = array.object_enumerator();
assert!(enumerator.count() == 4);

let enumerator = array.object_enumerator();
assert!(enumerator
.enumerate()
.all(|(i, obj)| obj.get() == Some(i as u32)));
assert!(enumerator.enumerate().all(|(i, obj)| obj.get() == i as u32));
}

#[test]
fn test_fast_enumerator() {
let vec = (0u32..4).map(NSValue::from_value).collect();
let vec = (0u32..4).map(NSValue::new).collect();
let array = NSArray::from_vec(vec);

let enumerator = array.enumerator();
assert!(enumerator.count() == 4);

let enumerator = array.enumerator();
assert!(enumerator
.enumerate()
.all(|(i, obj)| obj.get() == Some(i as u32)));
assert!(enumerator.enumerate().all(|(i, obj)| obj.get() == i as u32));
}
}
9 changes: 9 additions & 0 deletions objc2_foundation/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,12 @@ macro_rules! object_impl {
}
);
}

macro_rules! unsafe_def_fn {
($v:vis fn new) => {
$v fn new() -> Id<Self, <Self as INSObject>::Ownership> {
let cls = <Self as INSObject>::class();
unsafe { Id::new(NonNull::new_unchecked(msg_send![cls, new])) }
}
};
}
9 changes: 4 additions & 5 deletions objc2_foundation/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,14 @@ pub unsafe trait INSObject: Sized + Message {
let result: Bool = unsafe { msg_send![self, isKindOfClass: cls] };
result.is_true()
}

fn new() -> Id<Self, Self::Ownership> {
let cls = Self::class();
unsafe { Id::new(msg_send![cls, new]) }
}
}

object_struct!(unsafe NSObject, Owned);

impl NSObject {
unsafe_def_fn!(pub fn new);
}

#[cfg(test)]
mod tests {
use super::{INSObject, NSObject};
Expand Down
2 changes: 2 additions & 0 deletions objc2_foundation/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const UTF8_ENCODING: usize = 4;
const UTF8_ENCODING: i32 = 4;

pub unsafe trait INSString: INSObject {
unsafe_def_fn!(fn new);

fn len(&self) -> usize {
unsafe { msg_send![self, lengthOfBytesUsingEncoding: UTF8_ENCODING] }
}
Expand Down
65 changes: 15 additions & 50 deletions objc2_foundation/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@ use super::{INSCopying, INSObject};
pub unsafe trait INSValue: INSObject {
type Value: 'static + Copy + Encode;

// Default / empty new is not provided because `-init` returns `nil` on
// Apple and GNUStep throws an exception on all other messages to this
// invalid instance.

/// TODO.
///
/// If the value was created using [`INSObject::new`], the value may not
/// be initialized. In that case, this will return [`None`].
fn get(&self) -> Option<Self::Value> {
// If encoding is `None`, this was created using INSObject::new
fn get(&self) -> Self::Value {
if let Some(encoding) = self.encoding() {
// TODO: This can be a debug assertion (?)
assert!(&Self::Value::ENCODING == encoding);
Some(unsafe { self.get_unchecked() })
assert!(&Self::Value::ENCODING == encoding, "Wrong encoding");
unsafe { self.get_unchecked() }
} else {
None
panic!("Missing NSValue encoding");
}
}

Expand All @@ -44,7 +44,7 @@ pub unsafe trait INSValue: INSObject {
result.map(|s| unsafe { CStr::from_ptr(s.as_ptr()) }.to_str().unwrap())
}

fn from_value(value: Self::Value) -> Id<Self, Self::Ownership> {
fn new(value: Self::Value) -> Id<Self, Self::Ownership> {
let cls = Self::class();
let bytes = &value as *const Self::Value as *const c_void;
let encoding = CString::new(Self::Value::ENCODING.to_string()).unwrap();
Expand All @@ -70,31 +70,7 @@ unsafe impl<T: 'static> INSObject for NSValue<T> {
type Ownership = Shared;

fn class() -> &'static Class {
#[cfg(not(gnustep))]
return class!(NSValue);

#[cfg(gnustep)]
// We can't use NSValue directly, because its `new` method throws an
// exception (instead of just becoming an invalid NSValue). Luckily,
// the `GSValue` subclass has the desired behaviour, so we can just
// use that. Unfortunately, this is less efficient for the following:
// ```
// match T::ENCODING {
// Encoding::Object => ...,
// Encoding::Struct("_NSPoint", _) => ...,
// Encoding::Pointer(&Encoding::Void) => ...,
// Encoding::Struct("_NSRange", _) => ...,
// Encoding::Struct("_NSRect", _) => ...,
// Encoding::Struct("_NSSize", _) => ...,
// }
// ```
//
// See GNUStep's `NSValue +valueClassWithObjCType` and
// `GSConcreteValueTemplate.m` for more, though we can't use the
// classes in there either, because their `new` methods return valid
// objects (and so `<NSValue<NSRange>>::new()` would work differently
// on GNUStep).
return class!(GSValue);
class!(NSValue)
}
}

Expand All @@ -108,31 +84,20 @@ unsafe impl<T: 'static> INSCopying for NSValue<T> {

#[cfg(test)]
mod tests {
use crate::{INSObject, INSValue, NSRange, NSValue};
use crate::{INSValue, NSRange, NSValue};
use objc2::Encode;

#[test]
fn test_value() {
let val = NSValue::from_value(13u32);
assert_eq!(val.get().unwrap(), 13);
let val = NSValue::new(13u32);
assert_eq!(val.get(), 13);
assert!(&u32::ENCODING == val.encoding().unwrap());
}

#[test]
fn test_value_new() {
let val = <NSValue<u8>>::new();
assert!(val.encoding().is_none());
assert!(val.get().is_none());
}

#[test]
fn test_value_nsrange() {
let val = NSValue::from_value(NSRange::from(1..2));
let val = NSValue::new(NSRange::from(1..2));
assert!(&NSRange::ENCODING == val.encoding().unwrap());
assert_eq!(val.get().unwrap(), NSRange::from(1..2));

let val = <NSValue<NSRange>>::new();
assert!(val.encoding().is_none());
assert!(val.get().is_none());
assert_eq!(val.get(), NSRange::from(1..2));
}
}

0 comments on commit 2fb4fb8

Please sign in to comment.