diff --git a/objc2/src/rc/id.rs b/objc2/src/rc/id.rs index 44ecc956f..6c6c06837 100644 --- a/objc2/src/rc/id.rs +++ b/objc2/src/rc/id.rs @@ -173,10 +173,7 @@ impl Id { own: PhantomData, } } -} -// TODO: Add ?Sized bound -impl Id { /// Retains the given object pointer. /// /// This is useful when you have been given a pointer to an object from @@ -212,14 +209,20 @@ impl Id { #[doc(alias = "objc_retain")] #[cfg_attr(not(debug_assertions), inline)] pub unsafe fn retain(ptr: NonNull) -> Id { - let ptr = ptr.as_ptr() as *mut ffi::objc_object; + let ptr = ptr.as_ptr(); + let obj_ptr = ptr as *mut ffi::objc_object; // SAFETY: The caller upholds that the pointer is valid - let res = unsafe { ffi::objc_retain(ptr) }; - debug_assert_eq!(res, ptr, "objc_retain did not return the same pointer"); + let res = unsafe { ffi::objc_retain(obj_ptr) }; + debug_assert_eq!(res, obj_ptr, "objc_retain did not return the same pointer"); // SAFETY: Non-null upheld by the caller, and `objc_retain` always // returns the same pointer, see: // https://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-runtime-objc-retain - let res = unsafe { NonNull::new_unchecked(res as *mut T) }; + // + // TODO: Use `res` instead, in the odd case where `objc_retain` did + // not return the same pointer. We can't do this currently because we + // have a ?Sized bound on T to support extern types, which means we + // can go from `*mut T` to `*mut objc_object`, but not the other way. + let res = unsafe { NonNull::new_unchecked(ptr) }; // SAFETY: We just retained the object, so it has +1 retain count unsafe { Self::new(res) } } @@ -232,14 +235,19 @@ impl Id { // retained objects it is hard to imagine a case where the inner type // has a method with the same name. - let ptr = ManuallyDrop::new(self).ptr.as_ptr() as *mut ffi::objc_object; + let ptr = ManuallyDrop::new(self).ptr.as_ptr(); + let obj_ptr = ptr as *mut ffi::objc_object; // SAFETY: The `ptr` is guaranteed to be valid and have at least one // retain count. // And because of the ManuallyDrop, we don't call the Drop // implementation, so the object won't also be released there. - let res = unsafe { ffi::objc_autorelease(ptr) }; - debug_assert_eq!(res, ptr, "objc_autorelease did not return the same pointer"); - res as *mut T + let res = unsafe { ffi::objc_autorelease(obj_ptr) }; + debug_assert_eq!( + res, obj_ptr, + "objc_autorelease did not return the same pointer" + ); + // TODO: Return `res`, see explanation in `retain`. + ptr } // TODO: objc_retainAutoreleasedReturnValue @@ -259,8 +267,7 @@ impl Id { // } // } -// TODO: Add ?Sized bound -impl Id { +impl Id { /// Autoreleases the owned [`Id`], returning a mutable reference bound to /// the pool. /// @@ -297,8 +304,7 @@ impl Id { } } -// TODO: Add ?Sized bound -impl Id { +impl Id { /// Autoreleases the shared [`Id`], returning an aliased reference bound /// to the pool. /// @@ -325,8 +331,7 @@ impl From> for Id { } } -// TODO: Add ?Sized bound -impl Clone for Id { +impl Clone for Id { /// Makes a clone of the shared object. /// /// This increases the object's reference count.