Messages in this thread | | | Date | Wed, 03 Apr 2024 15:52:45 +0000 | From | Benno Lossin <> | Subject | Re: [PATCH 2/9] rust: list: add tracking for ListArc |
| |
I think the commit one-line description sounds a bit strange, how about "rust: list: add ListArc tracking strategies"?
On 02.04.24 14:16, Alice Ryhl wrote: > @@ -33,19 +34,64 @@ pub trait ListArcSafe<const ID: u64 = 0> { > unsafe fn on_drop_list_arc(&self); > } > > +/// Declares that this type is able to safely attempt to create `ListArc`s at any time. > +/// > +/// # Safety > +/// > +/// Implementers must ensure that `try_new_list_arc` does not return `true` if a `ListArc` already > +/// exists. > +pub unsafe trait TryNewListArc<const ID: u64 = 0>: ListArcSafe<ID> { > + /// Attempts to convert an `Arc<Self>` into an `ListArc<Self>`. Returns `true` if the > + /// conversion was successful. > + fn try_new_list_arc(&self) -> bool; > +} > + > /// Declares that this type supports [`ListArc`]. > /// > -/// When using this macro, it will only be possible to create a [`ListArc`] from a [`UniqueArc`]. > +/// When using this macro, you may choose between the `untracked` strategy where it is not tracked > +/// whether a [`ListArc`] exists, and the `tracked_by` strategy where the tracking is deferred to a > +/// field of the struct. The `tracked_by` strategy can be combined with a field of type > +/// [`AtomicListArcTracker`] to track whether a [`ListArc`] exists. > #[macro_export] > macro_rules! impl_list_arc_safe { > (impl$({$($generics:tt)*})? ListArcSafe<$num:tt> for $t:ty { untracked; } $($rest:tt)*) => { > - impl$(<$($generics)*>)? $crate::list::ListArcSafe<$num> for $t { > + impl$(<$($generics)*>)? ListArcSafe<$num> for $t {
This change seems unintentional.
> unsafe fn on_create_list_arc_from_unique(&mut self) {} > unsafe fn on_drop_list_arc(&self) {} > } > $crate::list::impl_list_arc_safe! { $($rest)* } > }; > > + (impl$({$($generics:tt)*})? ListArcSafe<$num:tt> for $t:ty { > + tracked_by $field:ident : $fty:ty; > + } $($rest:tt)*) => { > + impl$(<$($generics)*>)? ListArcSafe<$num> for $t {
Here you also want to access `ListArcSafe` via `$crate::list::ListArcSafe`.
> + unsafe fn on_create_list_arc_from_unique(&mut self) { > + let me = self as *mut Self; > + let field: *mut $fty = unsafe { ::core::ptr::addr_of_mut!((*me).$field) };
I think we should also have `SAFETY` comments in macros.
Also why can't this be done using safe code?:
let field: &mut $fty = &mut self.$field;
> + unsafe { <$fty as $crate::list::ListArcSafe<$num>>::on_create_list_arc_from_unique( > + &mut *field > + ) };
Formatting? rustfmt gives me this:
unsafe { <$fty as $crate::list::ListArcSafe<$num>>::on_create_list_arc_from_unique( &mut *field ) };
(maybe the `;` should be inside the `unsafe` block in this case?)
-- Cheers, Benno
> + } > + unsafe fn on_drop_list_arc(&self) { > + let me = self as *const Self; > + let field: *const $fty = unsafe { ::core::ptr::addr_of!((*me).$field) }; > + unsafe { <$fty as $crate::list::ListArcSafe<$num>>::on_drop_list_arc(&*field) }; > + } > + } > + unsafe impl$(<$($generics)*>)? TryNewListArc<$num> for $t > + where > + $fty: TryNewListArc<$num>, > + { > + fn try_new_list_arc(&self) -> bool { > + let me = self as *const Self; > + let field: *const $fty = unsafe { ::core::ptr::addr_of!((*me).$field) }; > + unsafe { <$fty as $crate::list::TryNewListArc<$num>>::try_new_list_arc(&*field) } > + } > + } > + $crate::list::impl_list_arc_safe! { $($rest)* } > + }; > + > () => {}; > } > pub use impl_list_arc_safe;
| |