Re: [PATCH v4] scripts: checkpatch: warn on Rust panicking methods

[Dirk Behme <dirk.behme@gmail.com>]

On 03.02.26 17:54, Gary Guo wrote:
> On Tue Feb 3, 2026 at 4:32 PM GMT, Onur Özkan wrote:
>> On Tue, 03 Feb 2026 16:02:02 +0000
>> "Gary Guo" <gary@garyguo.net> wrote:
>>
>>> On Tue Feb 3, 2026 at 3:49 PM GMT, Onur Özkan wrote:
>>>> On Tue,  3 Feb 2026 08:25:41 -0700
>>>> Jkhall81 <jason.kei.hall@gmail.com> wrote:
>>>>
>>>>> Nice, emails sent from gmail get automatically rejected.
>>>>>
>>>>> So, Dirk.  To satisfy your concerns the current 10ish line
>>>>> code update is going to slowly, after many more emails
>>>>> written in nano, mutate into a franken-regex-perl beast. 
>>>>> checkpatch.pl is already huge.  I'm not a fan of this 
>>>>> approach.
>>>>
>>>> Me neither. I wonder why we are doing this instead of using the
>>>> unwrap_used and expect_used linting rules from clippy. This would
>>>> catch the problem much earlier than checkpath since many of us build
>>>> the kernel with CLIPPY=1 flag.
>>>
>>> Because it's okay to `panic` or use `expect`. checkpatch will just
>>> warn you once when the code is introduced, not continuously in each
>>> build.
>>
>> That's interesting because it implies that it's okay for people to use
>> them without "// PANIC..." comments. That sounds problematic since it
>> means some instances will have that comment while others may not.
> 
> My personal view has always been it's okay to not have it. It's a burden to
> developers to always have to write these. In many cases, `panic()` or `expect()`
> calls are needed because you have something of `Option<>` and you know it's not
> going to be `None`. The C equivalent would just be a pointer and you use it
> without checking for NULL; we never ask people to justify why they feel it's
> okay to dereference a pointer.
> 
> Sure, if people would like to justify why they think it won't panic, brilliant,
> keeping doing it. But I don't want to make it harder for people to write Rust
> code compared to C.


The question is if we could find a way to make it *consistent*?

I mean how should a developer know if the warning (he gets once, or
even if he checks an existing file with -f always) is relevant or not?
We introduce the warning because we want to discourage the use of
`unwrap()`. At the same time there are places where its usage is
allowed or even needed. How to know what is valid? The warning or the
usage?

So do we find an acceptable way to mark the allowed ones? Or do we
drop this patch entirely and hope to catch the "forbidden" ones by
review? Or?

Best regards

Dirk


>> In my opinion, adding a clippy rule and using "#[allow(...)]" in the
>> places where it's acceptable to use them makes more sense. This is at
>> least more consistent and doesn't bloat the checkpatch file.
> 
> `#[allow()]` looks quite verbose, and also you cannot apply it everywhere (just
> blocks or items).
> 
> Best,
> Gary
> 
>