Messages in this thread | | | From | Wedson Almeida Filho <> | Date | Sat, 23 Sep 2023 11:20:15 -0300 | Subject | Re: [PATCH 2/2] rust: arc: remove `ArcBorrow` in favour of `WithRef` |
| |
On Sat, 23 Sept 2023 at 11:15, Alice Ryhl <aliceryhl@google.com> wrote: > > On Sat, Sep 23, 2023 at 4:11 PM Wedson Almeida Filho <wedsonaf@gmail.com> wrote: > > > > On Fri, 22 Sept 2023 at 21:13, Gary Guo <gary@garyguo.net> wrote: > > > > > > On Thu, 21 Sep 2023 18:34:40 -0300 > > > Wedson Almeida Filho <wedsonaf@gmail.com> wrote: > > > > > > > From: Wedson Almeida Filho <walmeida@microsoft.com> > > > > > > > > With GATs, we don't need a separate type to represent a borrowed object > > > > with a refcount, we can just use Rust's regular shared borrowing. In > > > > this case, we use `&WithRef<T>` instead of `ArcBorrow<'_, T>`. > > > > > > > > Co-developed-by: Boqun Feng <boqun.feng@gmail.com> > > > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com> > > > > Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com> > > > > > > Reviewed-by: Gary Guo <gary@garyguo.net> > > > > > > The implementation looks good to me, thanks Wedson. > > > > > > A minor thing that worth considering is to implement `AlwaysRefCounted` > > > to `WithRef` and reimplement `Arc` with `ARef<TaskRef<T>>` or add > > > conversion functions between them. > > > > > > It feels natural to have this this impl (because `WithRef` is indeed > > > always ref counted), but on the other hand I don't currently foresee > > > anyone to currently want to use this API :( > > > > Yes, I like the idea of defining `Arc<T>` as `ARef<WithRef<T>>`. My > > concern had to do with error messages for type aliases but you or > > Benno (or Bjorn?) had a link to plans from the compiler team to > > improve this. And we're using type aliases for our locks anyway. > > > > So this change is in my queue. (As well as the changes to the guard we > > discussed a couple of months ago.) > > Will this not cause issues with DispatchFromDyn and the Receiver > trait? Maybe it's best to have Arc be a real type and just provide a > conversion?
I don't think it will but I haven't really tried it yet so can't say for sure.
One thing I learned (obvious in hindsight) when doing the locks is that we can have impl blocks for specialisations of a type, so `DispatchFromDyn<Arc<T>>` will expand to `DispatchFromDyn<ARef<WithRef<T>>>` and it should be ok.
Or are you worried about something else?
| |