Re: [PATCH v5 6/6] rust: use strict provenance APIs
From: Tamir Duberstein
Date: Mon Mar 17 2025 - 16:36:36 EST
On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@xxxxxxxxx> wrote:
>
> On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote:
> > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@xxxxxxxxx> wrote:
> > >
> > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@xxxxxxxxx> wrote:
> > > >
> > > > Then we should fix clippy or how we set msrv rather adding the stub.
> > > > @Miguel?
> > >
> > > I filed https://github.com/rust-lang/rust-clippy/issues/14425.
> >
> > I don't think we can wait for that to be fixed, though. Usually clippy
> > is distributed with rustc via rustup, so even if this is eventually
> > fixed, all versions between 1.84.0 and the fix will need this
> > workaround until MSRV is >= 1.84.0.
>
> We need to take one step back to evalute this "workaround".
>
> First, expose_provenance() and with_exposed_provenance{,_mut}() API are
> clearly defined as equavilent to `as` operation [1]. Therefore, the
> changes in this patch doing the conversion with expose_provenance() and
> with_exposed_provenance{,_mut}() don't change anything related to
> provenance in practice.
>
> I do agree we want to use the explicit provenance API, but I don't think
> we want to introduce some API that we know we will change them latter
> when we bump the rustc minimal version. So the question is: are these
> stubs what we want even though in the future our minimal rustc version
> stablizes provenance API? If not, then the cost of this patch cannot
> justify its benefits IMO.
>
> Now let's also look into why we choose a msrv for clippy, I would guess
> it's because we need to support all the versions of rustc starting at
> 1.78 and we want clippy to report a problem based on 1.78 even though
> we're using a higher version of rustc. But for this particular case, we
> use a feature that has already been stablized in a higher version of
> rustc, which means the problem reported by clippy doesn't help us, nor
> does it provide better code. Frankly speaking, I think we have other
> ways to ensure the support of all rustc versions without a msrv for
> clippy. If I was to choose, I would simply drop the msrv. But maybe I'm
> missing something.
>
> The point is tools should help us to write good and maintainable code,
> we shouldn't introduce complicated structure of code just because some
> tools fail to do its job.
>
> [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html
Even if we globally disable this clippy lint, we still need stubs
because exposed_provenance was added in 1.79.0. Did your suggestion
address this? Perhaps I missed it.