On Thu, Jun 20, 2019 at 1:12 PM Stefan Reinauer stefan.reinauer@coreboot.org wrote:
On 20 Jun 2019 08:26, ron minnich rminnich@gmail.com wrote:
clang-format is not a textual preprocessor. It is basically the ast builder of followed by output.
So in your case, I just tried it main() {
if (foo) bar(); baz(); }
and got the right thing, i.e. braces around bar but not baz.
The right thing (e.g. the obviously intended thing) would be too have braces around both.
clang-format in this case masks the problem and makes it harder to identify by expanding the syntax of the unwanted behavior to look intentional.
Nico and Stefan, you make a good point, but I would then argue for even better tools, like clang-tidy: /tmp/x.c:13:10: warning: statement should be inside braces [readability-braces-around-statements] if (foo) ^ {
In this case, there is a warning thrown, and the author has to clean it up.
I don't believe, based on a lot of the history of this sort of problem in C, that we should depend on human reviewers to catch mistakes like this. These tools exist because of a demonstrated need. I think coreboot could benefit from their proper application.
You've presented a good case, but what about something like this: if (foo) bar(); baz();
what's intended? There's an indent, but it's partial. I would not want to guess. But I agree with you that an invisible fixup would be inappropriate.