GCC 6 Will Warn You About Misleading Code Indentations

[haagch]

Those both look truly awful to me for readability and I'm surprised the latter ever got through code review. Maybe bad luck with where diffs start and end?

There's a lot that gets through review. For example this one:
https://cgit.freedesktop.org/mesa/me...94fc40cac5e573
Would have probably been caught by this warning too.
(Was fixed later: https://cgit.freedesktop.org/mesa/me...f8cf6f88338eda)

In the second case here it depends on how the code works. I think it may be just mistakenly indented one level too much, then it would be three blocks

if (params->wm_prog_data)

if (params->src.mt)

if (params->dst.num_samples > 1)

on the same level but only people who understand what the code does can really tell.

[nanonyme]

The second may in fact be a real bug. Without that TODO line the indentation level would be right. Probably worth verifying history

[nanonyme]

The warning works pretty well, actually.

I've seen two of these when compiling mesa:

https://cgit.freedesktop.org/mesa/mesa/tree/src/compiler/glsl/ast_function.cpp?id=742bc53d040632fd98fac8930e4e8749f5c430e2#n1533

https://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/drivers/dri/i965/gen7_blorp.c?id=742bc53d040632fd98fac8930e4e8749f5c430e2#n491

To may layman's eyes they both appear to be harmless, but still misleadingly indented.

Those both look truly awful to me for readability and I'm surprised the latter ever got through code review. Maybe bad luck with where diffs start and end?

[haagch]

The warning works pretty well, actually.

I've seen two of these when compiling mesa:

https://cgit.freedesktop.org/mesa/mesa/tree/src/compiler/glsl/ast_function.cpp?id=742bc53d040632fd98fac8930e4e8749f5c430e2#n1533

https://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/drivers/dri/i965/gen7_blorp.c?id=742bc53d040632fd98fac8930e4e8749f5c430e2#n491

To may layman's eyes they both appear to be harmless, but still misleadingly indented.

[waxhead]

NO. "Debian testing" is just "Debian unstable" two weeks later. It gets the exact same packages, only with a 2 week delay. End-users shouldn't be running testing it is just the worst of two worlds, and only exists as a biproduct of the Debian release procedures. And unstable is not called unstable because the packages are considered unstable, but because the exact SET of packages and package versions is unstable, because the packages are updated all the time (and not with a two week delay), in other words, in Debian terms unstable means rolling. Once upon a time they also had untested packages in unstable, but since the introduction of experimental several years ago, packages have already been through QA and testing before even getting to unstable.

Not quite right... in fact your arguments may indicate that you are a bit ...wait for it.... unstable (sorry but I could not help it)
The package must be in unstable for a minimum of 10, 5 or 2 days and...
The package must be compiled for all architectures
The package must not have release critical bugs
The package must have all dependencies satisfied

Here are the references just in case you missed it :
1. https://www.debian.org/doc/manuals/d...ives#s-testing
2. https://www.debian.org/devel/testing

So your arguments are wrong:
1. Testing is NOT the same as Debian unstable delayed by two weeks
2. Testing is NOT the worst of two worlds, the reason should be obvious by looking at the links above.

The Debian packages go through this process
A new package end up in EITHER experimental or unstable (reference: https://wiki.debian.org/DebianExperimental)

Experimental -> Unstable -> Testing (-> Testing freeze -> Stable)

While packages have been through some testing before reaching unstable, they have been through even more testing and QA before reaching testing so I would argue that running testing is in fact a good thing (but hey, Debian would hardly work if people did not run the unstable distribution).

I think that we can at least agree that over all Debian is a good and stable product regardless if you run 'Unstable' or 'Testing'

[SystemCrasher]

Debian unstable is basically a stable rolling release.

And "stable" vs "rolling" are mutually exclusive. When you bump versions of software all the time... quite many software authors do not really care about compatibility, at least not on major version upgrades. So things can break in fancy ways. "Testing" and "unstable" have their names for a reason, and trying to pretend it is not a case looks really silly.

[oibaf]

NO. "Debian testing" is just "Debian unstable" two weeks later. It gets the exact same packages, only with a 2 week delay. End-users shouldn't be running testing it is just the worst of two worlds, and only exists as a biproduct of the Debian release procedures. And unstable is not called unstable because the packages are considered unstable, but because the exact SET of packages and package versions is unstable, because the packages are updated all the time (and not with a two week delay), in other words, in Debian terms unstable means rolling. Once upon a time they also had untested packages in unstable, but since the introduction of experimental several years ago, packages have already been through QA and testing before even getting to unstable.

NO. For starting it's not 2 weeks, but it depends on the development state, currently it is 5 days. And that is not the only restriction, to get into testing the package must respect some other quality features, for example not having serious bugs reported in those 5 days.

[pal666]

You can already get gcc-6 there.

you can't, gcc 6 doesn't exist yet

[Luke_Wolf]

You can always improve warnings. And in this case a always return false is fixed by adding the missing braces. I on the other hand prefer braces on separate lines and that also introduce problems.

For example a typo like if(whatever); will cause the block to never get executed while if(whatever){; does not matter at all. Not sure if GCC have warning for this , but I sure would like to have that in too

As a believer in braces on separate lines if(whatever); is always brought up as a "problem" but I've never even heard of it happening in practice, meanwhile when dealing with other people's code (my IDE automatically inserts matching parenthesis, braces, and such), switching over to braces on their own line has highlighted and fixed quite a few problems before.

[carewolf]

I use Debian daily and while TESTING can be considered a mostly stable rolling release I would not say the same about unstable, neither would Debian that's why they name it unstable. Testing on the other hand is very stable and have not given me much trouble at all as long as you pay some attention to what happens to your packages during updates!

NO. "Debian testing" is just "Debian unstable" two weeks later. It gets the exact same packages, only with a 2 week delay. End-users shouldn't be running testing it is just the worst of two worlds, and only exists as a biproduct of the Debian release procedures. And unstable is not called unstable because the packages are considered unstable, but because the exact SET of packages and package versions is unstable, because the packages are updated all the time (and not with a two week delay), in other words, in Debian terms unstable means rolling. Once upon a time they also had untested packages in unstable, but since the introduction of experimental several years ago, packages have already been through QA and testing before even getting to unstable.