Re: Christmas wish list: -Werror and -Wno-warn-about-X

["Herbert, Marc" <marc.herbert@intel.com>]

I just realized I never answered this? Sorry.

Adding a couple people in Bcc:, full thread is here:
https://lore.kernel.org/linux-sparse/3eifvstptigzcjthdjwshxxxqtn3yjtm4jifhs4sqcsrqhozjh@6ixifzmsekkq/


> On 19 Jan 2024, at 17:27, Luc Van Oostenryck <lucvoo@kernel.org> wrote:
> 
> As far as I understand it, -Werror has been replaced by -Wsparse-error in
> 2014 because sparse was mainly used with exactly the same options as the
> ones used for the compiler (which is good), there was too much error
> messages and people didn't want to have their build to fail (see commits
> 4d8811879a1c & fe57afa0b44a). So, essentially the exact opposite of what
> you would like :(
> 
> -Wsparse-error is really a pain because it also turn every warning into
> an error (and then exit on these errors).
> 
> It should indeed be possible to have an exit(1) on errors but I'm very wary
> of changing the current behaviour. What about something like -ffail-on-error?


I'm not sure how the ideal user interface should look like but I know
what it should _provide_. It should be possible to make this decision
for each error type/check individually:

- silence
- warning, exit 0
- error, exit !0

gcc does achieve this. It's a bit awkard and probably not the best user
interface but consistency and familiarity is key so why not the same UI
for sparse?
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

So this is one way CI has been successfully handling gcc warnings:

- First, define the subset of "forbidden" warnings in your project.

- Craft the corresponding -Wall + -Wno-warn-something command line.

- Hardcode that list in the project build configuration.

- Provide come EXTRA_CFLAGS= parameter for local/temporary
  fine-tuning/experimentation.

- Do NOT hardcode -Werror not to inconvenience developers. Obviously,
  sometimes you want to run tests on code with warnings.

- Do hardcode EXTRA_CFLAGS=-Werror in CI

And that's it, grep-free!

If you need even more flexibility, use -W[no-]error=some-warning




>> So far we've been using a brittle script that captures the incredibly
>> verbose and mostly unusable output (mostly due to some hard-to-fix
>> warnings located in common .h files) and "post-processes" it
>> with... "grep".
>> https://github.com/thesofproject/sof/commit/b7708182fbe5d
>> 
>> I'm curious how people typically automate runnning sparse on the
>> Linux kernel. Does everyone "greps" logs too? Or is it more like
>> `$(wc -l) == 0`?
> 
> I think so (but I could be very wrong). An exit on error is only useful
> if the output is clean and the errors not too frequent. I think it's
> often not the case.

This is a problem only with parallel builds that have mostly unreadable
output anyway not matter what. Here's a dead-simple and very robust
workaround that I have successfully used a lot in CI:

   make -j || make -j1

Ta-da: now the _error_ that stopped the build is last, _after_ all the
warnings that did not.

> But, if your main problem is with error messages concerning address spaces,
> have you tried to simply use -Wno-address-space?

Funny you say that because address spaces is the one thing we care about
the most and why we started using sparse :-D It's the only thing
we "grep" for failures:

https://github.com/thesofproject/sof/blob/d345c56e71b495a1e4eec1a48e48d3d5be055cda/scripts/parse_sparse_output.sh 

But I get your point: "someone" should probably look at what -Wno-stuff
options sparse has available.

Also note: some warnings point at "real" issues and if you silence them
then they will never get fixed :-)

As a coincidence, this sign warning was just fixed after 2 years:
https://github.com/zephyrproject-rtos/zephyr/issues/53505
It was especially verbose (half the total!) because located in a .h file.


> Some other error messages issued by sparse can't be disabled but most of these
> are, I think, parsing or typing errors that are more or less unrecoverable.

Yes: failing on unrecoverable errors is good :-)


> Purely for my own curiosity, could you send me one of your log of sparse
> errors?

Running daily there:
https://github.com/thesofproject/sof/actions/workflows/daily-tests.yml 

-> Sparse Zephyr -> zephyr-sparse-analyzer / warnings-subset 
https://github.com/thesofproject/sof/actions/runs/11206675653/job/31147770359

Corresponding script is here: 
https://github.com/thesofproject/sof/commit/0061953a595b18c12a5962edced12d9ac9ac1ce2

Take care

Marc