Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax Sized bounds where easily possible #98

Merged
merged 3 commits into from
Dec 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions objc2-foundation/CHANGELOG.md
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/).
### Removed
* **BREAKING**: Removed associated `Ownership` type from `INSObject`; instead,
it is present on the types that actually need it (for example `NSCopying`).
* **BREAKING**: Removed `Sized` bound on `INSObject`.

### Fixed
* Soundness issue with `NSValue`, `NSDictionary`, `NSArray` and
Expand Down
2 changes: 1 addition & 1 deletion objc2-foundation/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use super::{
NSRange,
};

unsafe fn from_refs<A: INSArray>(refs: &[&A::Item]) -> Id<A, A::Ownership> {
unsafe fn from_refs<A: INSArray + ?Sized>(refs: &[&A::Item]) -> Id<A, A::Ownership> {
let cls = A::class();
let obj: *mut A = unsafe { msg_send![cls, alloc] };
let obj: *mut A = unsafe {
Expand Down
4 changes: 2 additions & 2 deletions objc2-foundation/src/dictionary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use super::{INSCopying, INSFastEnumeration, INSObject, NSArray, NSEnumerator};

unsafe fn from_refs<D, T>(keys: &[&T], vals: &[&D::Value]) -> Id<D, Shared>
where
D: INSDictionary,
T: INSCopying<Output = D::Key>,
D: INSDictionary + ?Sized,
T: INSCopying<Output = D::Key> + ?Sized,
{
let cls = D::class();
let count = min(keys.len(), vals.len());
Expand Down
8 changes: 4 additions & 4 deletions objc2-foundation/src/enumerator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ unsafe impl<T: INSObject> RefEncode for NSFastEnumerationState<T> {
const ENCODING_REF: Encoding<'static> = Encoding::Pointer(&Self::ENCODING);
}

fn enumerate<'a, 'b: 'a, C: INSFastEnumeration>(
fn enumerate<'a, 'b: 'a, C: INSFastEnumeration + ?Sized>(
object: &'b C,
state: &mut NSFastEnumerationState<C::Item>,
buf: &'a mut [*const C::Item],
Expand All @@ -119,7 +119,7 @@ fn enumerate<'a, 'b: 'a, C: INSFastEnumeration>(

const FAST_ENUM_BUF_SIZE: usize = 16;

pub struct NSFastEnumerator<'a, C: 'a + INSFastEnumeration> {
pub struct NSFastEnumerator<'a, C: 'a + INSFastEnumeration + ?Sized> {
object: &'a C,

ptr: *const *const C::Item,
Expand All @@ -129,7 +129,7 @@ pub struct NSFastEnumerator<'a, C: 'a + INSFastEnumeration> {
buf: [*const C::Item; FAST_ENUM_BUF_SIZE],
}

impl<'a, C: INSFastEnumeration> NSFastEnumerator<'a, C> {
impl<'a, C: INSFastEnumeration + ?Sized> NSFastEnumerator<'a, C> {
fn new(object: &'a C) -> Self {
Self {
object,
Expand Down Expand Up @@ -174,7 +174,7 @@ impl<'a, C: INSFastEnumeration> NSFastEnumerator<'a, C> {
}
}

impl<'a, C: INSFastEnumeration> Iterator for NSFastEnumerator<'a, C> {
impl<'a, C: INSFastEnumeration + ?Sized> Iterator for NSFastEnumerator<'a, C> {
type Item = &'a C::Item;

fn next(&mut self) -> Option<&'a C::Item> {
Expand Down
6 changes: 1 addition & 5 deletions objc2-foundation/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@ use objc2::Message;

use super::NSString;

// The Sized bound is unfortunate; ideally, Objective-C objects would not be
// treated as Sized. However, rust won't allow casting a dynamically-sized
// type pointer to an Object pointer, because dynamically-sized types can have
// fat pointers (two words) instead of real pointers.
pub unsafe trait INSObject: Sized + Message {
pub unsafe trait INSObject: Message {
fn class() -> &'static Class;

fn hash_code(&self) -> usize {
Expand Down
3 changes: 3 additions & 0 deletions objc2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- `Class::instance_variables`
- `Protocol::protocols`
- `Protocol::adopted_protocols`
* Relaxed `Sized` bound on `rc::Id` and `rc::WeakId` to prepare for
`extern type` support.
* **BREAKING**: Relaxed `Sized` bound on `rc::SliceId` and `rc::DefaultId`.

### Removed
* **BREAKING**: Removed the raw FFI functions from the `runtime` module. These
Expand Down
4 changes: 2 additions & 2 deletions objc2/src/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use crate::{ffi, Encode, EncodeArguments, Encoding, Message};
/// Types that can be used as the implementation of an Objective-C method.
pub trait MethodImplementation {
/// The callee type of the method.
type Callee: Message;
type Callee: Message + ?Sized;
/// The return type of the method.
type Ret: Encode;
/// The argument types of the method.
Expand All @@ -59,7 +59,7 @@ macro_rules! method_decl_impl {
(-$s:ident, $r:ident, $f:ty, $($t:ident),*) => (
impl<$s, $r, $($t),*> MethodImplementation for $f
where
$s: Message,
$s: Message + ?Sized,
$r: Encode,
$($t: Encode,)*
{
Expand Down
4 changes: 2 additions & 2 deletions objc2/src/message/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub(crate) mod private {
impl<'a, T: Message + ?Sized> Sealed for &'a T {}
impl<'a, T: Message + ?Sized> Sealed for &'a mut T {}
impl<T: Message + ?Sized> Sealed for NonNull<T> {}
impl<T: Message, O: Ownership> Sealed for Id<T, O> {}
impl<T: Message + ?Sized, O: Ownership> Sealed for Id<T, O> {}

impl<T: MessageReceiver + ?Sized> Sealed for ManuallyDrop<T> {}
}
Expand Down Expand Up @@ -248,7 +248,7 @@ unsafe impl<T: Message + ?Sized> MessageReceiver for NonNull<T> {
}
}

unsafe impl<T: Message, O: Ownership> MessageReceiver for Id<T, O> {
unsafe impl<T: Message + ?Sized, O: Ownership> MessageReceiver for Id<T, O> {
#[inline]
fn as_raw_receiver(&self) -> *mut Object {
// TODO: Maybe don't dereference here, just to be safe?
Expand Down
6 changes: 3 additions & 3 deletions objc2/src/rc/autorelease.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ impl AutoreleasePool {
/// the lifetime is bound to the pool instead of being unbounded.
#[inline]
#[allow(clippy::needless_lifetimes)]
pub unsafe fn ptr_as_ref<'p, T>(&'p self, ptr: *const T) -> &'p T {
pub unsafe fn ptr_as_ref<'p, T: ?Sized>(&'p self, ptr: *const T) -> &'p T {
self.__verify_is_inner();
// SAFETY: Checked by the caller
unsafe { &*ptr }
Expand All @@ -122,7 +122,7 @@ impl AutoreleasePool {
#[inline]
#[allow(clippy::needless_lifetimes)]
#[allow(clippy::mut_from_ref)]
pub unsafe fn ptr_as_mut<'p, T>(&'p self, ptr: *mut T) -> &'p mut T {
pub unsafe fn ptr_as_mut<'p, T: ?Sized>(&'p self, ptr: *mut T) -> &'p mut T {
self.__verify_is_inner();
// SAFETY: Checked by the caller
unsafe { &mut *ptr }
Expand Down Expand Up @@ -202,7 +202,7 @@ auto_trait! {
}

#[cfg(not(feature = "unstable_autoreleasesafe"))]
unsafe impl<T> AutoreleaseSafe for T {}
unsafe impl<T: ?Sized> AutoreleaseSafe for T {}
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This improves usability even without extern type!


#[cfg(feature = "unstable_autoreleasesafe")]
impl !AutoreleaseSafe for AutoreleasePool {}
Expand Down
42 changes: 25 additions & 17 deletions objc2/src/rc/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ use crate::Message;
/// do so. If you need to run some code when the object is destroyed,
/// implement the `dealloc` method instead.
///
/// This allows `?Sized` types `T`, but the intention is to only support when
/// `T` is an `extern type` (yet unstable).
///
/// # Examples
///
/// ```no_run
Expand Down Expand Up @@ -95,9 +98,9 @@ use crate::Message;
/// ```
#[repr(transparent)]
// TODO: Figure out if `Message` bound on `T` would be better here?
// TODO: Add `?Sized + ptr::Thin` bound on `T` to allow for extern types
// TODO: Add `ptr::Thin` bound on `T` to allow for only extern types
// TODO: Consider changing the name of Id -> Retain
pub struct Id<T, O: Ownership> {
pub struct Id<T: ?Sized, O: Ownership> {
/// A pointer to the contained object. The pointer is always retained.
///
/// It is important that this is `NonNull`, since we want to dereference
Expand All @@ -116,8 +119,7 @@ pub struct Id<T, O: Ownership> {
own: PhantomData<O>,
}

// TODO: Maybe make most of these functions "associated" functions instead?
impl<T: Message, O: Ownership> Id<T, O> {
impl<T: Message + ?Sized, O: Ownership> Id<T, O> {
/// Constructs an [`Id`] to an object that already has +1 retain count.
///
/// This is useful when you have a retain count that has been handed off
Expand Down Expand Up @@ -171,7 +173,10 @@ impl<T: Message, O: Ownership> Id<T, O> {
own: PhantomData,
}
}
}

// TODO: Add ?Sized bound
impl<T: Message, O: Ownership> Id<T, O> {
/// Retains the given object pointer.
///
/// This is useful when you have been given a pointer to an object from
Expand Down Expand Up @@ -254,6 +259,7 @@ impl<T: Message, O: Ownership> Id<T, O> {
// }
// }

// TODO: Add ?Sized bound
impl<T: Message> Id<T, Owned> {
/// Autoreleases the owned [`Id`], returning a mutable reference bound to
/// the pool.
Expand Down Expand Up @@ -291,6 +297,7 @@ impl<T: Message> Id<T, Owned> {
}
}

// TODO: Add ?Sized bound
impl<T: Message> Id<T, Shared> {
/// Autoreleases the shared [`Id`], returning an aliased reference bound
/// to the pool.
Expand All @@ -308,7 +315,7 @@ impl<T: Message> Id<T, Shared> {
}
}

impl<T: Message> From<Id<T, Owned>> for Id<T, Shared> {
impl<T: Message + ?Sized> From<Id<T, Owned>> for Id<T, Shared> {
/// Downgrade from an owned to a shared [`Id`], allowing it to be cloned.
#[inline]
fn from(obj: Id<T, Owned>) -> Self {
Expand All @@ -318,6 +325,7 @@ impl<T: Message> From<Id<T, Owned>> for Id<T, Shared> {
}
}

// TODO: Add ?Sized bound
impl<T: Message> Clone for Id<T, Shared> {
/// Makes a clone of the shared object.
///
Expand All @@ -338,7 +346,7 @@ impl<T: Message> Clone for Id<T, Shared> {
/// borrowed data.
///
/// [dropck_eyepatch]: https://doc.rust-lang.org/nightly/nomicon/dropck.html#an-escape-hatch
impl<T, O: Ownership> Drop for Id<T, O> {
impl<T: ?Sized, O: Ownership> Drop for Id<T, O> {
/// Releases the retained object.
///
/// The contained object's destructor (if it has one) is never run!
Expand All @@ -363,25 +371,25 @@ impl<T, O: Ownership> Drop for Id<T, O> {
/// clone a `Id<T, Shared>`, send it to another thread, and drop the clone
/// last, making `dealloc` get called on the other thread, and violate
/// `T: !Send`.
unsafe impl<T: Sync + Send> Send for Id<T, Shared> {}
unsafe impl<T: Sync + Send + ?Sized> Send for Id<T, Shared> {}

/// The `Sync` implementation requires `T: Sync` because `&Id<T, Shared>` give
/// access to `&T`.
///
/// Additiontally, it requires `T: Send`, because if `T: !Send`, you could
/// clone a `&Id<T, Shared>` from another thread, and drop the clone last,
/// making `dealloc` get called on the other thread, and violate `T: !Send`.
unsafe impl<T: Sync + Send> Sync for Id<T, Shared> {}
unsafe impl<T: Sync + Send + ?Sized> Sync for Id<T, Shared> {}

/// `Id<T, Owned>` are `Send` if `T` is `Send` because they give the same
/// access as having a T directly.
unsafe impl<T: Send> Send for Id<T, Owned> {}
unsafe impl<T: Send + ?Sized> Send for Id<T, Owned> {}

/// `Id<T, Owned>` are `Sync` if `T` is `Sync` because they give the same
/// access as having a `T` directly.
unsafe impl<T: Sync> Sync for Id<T, Owned> {}
unsafe impl<T: Sync + ?Sized> Sync for Id<T, Owned> {}

impl<T, O: Ownership> Deref for Id<T, O> {
impl<T: ?Sized, O: Ownership> Deref for Id<T, O> {
type Target = T;

/// Obtain an immutable reference to the object.
Expand All @@ -393,7 +401,7 @@ impl<T, O: Ownership> Deref for Id<T, O> {
}
}

impl<T> DerefMut for Id<T, Owned> {
impl<T: ?Sized> DerefMut for Id<T, Owned> {
/// Obtain a mutable reference to the object.
#[inline]
fn deref_mut(&mut self) -> &mut T {
Expand All @@ -404,7 +412,7 @@ impl<T> DerefMut for Id<T, Owned> {
}
}

impl<T, O: Ownership> fmt::Pointer for Id<T, O> {
impl<T: ?Sized, O: Ownership> fmt::Pointer for Id<T, O> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Pointer::fmt(&self.ptr.as_ptr(), f)
}
Expand All @@ -414,15 +422,15 @@ impl<T, O: Ownership> fmt::Pointer for Id<T, O> {
//
// See https://doc.rust-lang.org/1.54.0/src/alloc/boxed.rs.html#1652-1675
// and the `Arc` implementation.
impl<T, O: Ownership> Unpin for Id<T, O> {}
impl<T: ?Sized, O: Ownership> Unpin for Id<T, O> {}

impl<T: RefUnwindSafe, O: Ownership> RefUnwindSafe for Id<T, O> {}
impl<T: RefUnwindSafe + ?Sized, O: Ownership> RefUnwindSafe for Id<T, O> {}

// Same as `Arc<T>`.
impl<T: RefUnwindSafe> UnwindSafe for Id<T, Shared> {}
impl<T: RefUnwindSafe + ?Sized> UnwindSafe for Id<T, Shared> {}

// Same as `Box<T>`.
impl<T: UnwindSafe> UnwindSafe for Id<T, Owned> {}
impl<T: UnwindSafe + ?Sized> UnwindSafe for Id<T, Owned> {}

#[cfg(test)]
mod tests {
Expand Down
Loading