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.

Stefan

The history of reviewers looking at code is they miss this kind of
error. Constantly. I'm in favor of as much automation as we can get.

ron

On Thu, Jun 20, 2019 at 5:25 AM Nico Huber <nico.h@gmx.de> wrote:
>
> On 20.06.19 06:01, Jacob Garber wrote:
> > On Wed, Jun 19, 2019 at 08:38:14PM -0700, ron minnich wrote:
> >> Given the number of serious problems that lack of braces causes, I
> >> like this proposal. It's indicative that both Rust and Go require the
> >> {}, for reasons of safety.
> >
> > There was a famous vulnerability in Apple's SSL code several years ago
> > because of lack of braces. clang-format can also reformat old code to have
> > mandatory braces if I'm not mistaken.
>
> What will clang-format do if it encounters?
>
>     if (foo)
>             bar();
>             baz();
>
> a)
>     if (foo) {
>             bar();
>     }
>     baz();
>
> or b)
>     if (foo) {
>             bar();
>             baz();
>     }
>
> Will it spit out a warning? If not, this shows how dangerous automatic
> formatting can be. Because after the formatter run, it's much less ob-
> vious for the reviewer that something is wrong.
>
> Nico
_______________________________________________
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-leave@coreboot.org