GCC 6: -Wmisleading-indentation vs. “goto fail;” | Hacker News Reader

GCC 6: -Wmisleading-indentation vs. “goto fail;”

developerblog.redhat.com

199 pointsdmalcolm165 comments

Making braces optional in single-statement if/else/while/for clauses is one of the biggest anti-features in C. It's frustrating that it was ported forward to more modern languages like Java, JavaScript, C#, etc.

I'm glad Python (with semantic whitespace) and Go (with gofmt) solve this problem.

In Rust, code like this simply won't compile http://is.gd/e6mBlG

Well, yeah, but that's not a huge advantage of Rust. In any language where that's not an allowed construct (and there's quite a few), it won't compile. It won't compile in Perl either (It fails at the compilation stage, not at runtime).

Single-line ifs are pretty useful with traditional-style C libraries that expect all checks to be done at call site:

  if (ptr) call_oldschool_thingy(ptr);

I'm not sure that expanding your statement to multiple lines is any less "useful". I suppose in certain cases it can make code much more verbose, as one line effectively becomes 3-4, which can make it harder to navigate.

Fairly often there's a whole sequence of this type of calls. For example cleanup of multiple objects:

  if (image) foo_release(image);
  if (label) foo_release(label);
  if (data) foo_buffer_destroy(data);
  if (window) foo_window_destroy(window);

It's easier to see that all objects are being cleaned up when each occupies just one line, as that typically matches the look of the initialization:

  window = foo_window_create();
  data = foo_buffer_create(1, 2, 3);
  label = foo_label_create(window);
  image = foo_image_create(window);

Those if statement should be written using the ternary operator. In my subjective opinion, putting the expression in the same line as the if statement is awful. Objectively it is worse because you create a possibility of certain types of errors, like a hanging statement or similar. Ternary operator doesn't have those.

The second example is missing error checking. So the real code isn't that nice.

My point is that C shouldn't look like Python. Small amount of functionality should be written unambiguously and take a lot of space if necessary. Because of the nature of C, it needs a lot boilerplate, and will take a lot of screen space anyway, but that is not a problem, as we are not coding on paper.

How would you use the ternary operator there? Like this?

    image ? foo_release(image) : 0;

Yes. And cast that 0 to (void), so the compiler won't complain over an unused expression.

Any C programmer will recognize what (void)0 means, do nothing.

Sure, but I find it strange to use the ternary operator and discarding the value. It's not wrong, but I think most C programmers prefer a simple if statement.

I was giving an replacement for the if statement presented above, where both expressions are on the same line.

I'm closing in on year 25 of programming in c, and I still refuse to use the ternary operator.

It would be really nice if instead of downvoting, someone could provide a counterargument, or comment on parts the disagree.

I downvoted it to indicate that I don't agree with your preference. I rarely downvote for disagreement, but this was a case where I thought it worthwhile to offer the feedback that others strongly do not share your view. My equally strongly felt subjective personal belief is that single line if statements without braces are fine, but that once a second line is required braces should be mandatory.

I find the single line "if" statement without braces to be clearer and simpler than ternary with a (void) expression, and don't think the downsides are significant. It breaks if you were to add another statement after the semicolon on the same line, but I think that should almost always be avoided anyway.

I do find it interesting that others prefer two-line without braces over the single line approach. I find this one to be more dangerous than the single line. Possibly because with line-oriented debuggers it can be hard to set the right breakpoint?

It's quite possible that others are downvoting because they think you are trolling, and that no one would actually believe the ternary operator to be clearer. I wondered also about your defense of Allman braces, which I didn't downvote because I think it's a good example of how different the others's views can be on what seems obvious. While I think (some of) your views are in the (very small) minority, please keep posting them!

Braces don't prevent you from using single-line if:

  if (ptr) { call_oldschool_thingy(ptr); }

many people use specific styles for where to place { and } (e.g in a line by itself) so this might look 'ugly'

Nothing prevents them from not using single-line ifs.

Meshes perfectly fine with one true brace style

Was just discussing this at work. A good compromise would be to allow statements outside of of block-parens only when they are on the same line as the control statement.

IMHO there should be a -W to enforce this so those of us with -Werror on can catch it and burn it with fire.

Here at Google the style guide requires block-parens always.

In the project I work on our style also requires block-parens. Sometimes it feels unnecessary, but it does make up in terms of consistency: it's easy to glance through the code and inspect the conditions only.

I got used to this quite a bit and these days if I write code outside the project I work on, I d sometimes omit the block-parens, but I always add extra indentation to ensure the condition is visually well-separated, e.g.:

  if (condition1 || condition2)    execute_foo1(with_bar);
  if (condition3)                  exectute_foo2();

Both Rust and Golang have optional parens and mandatory braces, so for the same number of keystrokes one would have something more like:

    if ptr { call_oldschool_thingy(ptr) }

(Keystroke count isn't an important metric for me, but apparently very important to some people.)

I love this style. Braceless style always breaks when refactoring or adding/removing debug logging – so it makes sense to make the braces mandatory. And then, because you have a delimiting if and the starting brace, you don't need to have the parenthesis. Great!

Now if only Rust supported the elision of ; ... (Yeah, I know that they have a reason to not do that, but I don't buy it.)

Does this outweigh the disadvantages? I don't think so.

I've never met a C or C++ debugger that handled this case nicely, unfortunately. Totally ridiculous, of course, because they could! They just never seem to.

Meeting snippets of code like that when debugging is a constant source of frustration for me, because I have to stop what I was doing, edit the code, rebuild, then get back to where I was. Particularly galling when the code in question is in a commonly-included header, and the subsequent build takes several minutes, and I was on a particularly productive-looking trail.

(The last project I worked on solved (?) this problem by performing so shamefully poorly in an unoptimised build that it effectively didn't work. So you had to debug the optimised build. And so single-stepping at the source level just didn't work properly anyway.)

  >I've never met a C or C++ debugger that handled this case nicely, unfortunately.

Apparently you never use Visual Studio. It can highlight a portion of the statement on each step (at least it used to)

Last I looked, this only worked for C# (possibly all CLR-based languages...) - but native C/C++ breakpoints were addressed by file and line only.

That's likely because with C#, the debugger can use deoptimization and on-stack replacement to get expressions evaluated in the order they happen in the code, as long as the instruction re-ordering happens only in the bytecode to native code compilation. Even though the debugging symbols just have line numbers, the debugger could potentially regenerate they bytecode to figure out which operations correspond to which expressions.

In C++ optimized builds, you might be jumping back and forth between lines due to the compiler re-ordering instructions, and there isn't an easy way to get back code that's in-order.

I think that gofmt is particularly innovative, in the sense that it acknowledges that formatting is integral part of the language.

In the sense that a programming language is not only made to be parsed by a computer, but also read back by a human.

Yes, and that the nuances of how things are formatted are really not that important, but rather having a standard which provides a consistent reading experience is the important aspect.

I agree completely with this. After fighting over code formatting for so long (often starting those fights myself), I have thankfully come to realize that what format you use almost never matters, only that you use some standardized format.

And in the case of go that standard is language wide, not just project, team, or company wide.

Except the go standard is awful, so a language-wide formatting requirement with bad defaults makes the language basically unusable (since programs are made to be read, not run).

I think that's one of the best contributions golang has made to the programming world. The idea that the language itself should be easy and unambiguous for the compiler and the human to parse. And that a single formatting standard makes life easier for everyone.

If we ever move beyond using text files for storing code, we could eliminate formatting differences entirely... but efforts in that direction have run into many issues in the past.

> I think that gofmt is particularly innovative, in the sense that it acknowledges that formatting is integral part of the language.

gofmt is still optional. Python significant white spaces are not.

I thought that the go compiler would throw an error of your code is not in the format produced by gofmt. You don't need to use the tool, but you need to have your code in the same format that the tool would produce.

> I thought that the go compiler would throw an error of your code is not in the format produced by gofmt.

This is not the case.

Nope. The gofmt-contract is a purely social one. But it's one held by nearly the entire go community which probably gave you that impression for good reason.

Wow, true. The first thing that I read was how to set up emacs to run gofmt every time you save a src file. After that, I never questioned it.

Actually for C there has always been GNU indent, which serves a similar function as gofmt.

Rust does something similar. It doesn't allow for brace-less clauses and it also bitches and moans at you about 'miss-using' camelCase, snake_case, etc. Which is great because it means that by default all project will follow a similar mark-up.

We also have 'rustfmt' in development, which goes even further than the built-in lints.

  >I'm glad Python (with semantic whitespace)... solve this problem.

Python solved it....then added the problem of making things you can't even see semantically significant. If you write Python, it's helpful to have an editor that makes the difference between tabs and spaces visible....

Since Python 3 the interpreter will halt when detecting mixed whitespace, forcing the programmer to deal with it.

https://www.python.org/dev/peps/pep-0008/#tabs-or-spaces

This is only a problem in Python 2.

> it's helpful to have an editor that makes the difference between tabs and spaces visible....

I do this for any and every language. Mixing tabs and spaces always sucks.

> one of the biggest anti-features in C

Please. I'm as big a C apologist as you'll find and even I can think of six or nine thing objectively worse about the language.

This issue is merely a great bike shed because it allows all of us rabble to join in on a discussion about "compiler" technology.

Look: it's a good warning. People should clearly use it. It probably should have been written long ago, and probably would have if our editors hadn't been enforcing this since time immemorial.

And now we've got the warning, we can go back to leaving the braces out more often ;)

Good point, there's plenty of bad stuff about C.

But optional braces are perhaps the most pointless anti-feature in C. The only benefit is a minor (and subjective) improvement in aesthetics.

Your point being that... aesthetics in a technology (a "programming language") whose whole existence derives from making computer programs easier to write, read and reason about is... unimportant?

There's probably a word that the parent was looking for; it wasn't quite aesthetics, at least as you've defined it.

Sometimes you can write code that looks "prettier", at the expensive of the ability to read and reason about it. Optional braces are frequently used for this effect. Yes, optional braces allow you to fit more code on the screen with less noise—and therefore could be thought to enhance some kind of readability. But at the same time, they require you to think harder about the grouping of the code around them. Unless you've got a reformatting linter, you could have code like this:

    if(foo)
      bar; baz;
    quux;

and optional braces require an extra subroutine to be constantly running in your head, looking for those mis-arranged `baz`es that will get executed anyway. That cognitive load detracts from your ability to read and reason about the code.

(What'd be really interesting, in my opinion, would be to make C's braceless blocks, and only braceless blocks, have semantic-whitespace. Then the above would be able to be easily reasoned through: `baz` is part of the conditional, because it's on the same line. But this breaks a lot of rather old and inflexible assumptions about how C is parsed, that macro-writers et al depend on.)

I don't find something like the below frustrating really

if (a)

...

else

...

But for some reason I find the below _very_ frustrating. It feels misleading and I find it to be very, very ugly.

if (a)

...

else {

    /* Multi line block */

}

I've actually never seen anyone do either of those.

I've seen

if (a) thing;

but never with an else, and certainly never with an else that has a multi-line block. That's definitely somethign I'd call out in a CR.

I call it out in CRs when I see it and I've been fixing any of these I come across in code at work.

I'd rather a compiler forces me to put braces around everything than let people have the opportunity to do something like the latter from my original comment.

Why is that an issue? You would use something like that extensively in Go or JS for example, where you need to perform a check on the returned (Go) or passed (JS callback) error value.

The else is executed if there is no error, in my style at least, so naturally it will contain more logic.

I just have a problem with the mixing of braces on an if/else chain I guess. I don't know how to explain it clearly, it just rubs me the wrong way?

I agree. I've trained myself to religiously apply curly braces to all blocks, but it'd be more reassuring to know there was a compiler check for it. I do have linters set up to check for it, but I just wish it were default.

This is also one of the reasons I like Racket: there's no such thing as ambiguous block delimitation. Also, everything-is-an-expression is very useful.

Haskell's solution to this problem is also excellent and needs to be better known and copied into newer languages. The language has curly braces around the bodies of 'let', 'case', 'where', and 'do' constructs, and also the bodies of modules, and semicolons between the constituents of those constructs -- but when the compiler demands a '{' and doesn't see one, it uses indentation to insert them in the obvious places. If your indentation is incorrect, you're likely to get an implicit ';' or '}' put in the wrong place, and you go fix it.

The scheme works so well that people will often go months into their Haskell education before they encounter code with explicit braces and semicolons.

I've actually never seen production code with braces or semicolons. The only place it's semi-common is when people write complicated expressions and need to put them on one line for the REPL (GHCi).

I think another use-case for non-layout version is generated code. If you happen to generate Haskell code, it's easier to keep the syntax correct with the explicit notation rather than the implicit whitespace version.

I don't like Python forcing you to use a particular identation. And in my opinion optional braces are not an anti-feature, "with great powers come great responsibilities": like any other tool it's up to who is using it to use it in the right way.

I would disagree that optional braces are a "great power." I feel like it's a really nothing feature.

> like any other tool it's up to who is using it to use it in the right way.

And mandatory seat belt use is foolish, bike helmets are for wimps, and blade guards on saws are a waste of space and money.

Just use those tools responsibly :P

> Making braces optional in single-statement if/else/while/for clauses is one of the biggest anti-features in C.

What's frustrating is that K&R still has a lot of example code like that as well. It would be wonderful to see a 3rd edition even if it do nothing but change its code to use braces.

The argument for significant white space in Python goes as follows:

You need indentation for humans to understand the structure. Why do you also need braces for the parser to understand the structure when the parser can use the same information that your eyes use? You therefore avoid the possibility of the two signals contradicting each other.

Because, by having both, you can have your editor auto-indent and then you get a visual indication of where implementation does not equal intention.

If you don't have braces like in Python, then you always have the same visual feedback don't you? If you were to add braces to Python and auto-indent you'd end up with the same look as you would with python with only whitespace, only now there are superfluous brackets.

Rearranging code, refactoring, moving blocks, inserting conditionals etc would be easier and could be auto-indented if there were braces. Instead you need to carefully ensure that everything aligns correctly with the intended meaning at the new location.

Also, a closing brace is a nice signal to the editor that it's time to "outdent".

Finally, people seem to forget that python already has an opening brace, except it's spelled ':'. So why no closing brace to match? This seems inconsistent to me.

You don't need advanced auto-indenting when the existing code is already correctly indented. Any decent programmers editor will automatically change the starting indentation point when you paste a block.

Your other points could be debated but really, is coding so keyboard-limited that saving a single keystroke is a major issue? Python has focused on comprehensibility, which I think is the right balance given how frequently people need to understand code versus write it.

If you move a block of code to a place just after e.g. a loop, it isn't clear whether it should be inside or outside the loop, your editor cannot guess this. You still need to tell it at what level the new code should be inserted. And if the end of the loop happens to be nested further, the ambiguity is exacerbated.

I switch between C and Python quite frequently. With C, I just type the code, and let my editor manage the indentation, which it can do perfectly without any help from me. With Python, I find that I spend a lot more time thinking about the formatting itself, especially at the end of block, because it's up to me to get it right.

I fail to see how the _lack_ of a closing brace _improves_ comprehensibility. What Python has done is removed a helpful indicator, and put more of the responsibility on the programmer.

I'm happy to type that extra character in C, because it helps lower my cognitive workload so I can focus more on the problem I'm trying to solve. The closing brace doesn't mean anarchy, or make the code harder to understand later.

> If you move a block of code to a place just after e.g. a loop, it isn't clear whether it should be inside or outside the loop, your editor cannot guess this. You still need to tell it at what level the new code should be inserted. And if the end of the loop happens to be nested further, the ambiguity is exacerbated.

1. Move the cursor to wherever you want to put the block 2. Hit paste 3. Your editor adjusts the indentation so the entire pasted block is inserted as if you just keyed it in

It's simple and reliable and as a bonus it's portable across any language where the code is indented correctly.

> The closing brace doesn't mean anarchy, or make the code harder to understand later.

It's true that well-written C code can be almost exactly the same but the difference is that the world is full of sloppy C code where someone hammered out a bunch of changes, decided it was too much work to format it so the visual display matched the actual parsed structure, and left a trap for the next developer who touches that code. Yes, hopefully they'll review & test carefully but, as with e.g. memory management, we have decades of proof that depending on programmers to consistently follow desirable practice is a losing battle unless it's enforced by tools.

The point isn't that braces are bad and that whitespace is good but that Python will refuse to execute one class of sloppy code. Imagine if GCC made this flag mandatory or Clang refused to compile anything which didn't pass cleanly through clang-tidy – the benefit wouldn't be due to the braces but from the fact that one category of error would simply no longer be possible and every C developer on the planet would spend less time on cosmetic differences when reading other people's code.

I hear what you are saying, and really I don't mind that the formatting is required by the language, I'm not arguing against that.

But... we could have had both! Absolutely require the formatting, but also keep the magic symbols that help your tools help you!

Agreed, but with braces, i can press % over a '{' in vi and find the matching }

There is an argument that if you can't scan the code with your eye and spot the } then your code needs refactoring to make it easier to read.

Agreed, but i m talking about moving the cursor there.

That's easy to say until you're dealing with 2 million lines of code like that.

That's what plugins are for :P

Lack for jumping to the end of the current indent block is a missing-feature/bug in vi then. There is nothing technologically harder about finding the end of a block based on indentation than based on braces.

When I think about this I think about streaming protocols, you have two types those that totally lose the thread when there is an error and those that don't. And there is an issue of the typical hamming distance between valid code sequences.

I'm on the side of having to reflexively type an extra character or two to gain some parser redundancy and error detection. I've also read a few people talking about auto formatting tools and seems like they need all the help they can get.

Allowing explicit braces fixes deficiencies in python's grammar. For example, multi-line lambdas aren't currently possible in python.

> Allowing explicit braces fixes deficiencies in python's grammar. For example, multi-line lambdas aren't currently possible in python.

I'd clarify that to "lambdas can't contain statements". Lambdas can contain expressions, which can be nested arbitrarily, have side-effects (e.g. tuples guarantee left-to-right evaluation of elements), and be spread across multiple lines. It's not exactly pretty though ;)

I wrote about this a few years ago at http://programmers.stackexchange.com/questions/99243/why-doe... and slightly more obtusely at http://chriswarbo.net/blog/2012-11-17-anonymous_closures_in_...

This seems like it could have been avoided by people using braces around every block.

Omitting braces in this case leads to a lot of problems.

This warning is a practical solution to a real problem (of which the famous "goto fail" is an example).

Using braces everywhere is definitely good style, but it isn't a practical solution because there's a ton of existing C/C++ code that doesn't use braces.

Wouldn't it make sense to update that as you change the code? If you edit an if statement, you should add the braces in.

It would make sure that no new errors are introduced due to this.

That's a good policy for code reviews, but again it's hard to do it correctly in a tool.

The compiler would need to require braces in new code, but allow brace-free style in old code; you'd need to integrate it with the source control system so it knows what code is new. Doable, but tricky.

I've found improving style checks file-by-file to be a reasonable approach:

- all new files are checked

- old files are put in a whitelist for the style checker, the submitter is supposed to remove the file from the whitelist when signifcant changes are made

I'm very glad to see that I'm not the only one who does this. I wanted to see if others felt that this process worked.

It is good to hear others are doing it.

Ever heard of clang-format?

http://clang.llvm.org/docs/ClangFormat.html

Of course, that's the (silent) assumption. But humans sometimes fail to do this, which is why the warning is helpful. Very nice to see this in GCC.

Automatic brace insertion program with helper scripts: https://matt.sh/howto-c#_formatting

It is never an error to insert braces, and the rules for inserting braces are fully deterministic in C.

I'm in favor of braces around everything, but to be honest even I will occasionally use the indent if I only expect one action. e.g.

  if (true)
    foo();
  else
    bar();

This looks prettier to my eyes than

  if(true){
    foo();
  } else {
    bar();
  }

However, I might not be the last person to touch the code. My coworker might come later and add:

  if (true)
    foo();   
  else
    bar();
    baz();

And hence the indent problem.

I always opt out of the "no braces needed thing," it is my feeling that you should stick to the more "all around" method of writing something so you never need to switch back and forth..

Even if you ignore safety, braces are still better because if need to edit the code later, you won't need to change as many lines, so the source control history will be cleaner. (Similar to adding a trailing "," to lists that don't strictly need it.)

Btw, I suppose you wanted to add baz() after bar(). Adding it after foo(), as you did, will not compile.

Thanks, I fixed it.

Why should the inability to write basic C preclude me from being allowed to pontificate about brace style like the greats?

Prettier but potentially dangerous? That's the core of the problem, I believe.

I agree. I don't care much about if something looks good, but rather if it works well.

Everything is potentially dangerous. I mean, pointers, anyone? So many things that can go wrong with those.

And besides, code is for humans to read and only incidentally for computers to execute. Might as well optimize for prettiness.

This is the poster case for "code is for humans to read, so it shouldn't be easy to mis-read." Braceless misindented ifs are everything except easy to read.

I agree. But your last example won't compile anyway. Though this would:

  if (true)
    foo();
  else
    bar();
    baz();

But I have to admit if you ever actually do that, you have failed to fundamentally understand how your code is executed. No braces means one statement. Period.

In these cases languages which are auto formatted (C#, go-fmt, etc) likely have an advantage where these problems stick out more obviously.

But I have to admit if you ever actually do that, you have failed to fundamentally understand how your code is executed

I would say I have a pretty good understanding of how C & my code in general works, I'm currently trying to create a threadpool with support for co-routines/yielding to other threads in userspace. Nothing super fancy, but not something you can do without understanding how code is actually executed ;)

But I still lost an hours work last week because I hadn't put a brace after an if statement and when I came back to it, I didn't notice the lack of braces and put an extra statement behind it. This is why I usually stick to putting braces around everything.

In cases where I have only one statement I often drop the braces, but I also drop the newline.

  if (true) foo();
  else bar();

Not much room for a confused baz() here, or so I hope.

This is actually a worse solution. When speed reading code having control statements on their own lines is far more beneficial as you can at a glance, using the indentation, easily see what the flow of execution is.

You might find it worse, I don't. You might not be able to parse this quickly, I am.

No holy wars about style please! I was just offering an alternative that works well for me.

How about this one

(true) ? foo() : bar();

They will send the Inquisition after you for that one! :)

Well I'm Spanish after all ;)

Personally I'm a fan of braces everywhere (though I prefer Allman style), but the one area where I've given up is precondition checks at the start of a function.

Any style with braces looks like a cluttered mess compared to

    if (foo)
      throw ...
    if (bar)
      throw ...
    if (baz)
      throw ...

But I strictly limit that to the beginning of a function, and only `if (x) [throw|return] y;`

> Personally I'm a fan of braces everywhere (though I prefer Allman style)

you monster.

So much wasted vertical space. We use that at work and I am not at all a fan.

That's why smart programmers use 2x monitors in portrait mode.

Try it once. You'll never go back.

I think I don't really have that bar() baz() problem because I use blank lines to separate blocks of code from each other. And also because as a heathen I use Allman style not K&R.

Suppose Debian decides to hire you as their new release maintainer. They want to ensure that bugs such as the OpenSSL one mentioned in the article never happen again. What actions would you take before next month's release to attempt to catch it?

I see from your comment that you might hire a team of developers to go through a critical path of important C/C++ packages checking for style violations. Any code found missing braces will have a patch submitted upstream to correct the errant style. Any package that declines the style changes would be removed from Debian. Any developer you hire that misses style violations will be removed from the team.

Another approach might be: Turn on the warning mentioned in the article and manually review any cases that come up. That still might generate a lot of cases, but it at least seems possible.

Is there another actionable interpretation of your comment that I'm missing?

No. That is not what I mean. If you look at my response to iainmerric you will see that I don't feel you should change code that is working.

This type of bug is largely introduced from someone who is going in and changing a section of code. My philosophy is that if you are editing a section of code, you should update the braces in that section along with your current patch. Over time you see a larger and larger drop in these kinds of bugs as people always, on my team due to the habit of fixing braces and a matching drop from outside contributes due to a larger and larger portion of the code matching the style guide.

That seems like a somewhat different case, though. Your approach will help prevent new bugs like this from being introduced, but won't help you against existing examples of it in areas you're not currently working on. And just because it's been around a long time doesn't mean it's "working." Goto fail showed that the bugs can be subtle and easily missed for a long time.

Yes, but from what I understand this bug is more commonly introduced in code that is being updated.

Notice that I said "avoided", not "fixed". That is because I am rather focused on making code that is maintainable.

I posted this original comment to have a dialogue about something I include in my programming practices. I wanted to see how other people viewed this as a development technique.

You didn't say it explicitly, but your original comment came off more like "This option is unnecessary and should be removed from gcc, since we can simply enforce a style of using curly braces everywhere"

C doesn't do this, but I like this feature from Perl for single-statement checks:

    return if is_red($traffic_light);

    return unless is_green($traffic_light);

In C, I usually put the early returns on their own line without curly braces, but place them all together at the start of the function to make it easier to spot inconsistencies.

I love that feature of Perl, and to be explicitly clear to those that are unfamiliar with it, Perl does not allow unbraced single line if's in the normal sense (pre-conditional ifs), and does not allow braced post-conditional ifs. For example, these are valid:

    if ( $foo ) {
      bar();
      baa();
    }
    quux() if $foo;

And these are not:

    if ( $foo ) bar();
    { bar(); baz(); } if $foo;

This is the only item that I disagree with the Linux coding style ("Do not unnecessarily use braces where a single statement will do.")

There is a lot to dislike in the Linux coding style... and yes, this is one.

People will make mistakes. Policy does not prevent that. Tools do.

Not in every case. I'd say that policy does help in some cases and tools do in others.

What I often see is people just ignore the output from tooling and commit anyway.

No method of prevention is perfect, but everything you do can help to increase stability in projects.

A policy of "never ignore this warning" has a much better chance of being followed and removing the bugs than a policy of "always put brace on code blocks".

Yes, both are policy, but they are different kinds.

Of course, banishing single line blocks at the compiler would be infallible, but it's also not viable.

Tools won't prevent it without a policy to enforce. It's still ultimately up to us to determine the rules the tools follow.

Extra braces add visual clutter and reduce readability, especially when they mean a function no longer fits on one screen. So if there's a way to eliminate that kind of bug without having to add more braces then I'm all in favour of it.

(Personally my preference would be a linter that automatically runs on checkin, and refuses commits that do not conform to the style guide)

> add visual clutter and reduce readability

one person's clutter is another person's markers. I wouldn't agree they reduce readability

> when they mean a function no longer fits on one screen

What screen size? What resolution? What ide/editor window size? What font face & size? With or without soft line wrapping?

Honestly arguments about how "pretty" code is are fucking ridiculous. Despite what someone said, the purpose of source code is not to be "read" with "running" as a secondary task. That literally only applies to code written purely for educational purposes.

The purpose of code is to achieve the goals of the software as efficiently as possible. Efficiency is not just about speed - a fast but unreliable program is not efficient.

> What screen size? What resolution? What ide/editor window size? What font face & size? With or without soft line wrapping?

Does it matter? There will be a point at which an extra line makes the difference. And particularly with modern screen shapes, vertical space is at much more of a premium than horizontal space.

> Honestly arguments about how "pretty" code is are fucking ridiculous. Despite what someone said, the purpose of source code is not to be "read" with "running" as a secondary task. That literally only applies to code written purely for educational purposes. > The purpose of code is to achieve the goals of the software as efficiently as possible. Efficiency is not just about speed - a fast but unreliable program is not efficient.

The ability to understand software is vital to real-world usefulness though. Requirements change all the time, and so the ability to make changes to software is vital - and to effect desired changes to code you must first understand it.

> And particularly with modern screen shapes, vertical space is at much more of a premium than horizontal space.

What? Modern 10:16 screens give more lines than 3:4. 900x1440 gets like 70 lines with 100 columns at a decent font size, better than around 60 lines with 110 columns with 960x1280.

Good luck using your laptop vertically.

As I said in the other thread I'm not talking about readability.

The exact term used was pretty.

> There will be a point at which an extra line makes the difference

So why not remove all blank lines too. They're less important for reducing issues that single line/braceless flow control blocks can cause.

> So why not remove all blank lines too. They're less important for reducing issues that single line/braceless flow control blocks can cause.

By and large I do. But I don't think what you say is actually true. Given the choice between:

    stepa1
    if(something)
      stepa2
    stepa3

    stepb1
    stepb2
    stepb3

and

    stepa1
    if(something) {
      stepa2
    }
    stepa3
    stepb1
    stepb2
    stepb3

(same number of lines), I think the former is often more readable - stepa2 is visually separated in either case.

If the code can't be read, then it really doesn't matter how it runs because no one will understand it well. Trust me. Some of the code I work with has this problem. I fix it when I can, but there's just too much to fix it all.

The only code where readability isn't as important as functionality is finished code, and we all know that code is never finished.

I didn't say readability doesn't matter.

I'm talking about people who skip braces on single statements, skip semicolons in JavaScript, etc because it "looks prettier" without them

I remember doing this mistake back in college. Was required to program and demo the B-Tree backend for a mini database. Things were very well tested for tons of random sets of data but while waiting for the my turn to demo, added one line of debug print on an "if" statement which caused the now dangling statement to be always executed. This caused a subtle bug that the professor caught in his testing. Sucked because I had everything correct up to that point. Learned my lesson about last might changes and coding style at that point.

We have all learned that lesson one way or another. It's never fun.

I've done this for as many years as I can remember. Unfortunately, almost no one else I work with does.

Semi-related: What do people think about if-else-if... chains vs nested simple if-else blocks? I have seen many cases on the job where someone writes a complex if-else-if chain and then an oversight in their logic WRT the dependencies between conditions causes the wrong branch to be taken. I prefer the latter style of the ones I've put below, especially when the conditions are more complex. For me, it makes it easier to mentally picture the control flow.

    if(condition_a && condition_b){
      do_thing_a();
    } else if(condition_b){
      do_thing_b();
    } else {
      do_something_else();
    }

    if(condition_a){
      if(condition_b){
        do_thing_a();
      } else {
        do_thing_b();
      }
    } else {
      do_something_else();
    }

Furthermore, I prefer functional languages where if-then-else is an expression with a mandatory else (or doing control flow via pattern matching with enforced exhaustiveness like you can get with GHC). I don't like surprises.

I don't think there's a general statement to be made about this. I have no problem reading both examples and actually prefer the first. But it depends on the what the specific logic is - some conditions are easy to understand, despite their size, others are small but subtly complex. I certainly wouldn't want to mandate writing code like your second example without considering the use case first - YMMV.

Second example is very readable using Allman style:

    if( condition_a )
    {
         if( condition_b )
         {
             do_thing_a();
         } 
         else 
         {
             do_thing_b();
         }
    } 
    else 
    {
         do_something_else();
    }

Editor space is free, why not use it.

Horizontal space is free, vertical space is not.

The more lines that are visible on your screen, the less you have to keep in your working memory. Human memory is fragile, so you really don't want to rely on it.

I can only fit 51 lines vertically (damn widescreen laptop) so that one snippet fills a good 1/3 of my screen.

Personally I'd write that as

    if (condition_a) {
        if (condition_b) do_thing_a();
        else do_thing_b();
    }
    else {
        do_something_else();
    }

The more lines that are visible on your screen, the less you have to keep in your working memory.

Sure if you only saw a few lines at a time, then this might be a problem. But you can see 51 on a laptop. This is enough for almost all cases. And you can scroll if you need to look up something. If you stumble upon a case of multiple if statement that span several screens then no style is going to help you see it in full. In that rare case you might see a little more, at the expense of all code ever becoming less readable (if we assume Allman is more readable for the sake of this argument of course).

If for some reason your code has more than, let's say >50% cases where something spans multiple screens and needs to be seen as a whole (code which should be refactored, but let's ignore that), then you might have an argument to use your style, in all other cases you're doing premature optimization of your coding style, so to speak.

Thinking about it, I'd say the second one is better. I think it parse better in my mind as to what is meant.

However I also always comment nested logic chains with their intent in plain language. I have made bugs in going from intent->complex logic to often, and I found that writing comments at each fork greatly reduces these errors as well as the odds of missing an edge case.

e.g., I'd write "if(a) and not b" in plain English at the nested else statement, possibly followed by a "what & why" comment at the do_thing_b statement.

Kinda late but I want to point out these snippets have different behaviors:

    if A and B:
        ..
    elif B:
        ..

    if A and B:
       ..
    elif A and not B:
       ..

I prefer flattening because enumeration makes it obvious what's different between the branches despite the code duplication.

For example, three boolean conditions has 2^3 = 8 possible combinations. When nested this complexity is hidden but obviously apparent as a code smell when flattened.

It also echoes Python's ethos that flat is better than nested.

GCC 6 looks awesome, I'm looking forward to it!

https://gcc.gnu.org/gcc-6/changes.html

What a great idea. It's hard to believe no-one ever did this before! (Or did they?)

Well, some IDEs will warn you even during editing - a step ahead of compilation (e.g. IDEA and its kin). The earlier this antipattern is caught, the easier it is to fix.

Why not give to your developers an immediate way to reformat the whole source tree before each commit/pullrequest, using dedicated tools, like uncrustify, bcpp or AStyle? Just store the configuration file in the repository, hook the reformat pass to the beginning of your build, and you're done.

We've been doing this at work for several years ; and we found that this solved nearly every style-related issue (diffs, arguments over which code is 'prettier', artificial merge conflicts). It turns out the style becomes a lot less important issue once you can rely on a tool to apply it for you. And it also solves the misleading indentation issue (probably by preventing it to happen in the first place by causing a merge conflict).

In theory that works fine, but in practice, I find every tool leaves little bits of cruft laying around:

// comments that get formatted correctly but then // the // wrapping // isn't // merged // into // one // line

On the other hand

// this could // be a // tabular comment

So you'd need everyone to use the same tool and you'd still need to go back and fix things periodically. To be fair, it might still be a net win.

I wonder if it would be feasible to add a -allow-python-style option, which would allow the use of some Python syntax in C++, like significant whitespace or `if a:` instead of `if (a)`

Cython is already something sort of like this.

> it’s been finding real world bugs

One more case to support -Werror.

-Werror holds things back, because it makes the gcc maintainers hesitant to add more warnings on grounds of "it will break old code that uses -Werror", and in fact I was surprised to see that this warning is going into -Wall.

I use -Werror and I strongly hope maintainers keep adding new warnings. The cleaner the code, the better! If it is really necessary, you can disable specific warnings with pragmas.

Just add -Wno-error=something if you don't want to fix your code for a new warning.

This is so useful, i wonder why it wasnt there before. Especially when copy-pasting code with different indentation levels / tabs etc.

Nice. These sorts of warnings are why it's worth investing the extra effort to enable -Werror in your codebase if you can.

No, I've never seen a real use case for -Werror.

What's the difference if compilation stops at the warning? You'll (or your team, or whoever) fix it anyway, and if you won't, you have a people problem, that must be solved at the policy (or HR) level.

Solving people problems at the tooling level is a certain way to get unintended consequences and alienate the good people that weren't part of the problem. Of course, you can get some tooling to support your people, but tools to police them are worth less than zero.

Anyway, unrelated to that, I do like to live warnings on code that is not ready to production. It's an easy (effortless in fact) way to make sure it'll be fixed before deploying.

-Werror is a cheap unavoidable automatic code review tool that limits the blast radius of what you call "people problems". It slows good programmers down a little but it can stop the dangerously bad ones in their tracks.

In a large build, people will not notice warnings scrolling past. -Werror gives everyone the people of mind that you will notice warnings.

The problem with -Werror in open source projects is that it can produce errors for users building your code on different compilers or compiler versions than the one you originally developed it for.

As compilers change they can end up adding new warnings to the defaults, or for or example -Wall, or -Weverything. And different compilers might throw different warnings on the same code with the same options involved. Using -Werror will force an error, which can end up terminating the build for users, even when the code compiles cleanly on your version of your compiler.

So -Werror is fine if you are shipping binaries, but if you are shipping source code, it's probably a good idea to not use it in the public build system.

You'd think a stand-alone program could take care of this quite nicely? No need to clutter the compiler itself.

It's hard enough to get people to turn on warnings in the first place.

I have set up stand-alone static analyzers a couple of times, and it can be a giant hassle to get them working correctly. It is especially bad with complicated build systems for embedded applications.

To parse a C file correctly a tools needs to know the exact set of include path and defines you passed to the compiler. Then it needs to go read the whole tree of header files and preprocess everything. It needs to know where your standard header files are (different from the system header file when cross compiling). It needs to know about your compiler's built-in defines. It may also choke on any C extensions that your code (or any header file) is using.

In this particular case it may be enough to parse the file with some regexes but I wouldn't trust it; people do some crazy things with C macros.

Does GCC or clang have a warning for the following?

  if (foo);
  {
      bar();
  }

GCC warns with -Wextra (or -Wempty-body) and clang warns by default.

Interesting. I would have assumed that this kind of check should be done by static analysers.

Compiler warnings are static analyzers.

Reply on news.ycombinator.com