19 ms·

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

199 points by dmalcolm 11y ago

andybak 11y ago
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.
- RHSeeger 11y ago
  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.
  - Coding_Cat 11y ago
    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.
    - nisse72 11y ago
      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.
      - acdha 11y ago
        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.
        nisse72 11y ago
        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.
        acdha 11y ago
        > 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.
- return0 11y ago
  Agreed, but with braces, i can press % over a '{' in vi and find the matching }
  - monk_e_boy 11y ago
    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.
    - return0 11y ago
      Agreed, but i m talking about moving the cursor there.
    - ConceptJunkie 11y ago
      That's easy to say until you're dealing with 2 million lines of code like that.
  - Tyr42 11y ago
    That's what plugins are for :P
  - Coding_Cat 11y ago
    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.
    - Gibbon1 11y ago
      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.
- arohner 11y ago
  Allowing explicit braces fixes deficiencies in python's grammar. For example, multi-line lambdas aren't currently possible in python.
  - chriswarbo 11y ago
    > 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-doesnt-python-allow-multi-line-lambdas/252546#252546 http://programmers.stackexchange.com/questions/99243/why-doe... and slightly more obtusely at http://chriswarbo.net/blog/2012-11-17-anonymous_closures_in_python.html http://chriswarbo.net/blog/2012-11-17-anonymous_closures_in_...
21 11y ago
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.
gravypod 11y ago
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.
- epx 11y ago
  This is the only item that I disagree with the Linux coding style ("Do not unnecessarily use braces where a single statement will do.")
  - TickleSteve 11y ago
    There is a lot to dislike in the Linux coding style... and yes, this is one.
- kllrnohj 11y ago
  People will make mistakes. Policy does not prevent that. Tools do.
  - gravypod 11y ago
    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.
    - marcosdumay 11y ago
      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.
  - kbenson 11y ago
    Tools won't prevent it without a policy to enforce. It's still ultimately up to us to determine the rules the tools follow.
anon4 11y ago
Interesting. I would have assumed that this kind of check should be done by static analysers.
- jemfinch 11y ago
  Compiler warnings are static analyzers.
iainmerrick 11y ago
What a great idea. It's hard to believe no-one ever did this before! (Or did they?)
- Piskvorrr 11y ago
  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.
noamsml 11y ago
Nice. These sorts of warnings are why it's worth investing the extra effort to enable -Werror in your codebase if you can.
- marcosdumay 11y ago
  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.
  - pklausler 11y ago
    -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.
  - TorKlingberg 11y ago
    In a large build, people will not notice warnings scrolling past. -Werror gives everyone the people of mind that you will notice warnings.
- tbirdz 11y ago
  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.
strommen 11y ago
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.
- deweerdt 11y ago
  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.
  - nathanielc 11y ago
    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.
    - asadjb 11y ago
      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.
      - nathanielc 11y ago
        And in the case of go that standard is language wide, not just project, team, or company wide.
        seiji 11y ago
        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).
pif 11y ago
> it’s been finding real world bugs One more case to support -Werror.
- GFK_of_xmaspast 11y ago
  -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.
  - pif 11y ago
    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.
  - TorKlingberg 11y ago
    Just add -Wno-error=something if you don't want to fix your code for a new warning.
- deleted 11y ago
  [deleted]
return0 11y ago
This is so useful, i wonder why it wasnt there before. Especially when copy-pasting code with different indentation levels / tabs etc.
oftenwrong 11y ago
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(); } vs 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.
- jahewson 11y ago
  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.
- dcvuob 11y ago
  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.
  - alextgordon 11y ago
    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(); }
swehner 11y ago
You'd think a stand-alone program could take care of this quite nicely? No need to clutter the compiler itself.
- GFK_of_xmaspast 11y ago
  It's hard enough to get people to turn on warnings in the first place.
- TorKlingberg 11y ago
  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.
splicer 11y ago
Does GCC or clang have a warning for the following? if (foo); { bar(); }
- sesutton 11y ago
  GCC warns with -Wextra (or -Wempty-body) and clang warns by default.
Ono-Sendai 11y ago
GCC 6 looks awesome, I'm looking forward to it! https://gcc.gnu.org/gcc-6/changes.html https://gcc.gnu.org/gcc-6/changes.html
Ace17 11y ago
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).
- humanrebar 11y ago
  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.

199 points by dmalcolm 11y ago

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.

RHSeeger 11y ago
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.
- Coding_Cat 11y ago
  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.
  - nisse72 11y ago
    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.
    - acdha 11y ago
      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.
      - nisse72 11y ago
        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.
        acdha 11y ago
        > 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.
return0 11y ago
Agreed, but with braces, i can press % over a '{' in vi and find the matching }
- monk_e_boy 11y ago
  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.
  - return0 11y ago
    Agreed, but i m talking about moving the cursor there.
  - ConceptJunkie 11y ago
    That's easy to say until you're dealing with 2 million lines of code like that.
- Tyr42 11y ago
  That's what plugins are for :P
- Coding_Cat 11y ago
  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.
  - Gibbon1 11y ago
    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.
arohner 11y ago
Allowing explicit braces fixes deficiencies in python's grammar. For example, multi-line lambdas aren't currently possible in python.
- chriswarbo 11y ago
  > 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-doesnt-python-allow-multi-line-lambdas/252546#252546 http://programmers.stackexchange.com/questions/99243/why-doe... and slightly more obtusely at http://chriswarbo.net/blog/2012-11-17-anonymous_closures_in_python.html http://chriswarbo.net/blog/2012-11-17-anonymous_closures_in_...

21 11y ago

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.

gravypod 11y ago

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.

epx 11y ago
This is the only item that I disagree with the Linux coding style ("Do not unnecessarily use braces where a single statement will do.")
- TickleSteve 11y ago
  There is a lot to dislike in the Linux coding style... and yes, this is one.
kllrnohj 11y ago
People will make mistakes. Policy does not prevent that. Tools do.
- gravypod 11y ago
  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.
  - marcosdumay 11y ago
    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.
- kbenson 11y ago
  Tools won't prevent it without a policy to enforce. It's still ultimately up to us to determine the rules the tools follow.

anon4 11y ago

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

jemfinch 11y ago
Compiler warnings are static analyzers.

iainmerrick 11y ago

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

Piskvorrr 11y ago
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.

noamsml 11y ago

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

marcosdumay 11y ago
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.
- pklausler 11y ago
  -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.
- TorKlingberg 11y ago
  In a large build, people will not notice warnings scrolling past. -Werror gives everyone the people of mind that you will notice warnings.
tbirdz 11y ago
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.

strommen 11y ago

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.

deweerdt 11y ago
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.
- nathanielc 11y ago
  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.
  - asadjb 11y ago
    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.
    - nathanielc 11y ago
      And in the case of go that standard is language wide, not just project, team, or company wide.
      - seiji 11y ago
        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).

pif 11y ago

> it’s been finding real world bugs One more case to support -Werror.

GFK_of_xmaspast 11y ago
-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.
- pif 11y ago
  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.
- TorKlingberg 11y ago
  Just add -Wno-error=something if you don't want to fix your code for a new warning.
deleted 11y ago
[deleted]

return0 11y ago

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

oftenwrong 11y ago

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(); } vs 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.

jahewson 11y ago
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.
dcvuob 11y ago
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.
- alextgordon 11y ago
  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(); }

swehner 11y ago

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

GFK_of_xmaspast 11y ago
It's hard enough to get people to turn on warnings in the first place.
TorKlingberg 11y ago
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.

splicer 11y ago

Does GCC or clang have a warning for the following? if (foo); { bar(); }

sesutton 11y ago
GCC warns with -Wextra (or -Wempty-body) and clang warns by default.

Ono-Sendai 11y ago

GCC 6 looks awesome, I'm looking forward to it! https://gcc.gnu.org/gcc-6/changes.html https://gcc.gnu.org/gcc-6/changes.html

Ace17 11y ago

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).

humanrebar 11y ago
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.