[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANiq72=68-daT3J_NVvp+7NkGFWFPGQmwoXn4m54VYeRKPUkbA@mail.gmail.com>
Date: Fri, 6 Feb 2026 18:41:27 +0100
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Dirk Behme <dirk.behme@...bosch.com>, Gary Guo <gary@...yguo.net>
Cc: Jason Hall <jason.kei.hall@...il.com>, Joe Perches <joe@...ches.com>,
Miguel Ojeda <ojeda@...nel.org>, Dirk Behme <dirk.behme@...il.com>,
Onur Özkan <work@...rozkan.dev>,
rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
Boqun Feng <boqun.feng@...il.com>, Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>, Andy Whitcroft <apw@...onical.com>,
Dwaipayan Ray <dwaipayanray1@...il.com>, Lukas Bulwahn <lukas.bulwahn@...il.com>
Subject: Re: [PATCH v5] scripts: checkpatch: move Rust-specific lints to
separate file
On Fri, Feb 6, 2026 at 9:32 AM Dirk Behme <dirk.behme@...bosch.com> wrote:
>
> Gary had some concerns about `expect()` in
Yeah, for `expect()` in `checkpatch.pl`, the current message does not
make much sense, i.e. perhaps we want a slightly different one,
without the `// PANIC` bit.
Or perhaps, like Gary says, we don't warn for that one, at least for
now. I think we discussed pros/cons of using `expect()` to begin with
recently -- Gary?
> I'm totally fine with this. In consequence do we agree that this will
> result in some follow up patches annotating the existing "allowed"
> exceptional / intended cases (e.g. in doctests and tests) with `//
> PANIC:` comments?
Those are the ones that would not need annotations, i.e. they would be
false positives.
As far as I remember, the idea with the Clippy lint was to allow it in
certain places like e.g. `assert()`s inside doctests for sure (and
perhaps in doctests in general). Thus no `// PANIC:` needed for
certain cases.
And here for `checkpatch.pl`, if it were to trigger in a similar
situation, then it would be a false positive, and thus the developer
should not add it. But the patch here skips if it is inside a comment
(or doctest), no?
So the remaining concern if I understand correctly is too many false
positives inside `#[test]`s but outside an `assert()`s? Yeah, we may
need to track if we have seen `kunit_tests` (and reset if we go out of
the test module), to skip a bunch of warnings.
By the way, I would prefer we expand the comment on top of the line
explaining what it is supposed to cover, in order to evaluate whether
the regexes etc. are doing what we expect or not.
Thanks Dirk et al. for diving into this -- we do need to move forward
this topic, and there have been some disagreements on how much is too
much to warn about :)
Cheers,
Miguel
Powered by blists - more mailing lists