Re: Subject: [RFC] clang tooling cleanups

From: Tom Rix
Date: Tue Oct 27 2020 - 17:21:12 EST



On 10/27/20 1:50 PM, Nick Desaulniers wrote:
> On Tue, Oct 27, 2020 at 12:52 PM Joe Perches <joe@xxxxxxxxxxx> wrote:
>> (Adding Stephen Rothwell)
>>
>> On Tue, 2020-10-27 at 12:33 -0700, Tom Rix wrote:
>>> On 10/27/20 11:42 AM, Nick Desaulniers wrote:
>>>> (cutting down the CC list to something more intimate)
>> []
>>
>>> I am interested in treewide fixes.
>> As am I, but here the definition of fixes is undefined though.
>> Whitespace / style changes and other bits that don't change generated
>> object code aren't considered fixes by many maintainers.
>>
>>> Cleaning them up (maybe me not doing all the patches) and keeping them clean.
>>>
>>> The clang -Wextra-semi-stmt -fixit will fix all 10,000 problems
>> I rather doubt there are 10K extra semicolons in the kernel source tree.
>> Is there a proposed diff/patch posted somewhere?
> Ah, I think I see where I (and Joe) may be misinterpreting. Tom, do
> you mean to say that `clang -fixit` will attempt to fix EVERY warning
> in tree (including but regardless of -Wextra-semi-stmt, where
> -Wextra-semi-stmt is a red herring), so the clang-tidy check is
> specific to applying fixits just for -Wextra-semi-stmt? (If so, I
> wonder if we could improve clang to accept more fine grain control
> over *which* fixits we want applied. Not at all of them for all of
> the different distinct warnings, for example).

Using clang and -fixit with -Wextra-semi-stmt will fix all of this warning AND all of the other warnings.

(At least thats what i think it will do)

My opinion, clang doing the fixes is a neat but the purpose of the compiler is to compile.

Fixing should be left to all the other clang-tools.

Tom

>>> This clang tidy fixer will fix only the 100 problems that are 'switch() {};'
>>>
>>> When doing a treewide cleanup, batching a bunch of fixes that are the same problem and fix
>>> is much easier on everyone to review and more likely to be accepted.
>> That depends on the definition of batching.
>>
>> If individual patches are sent to multiple maintainers, the
>> acceptance / apply rate seems always < 50% and some are rejected
>> outright by various maintainers as "unnecessary churn".
>>
>> Single treewide patches are generally not applied unless by Linus.
>> The trivial tree isn't widely used for this.
>>
>> Perhaps a 'scripted' git tree could be established that is integrated
>> into -next that would allow these automated patches to be better
>> vetted and increase the acceptance rate of these automated patches.
>>
>>> Long term, a c/i system would keep the tree clean by running the switch-semi checker/fixer.
>>> And we can all move onto the next problem.
>> Good idea...
>> I hope a scripted patches mechanism will be established.
> Yes, if I can automate myself out of job, then I can hang out of the
> roof and drink margaritas all day. That is the plan of record, but
> this !##$'ing compiler is constantly broken!!!!1
>