Attention is currently required from: Nico Huber, Stefan Reinauer, David Hendricks, Paul Menzel, Angel Pons, ron minnich, Felix Held. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51825 )
Change subject: Documentation/contributing/coding_style: remove bug-causing rule ......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
For reference, Patrick brought this up in 2018 ("Proposing a change to Coding Style" [1]) when he ran into a bug in the Chrome EC codebase due to braces.
Well, yeah, as you can see I participated in that discussion (CB:26368) and voiced the same position already. It seems we were so divided on the issue that the suggestion was abandoned, so I don't really see how that would be any reason to now just pick this back up after 2 years and merge it without further discussion?
BTW it seems that I already found and suggested to use the very same checkpatch fix back then (and forgot about the details since). Unfortunately it doesn't seem like anybody was interested to look into that solution back then, despite the apparent concern about this error class. So this time I spent the extra effort myself to get it working and test it extensively, and yes, it does actually do a very impressive job (catching simple issues like Felix' CB:51786 and much more complicated ones). I still don't see any concrete evidence that linters can't do this, it's just that it doesn't seem anybody who cares about this so much ever bothered to write one. So here, I have one, it works very well, all I need is a +2 and we can put it to work. Can we please just do that rather than creating new and unnecessary hassle to developers and reviewers to solve a problem that a machine can solve for us?
Why is checkpatch so large? Because it's trying to do something that can't be done: parse a context-free grammar (C) with REs. Obligatory theory slides: https://www.dfki.de/compling/pdfs/cfg-slides.pdf
The part that's doing the statement parsing for this purpose is using Perl code and not trying to cover everything in a single RE, so this doesn't apply. See the ctx_statement_block() function for the gnarly but quite functional details.