
GCC 6 Will Warn You About Misleading Code Indentations - frostmatthew
https://www.phoronix.com/scan.php?page=news_item&px=GCC6-Misleading-Indentation
======
jimrandomh
> "Specifically, a warning is issued for if, else, while, and for clauses with
> a guarded statement that does not use braces, followed by an unguarded
> statement with the same indentation."

> "The warning is not issued for code involving multiline preprocessor logic
> ... The warning is not issued after a @code{#line} directive, since this
> typically indicates autogenerated code"

Seems like a good thing to have, and pretty conservative about not causing
false positives. In particular, constructs like double-iteration should be
fine:

    
    
        for(y=0; y<ymax; y++)
        for(x=0; x<xmax; x++)
        {
            //...
        }
    

Which I checked because I like this construct and people very rarely think
about it.

~~~
noobie
I apologize for the stupidity of my question but shouldn't that be

    
    
        for(y=0; y<ymax; y++)
        {
        for(x=0; x<xmax; x++)
        {
            //...
        }
        }
    

?

Edit: Oh, I get it, {} are only "mandatory" if we have more than one
instruction, the second for loop gets counted as one instruction and hence the
omitting of {}. Thanks!

~~~
jimrandomh
If I were doing it that way, I wouldn't give the outer blocks' curly braces
their own lines. For example in C#, where pretty much every file has a single
"namespace" block containing a single "class" block, I write it as:

    
    
        namespace SomeNamespace {
        class SomeClass
        {
            //...
        }}
    

Which avoids having shifted the whole file an extra indent level to the right.
But I haven't seen anyone else doing this.

~~~
Avshalom
I'm not writing C# but yeah: AS3 and Java both have the Package{class{ two-
step before you actually write any code. I don't even indent inside class:

    
    
      package{
      class{
      main{
        now we indent
      }
      }}

~~~
threeseed
You don't indent after a package e.g.

[https://github.com/apache/spark/blob/master/core/src/main/ja...](https://github.com/apache/spark/blob/master/core/src/main/java/org/apache/spark/JavaSparkListener.java)

And it's fair enough to indent with a main since it is so infrequent. Ideally
you only have one main entry point.

------
seanwilson
I wish more languages would have what are essentially compiler enforced style
guidelines (e.g. indentation, variable/function naming, file naming). So much
time is wasted debating style, dealing with code review comments about style
and making/using related tools.

~~~
yoz-y
I agree, although I think it should be done by a tool separate from the
compiler. Purely because I'd have a stable compiler first and more code = more
bugs.

------
KindDragon
PVS Studio support similar warning V640
[http://www.viva64.com/en/d/0258/](http://www.viva64.com/en/d/0258/)

------
rwmj
I wonder if it'll be able to make sense of our macro-abusing XML-in-C:
[https://github.com/libguestfs/libguestfs/blob/master/src/lau...](https://github.com/libguestfs/libguestfs/blob/master/src/launch-
libvirt.c#L1054) (Scroll up a couple of pages to see the definitions of the
macros)

------
userbinator
This would certainly be a useful feature, especially for those learning C, but
I wonder whether checking stylistic things should really be in the compiler or
part of a separate tool.

~~~
chris_wot
The genie is out of the bottle already, gcc quite sensibly will warn you if it
sees an assignment operator in an if statement. I'm sure it's saved countless
man hours and pretty awful bugs since it was introduced :-)

~~~
ifdefdebug
I hope it doesn't warn for things like

if(0 != (i = getValue()))

~~~
tspiteri
It doesn't since the assignment is in brackets. You can also write

    
    
        if ((i = getValue()))

------
__david__
Nice, maybe this will finally quiet the wrongheaded opinion that braces should
_always_ be used.

~~~
yoz-y
I do not think that the opinion of mandatory braces is wrong - in a language
that does have braces.

If not for anything else, then for the fact that yi{ will _always_ yank the
current block for me. Omitting braces will ruin this.

------
ArkyBeagle
Seems like simply using astyle would be an improvement over catching this at
compile-time.

~~~
nn3
The point of the warning is to find bugs, not enforce a particular coding
style.

Running astyle would make it worse because you would hide the bug.

~~~
ArkyBeagle
Having used astyle for exactly this, I simply have to disagree. Of course, you
have to look at the code between running astyle and compiling it.

------
hoodoof
So whitespace matters huh?

------
leni536
No more "goto fail;"?

------
jongraehl
clang-format is better

------
jsizz
And thus, every boneheaded project that ships with -Werror enabled will
inflict an excrement tornado of FAIL on its grateful users and packagers.

THIS IS WHY YOU SHOULD NOT USE -Werror. NOT EVER. (And if you can't bring
yourself to do anything about mere warnings, you shouldn't be in software
engineering.)

~~~
tomjakubowski
From the article:

> This new warning isn't enabled by default.

And even if it were, it's a matter of adding -Wno-misleading-indentation to
make builds green again. Not a big deal. And besides, it's likely that fixing
those warnings could be done mechanically. (and it's hardly true that _every_
project that ships with -Werror has such misleading indentation somewhere in
its code)

~~~
cperciva
_And even if it were, it 's a matter of adding -Wno-misleading-indentation to
make builds green again_

How does that help the person who is building the release you shipped six
months ago with a new compiler?

