All of lore.kernel.org
 help / color / mirror / Atom feed
From: Danilo Krummrich <dakr@kernel.org>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Abdiel Janulgue <abdiel.janulgue@gmail.com>,
	rust-for-linux@vger.kernel.org, aliceryhl@google.com,
	dakr@redhat.com, linux-kernel@vger.kernel.org,
	airlied@redhat.com, boqun.feng@gmail.com,
	Christian Brauner <brauner@kernel.org>
Subject: Re: [PATCH v2 0/5] Introduce Owned type and Ownable trait (was: "rust: page: Add support for vmalloc_to_page")
Date: Wed, 23 Oct 2024 14:01:56 +0200	[thread overview]
Message-ID: <ZxjltAbZmzbjXnKn@pollux> (raw)
In-Reply-To: <CANiq72nagKRUmHWVOFGNBuiVnqdmgqPm8b6ZtG1edobmrQMr3w@mail.gmail.com>

On Wed, Oct 23, 2024 at 11:51:47AM +0200, Miguel Ojeda wrote:
> On Wed, Oct 23, 2024 at 10:03 AM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > I wonder if it would make sense to make `CLIPPY=1` the default and only disable
> > it by explicitly passing `CLIPPY=0`.
> 
> That is what I wanted, but when I asked long ago to the Clippy
> maintainers if using `clippy-driver` was guaranteed to not affect
> codegen, the answer was no: for instance, optimization may be affected
> (at least back then), and the maintainers said the intention is that
> is not to be used for normal compiling. So I sent a PR to document
> that. See:
> 
>     https://github.com/rust-lang/rust-clippy/issues/8035
>     https://github.com/rust-lang/rust-clippy/pull/8037

That's pretty unfortunate, I didn't know.

I think for the long term it'd be good to find a way though. Once more and more
subsystems / people start adding Rust code, I could imagine patches to start
slipping through and fixing things up after it's been discovered in -next would
be painful.

> 
> Similarly, Christian proposed running `rustfmtcheck` unconditionally
> on build and offering a way to turn it off instead. I think that would
> be ideal too, but it could potentially lead to problems too, so I am
> not sure either; see e.g.:
> 
>     https://lore.kernel.org/rust-for-linux/CANiq72==AkkqCDaZMENQRg8cf4zdeHpTHwdWS3sZiFWm0vyJUA@mail.gmail.com/

Yeah, that's a tricky one; if not enabled by default I'd be a bit worried about
the same thing to happen as mentioned above.

Additionally, for development trees where things slipped through it'd be
annoying when `rustfmt` changes more stuff than expected.

> 
> So I wonder if we should instead go with a "dev mode" like `D=1` that
> enables Clippy, `rustfmtcheck`, `-Dwarnings` (even if `WERROR=n` and
> applying to everything, not just kernel objects,), potentially
> `rustdoc`-related warnings too, and whatever other tooling/checks in
> the future (e.g. klint), and not just for Rust but potentially for C
> and other bits too (e.g. `W=1`, some important subset of Coccinelle
> scripts...).

I think that'd be great for short / mid term, it'd make it much easier to ensure
that all relevant checks were executed and hence make it less likely for things
slip through.

For the long term, I think it'd be great to keep looking for ways to always
enable the clippy and format checks. Or at least the clippy ones if we're too
concerned about `rustfmt` to break.

> 
> That way, "normal builds" (i.e. those done by users) stay as
> fast/clean/warning-free/bug-free/optimized as possible even across
> compiler versions, potential bugs in the toolchain, etc. And I imagine
> it would be easier for newcomers, too.
> 
> Opinions welcome! I am happy to prepare an RFC, since it seems a few
> people would like something like that.
> 
> Cheers,
> Miguel
> 

      reply	other threads:[~2024-10-23 12:02 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-22 22:44 [PATCH v2 0/5] Introduce Owned type and Ownable trait (was: "rust: page: Add support for vmalloc_to_page") Abdiel Janulgue
2024-10-22 22:44 ` [PATCH v2 1/5] rust: types: add `Owned` type and `Ownable` trait Abdiel Janulgue
2024-10-23  8:10   ` Danilo Krummrich
2024-10-23  9:28   ` Alice Ryhl
2024-10-23 10:26     ` Abdiel Janulgue
2024-10-23 14:58       ` Boqun Feng
2024-10-23 17:52         ` Alice Ryhl
2024-10-23 18:07           ` Boqun Feng
2024-10-24  7:23             ` Alice Ryhl
2024-10-24  7:33               ` Alice Ryhl
2024-11-01 13:38                 ` Abdiel Janulgue
2024-11-01 13:49                   ` Alice Ryhl
2024-10-22 22:44 ` [PATCH v2 2/5] rust: page: Make ownership of the page pointer explicit Abdiel Janulgue
2024-10-22 22:44 ` [PATCH v2 3/5] rust: page: Extend support to vmalloc_to_page Abdiel Janulgue
2024-10-23  8:42   ` Danilo Krummrich
2024-10-23  9:03     ` Danilo Krummrich
2024-10-23 10:26     ` Abdiel Janulgue
2024-10-23 11:30       ` Danilo Krummrich
2024-10-22 22:44 ` [PATCH v2 4/5] rust: page: Add page_slice_to_page Abdiel Janulgue
2024-10-22 22:44 ` [PATCH v2 5/5] rust: firmware: implement `Ownable` for Firmware Abdiel Janulgue
2024-10-23  9:35   ` Danilo Krummrich
2024-10-23  9:45     ` Danilo Krummrich
2024-10-27 22:20     ` Boqun Feng
2024-10-28 13:37       ` Danilo Krummrich
2024-10-23  8:03 ` [PATCH v2 0/5] Introduce Owned type and Ownable trait (was: "rust: page: Add support for vmalloc_to_page") Danilo Krummrich
2024-10-23  9:51   ` Miguel Ojeda
2024-10-23 12:01     ` Danilo Krummrich [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZxjltAbZmzbjXnKn@pollux \
    --to=dakr@kernel.org \
    --cc=abdiel.janulgue@gmail.com \
    --cc=airlied@redhat.com \
    --cc=aliceryhl@google.com \
    --cc=boqun.feng@gmail.com \
    --cc=brauner@kernel.org \
    --cc=dakr@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=rust-for-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.