Message-ID: <CAJ-ks9n1oGAGSrXYWjvR+_raw8h+skkdfSYpeSuQZ9jEs5q-6Q@mail.gmail.com>
Date: Thu, 13 Mar 2025 13:50:06 -0400
From: Tamir Duberstein <tamird@...il.com>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: Masahiro Yamada <masahiroy@...nel.org>, Nathan Chancellor <nathan@...nel.org>, 
	Nicolas Schier <nicolas@...sle.eu>, Miguel Ojeda <ojeda@...nel.org>, 
	Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, 
	Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, 
	Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Brendan Higgins <brendan.higgins@...ux.dev>, David Gow <davidgow@...gle.com>, 
	Rae Moar <rmoar@...gle.com>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Luis Chamberlain <mcgrof@...nel.org>, Russ Weight <russ.weight@...ux.dev>, Rob Herring <robh@...nel.org>, 
	Saravana Kannan <saravanak@...gle.com>, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, 
	linux-pci@...r.kernel.org, linux-block@...r.kernel.org, 
	devicetree@...r.kernel.org
Subject: Re: [PATCH v2 5/5] rust: enable `clippy::as_underscore` lint

On Thu, Mar 13, 2025 at 10:11 AM Benno Lossin <benno.lossin@...ton.me> wrote:
>
> On Thu Mar 13, 2025 at 11:47 AM CET, Tamir Duberstein wrote:
> > On Wed, Mar 12, 2025 at 6:38 PM Benno Lossin <benno.lossin@...ton.me> wrote:
> >>
> >> On Wed Mar 12, 2025 at 11:24 PM CET, Tamir Duberstein wrote:
> >> > On Wed, Mar 12, 2025 at 5:30 PM Benno Lossin <benno.lossin@...ton.me> wrote:
> >> >>
> >> >> On Wed Mar 12, 2025 at 10:10 PM CET, Tamir Duberstein wrote:
> >> >> > On Wed, Mar 12, 2025 at 5:04 PM Tamir Duberstein <tamird@...il.com> wrote:
> >> >> >>
> >> >> >> On Wed, Mar 12, 2025 at 5:01 PM Benno Lossin <benno.lossin@...ton.me> wrote:
> >> >> >> > Always enable the features, we have `allow(stable_features)` for this
> >> >> >> > reason (then you don't have to do this dance with checking if it's
> >> >> >> > already stable or not :)
> >> >> >>
> >> >> >> It's not so simple. In rustc < 1.84.0 the lints *and* the strict
> >> >> >> provenance APIs are behind `feature(strict_provenance)`. In rustc >=
> >> >> >> 1.84.0 the lints are behind `feature(strict_provenance_lints)`. So you
> >> >> >> need to read the config to learn that you need to enable
> >> >> >> `feature(strict_provenance_lints)`.
> >> >>
> >> >> I see... And `strict_provenance_lints` doesn't exist in <1.84? That's a
> >> >> bit of a bummer...
> >> >>
> >> >> But I guess we could have this config option (in `init/Kconfig`):
> >> >>
> >> >>     config RUSTC_HAS_STRICT_PROVENANCE
> >> >>             def_bool RUSTC_VERSION >= 108400
> >> >>
> >> >> and then do this in `lib.rs`:
> >> >>
> >> >>     #![cfg_attr(CONFIG_RUSTC_HAS_STRICT_PROVENANCE, feature(strict_provenance_lints))]
> >> >
> >> > Yep! That's exactly what I did, but as I mentioned up-thread, the
> >> > result is that we only cover the `kernel` crate.
> >>
> >> Ah I see, can't we just have the above line in the other crate roots?
> >
> > The most difficult case is doctests. You'd have to add this to every
> > example AFAICT.
> >
> >> >> > Actually this isn't even the only problem. It seems that
> >> >> > `-Astable_features` doesn't affect features enabled on the command
> >> >> > line at all:
> >> >> >
> >> >> > error[E0725]: the feature `strict_provenance` is not in the list of
> >> >> > allowed features
> >> >> >  --> <crate attribute>:1:9
> >> >> >   |
> >> >> > 1 | feature(strict_provenance)
> >> >> >   |         ^^^^^^^^^^^^^^^^^
> >> >>
> >> >> That's because you need to append the feature to `rust_allowed_features`
> >> >> in `scripts/Makefile.build` (AFAIK).
> >> >
> >> > Thanks, that's a helpful pointer, and it solves some problems but not
> >> > all. The root Makefile contains this bit:
> >> >
> >> >> KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \
> >> >> -Zallow-features= $(HOSTRUSTFLAGS)
> >> >
> >> > which means we can't use the provenance lints against these host
> >> > targets (including e.g. `rustdoc_test_gen`). We can't remove this
> >> > -Zallow-features= either because then core fails to compile.
> >> >
> >> > I'm at the point where I think I need more involved help. Want to take
> >> > a look at my attempt? It's here:
> >> > https://github.com/tamird/linux/tree/b4/ptr-as-ptr.
>
> With doing `allow(clippy::incompatible_msrv)`, I meant doing that
> globally, not having a module to re-export the functions :)

Yeah, but I think that's too big a hammer. It's a useful warning, and
it doesn't accept per-item configuration.

> >> I'll take a look tomorrow, you're testing my knowledge of the build
> >> system a lot here :)
> >
> > We're guaranteed to learn something :)
>
> Yep! I managed to get it working, but it is rather janky and
> experimental. I don't think you should use this in your patch series
> unless Miguel has commented on it.
>
> Notable things in the diff below:
> * the hostrustflags don't get the *provenance_casts lints (which is
>   correct, I think, but probably not the way I did it with filter-out)
> * the crates compiler_builtins, bindings, uapi, build_error, libmacros,
>   ffi, etc do get them, but probably shouldn't?

Why don't we want host programs to have the same warnings applied? Why
don't we want it for all those other crates?

I'd expect we want uniform diagnostics settings throughout which is
why these things are in the Makefile rather than in individual crates
in the first place.

Your patch sidesteps the problems I'm having by not applying these
lints to host crates, but I think we should.

Powered by blists - more mailing lists