From fdcecc4490a55ba96aef6a2a87288c416de00708 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 1 Nov 2021 22:04:38 +0100 Subject: [PATCH] Remove INSObject::new 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. --- objc2_foundation/examples/basic_usage.rs | 2 +- .../examples/class_with_lifetime.rs | 4 +- objc2_foundation/examples/custom_class.rs | 8 ++- objc2_foundation/src/array.rs | 4 +- objc2_foundation/src/data.rs | 3 +- objc2_foundation/src/dictionary.rs | 4 +- objc2_foundation/src/enumerator.rs | 12 ++-- objc2_foundation/src/macros.rs | 9 +++ objc2_foundation/src/object.rs | 9 ++- objc2_foundation/src/string.rs | 2 + objc2_foundation/src/value.rs | 65 +++++-------------- 11 files changed, 52 insertions(+), 70 deletions(-) diff --git a/objc2_foundation/examples/basic_usage.rs b/objc2_foundation/examples/basic_usage.rs index 18eb7bda6..720ab96a5 100644 --- a/objc2_foundation/examples/basic_usage.rs +++ b/objc2_foundation/examples/basic_usage.rs @@ -1,6 +1,6 @@ use objc2::rc::autoreleasepool; use objc2_foundation::{ - INSArray, INSCopying, INSDictionary, INSObject, INSString, NSArray, NSDictionary, NSObject, + INSArray, INSCopying, INSDictionary, INSString, NSArray, NSDictionary, NSObject, NSString, }; diff --git a/objc2_foundation/examples/class_with_lifetime.rs b/objc2_foundation/examples/class_with_lifetime.rs index 75e59ab3f..63230f156 100644 --- a/objc2_foundation/examples/class_with_lifetime.rs +++ b/objc2_foundation/examples/class_with_lifetime.rs @@ -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 { + fn new(number_ptr: &'a mut u8) -> Id { unsafe { let obj: *mut Self = msg_send![Self::class(), alloc]; let obj: *mut Self = msg_send![obj, initWithPtr: number_ptr]; @@ -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()); diff --git a/objc2_foundation/examples/custom_class.rs b/objc2_foundation/examples/custom_class.rs index 8af0f59bb..e2b5b4666 100644 --- a/objc2_foundation/examples/custom_class.rs +++ b/objc2_foundation/examples/custom_class.rs @@ -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}; @@ -22,6 +23,11 @@ unsafe impl RefEncode for MYObject { } impl MYObject { + fn new() -> Id::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); diff --git a/objc2_foundation/src/array.rs b/objc2_foundation/src/array.rs index b9b810ecd..1ead4d067 100644 --- a/objc2_foundation/src/array.rs +++ b/objc2_foundation/src/array.rs @@ -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] } @@ -387,7 +389,7 @@ mod tests { assert_eq!(array.first(), array.get(0)); assert_eq!(array.last(), array.get(3)); - let empty_array: Id, _> = INSObject::new(); + let empty_array = >::new(); assert!(empty_array.first().is_none()); assert!(empty_array.last().is_none()); } diff --git a/objc2_foundation/src/data.rs b/objc2_foundation/src/data.rs index 418163a7c..d2759d79c 100644 --- a/objc2_foundation/src/data.rs +++ b/objc2_foundation/src/data.rs @@ -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] } } @@ -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; diff --git a/objc2_foundation/src/dictionary.rs b/objc2_foundation/src/dictionary.rs index 499ed5379..91d178dd7 100644 --- a/objc2_foundation/src/dictionary.rs +++ b/objc2_foundation/src/dictionary.rs @@ -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] } @@ -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, Shared> { let string = NSString::from_str(key); diff --git a/objc2_foundation/src/enumerator.rs b/objc2_foundation/src/enumerator.rs index c9ad31d6d..99c27c5f8 100644 --- a/objc2_foundation/src/enumerator.rs +++ b/objc2_foundation/src/enumerator.rs @@ -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)); } } diff --git a/objc2_foundation/src/macros.rs b/objc2_foundation/src/macros.rs index 457f25de1..d7fbf0f3e 100644 --- a/objc2_foundation/src/macros.rs +++ b/objc2_foundation/src/macros.rs @@ -75,3 +75,12 @@ macro_rules! object_impl { } ); } + +macro_rules! unsafe_def_fn { + ($v:vis fn new) => { + $v fn new() -> Id::Ownership> { + let cls = ::class(); + unsafe { Id::new(NonNull::new_unchecked(msg_send![cls, new])) } + } + }; +} diff --git a/objc2_foundation/src/object.rs b/objc2_foundation/src/object.rs index 3188a8821..d13fda438 100644 --- a/objc2_foundation/src/object.rs +++ b/objc2_foundation/src/object.rs @@ -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 { - 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}; diff --git a/objc2_foundation/src/string.rs b/objc2_foundation/src/string.rs index 628ec596a..b7502bb22 100644 --- a/objc2_foundation/src/string.rs +++ b/objc2_foundation/src/string.rs @@ -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] } } diff --git a/objc2_foundation/src/value.rs b/objc2_foundation/src/value.rs index 5f448a02e..20ba25442 100644 --- a/objc2_foundation/src/value.rs +++ b/objc2_foundation/src/value.rs @@ -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 { - // 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"); } } @@ -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 { + fn new(value: Self::Value) -> Id { 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(); @@ -70,31 +70,7 @@ unsafe impl INSObject for NSValue { 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 `>::new()` would work differently - // on GNUStep). - return class!(GSValue); + class!(NSValue) } } @@ -108,31 +84,20 @@ unsafe impl INSCopying for NSValue { #[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 = >::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 = >::new(); - assert!(val.encoding().is_none()); - assert!(val.get().is_none()); + assert_eq!(val.get(), NSRange::from(1..2)); } }