On Thu, Jun 20, 2019 at 1:12 PM Stefan Reinauer
<[email protected]> wrote:
>
>
>
> On 20 Jun 2019 08:26, ron minnich <[email protected]> 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.
_______________________________________________
coreboot mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to