
GCC 6: -Wmisleading-indentation vs. “goto fail;” - dmalcolm
http://developerblog.redhat.com/2016/02/26/gcc-6-wmisleading-indentation-vs-goto-fail/
======
strommen
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.

~~~
pavlov
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);

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

    
    
      if (ptr) { call_oldschool_thingy(ptr); }

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

~~~
frandroid
Nothing prevents them from not using single-line ifs.

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

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

~~~
monk_e_boy
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
Agreed, but i m talking about moving the cursor there.

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

~~~
AdmiralAsshat
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.

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

~~~
Grue3
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.

~~~
Piskvorrr
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.

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

~~~
dcvuob
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
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();
        }

~~~
dcvuob
_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.

------
Ono-Sendai
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)

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

~~~
Piskvorrr
Well, some _IDE_ s 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.

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

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

------
pif
> it’s been finding real world bugs

One more case to support -Werror.

~~~
GFK_of_xmaspast
-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
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.

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

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

~~~
marcosdumay
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
-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.

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

~~~
TorKlingberg
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
Does GCC or clang have a warning for the following?

    
    
      if (foo);
      {
          bar();
      }

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

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

~~~
jemfinch
Compiler warnings _are_ static analyzers.

