Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
MIME-Version: 1.0
References: <20230921213440.202017-1-wedsonaf@gmail.com> <20230921213440.202017-3-wedsonaf@gmail.com>
 <20230923081251.2f702cc7@gary-lowrisc-laptop>
In-Reply-To: <20230923081251.2f702cc7@gary-lowrisc-laptop>
From:   Wedson Almeida Filho <wedsonaf@gmail.com>
Date:   Sat, 23 Sep 2023 11:11:33 -0300
Message-ID: <CANeycqoYtbE1eMOoX_ZBp_Ti-jg9eJ+JBmHjiV=nno+ocvvOqA@mail.gmail.com>
Subject: Re: [PATCH 2/2] rust: arc: remove `ArcBorrow` in favour of `WithRef`
To:     Gary Guo <gary@garyguo.net>
Cc:     rust-for-linux@vger.kernel.org, Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Boqun Feng <boqun.feng@gmail.com>,
        =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>,
        Benno Lossin <benno.lossin@proton.me>,
        Andreas Hindborg <a.hindborg@samsung.com>,
        Alice Ryhl <aliceryhl@google.com>,
        linux-kernel@vger.kernel.org,
        Wedson Almeida Filho <walmeida@microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.)

Thanks,
-Wedson