Some real-life feedback on -Wmisleading-indentation

[David Brown]

On 11/01/16 08:20, Gerald Pfeifer wrote:
> Compiling Wine with GCC trunk (to become GCC 6) I noticed four
> dozen of warnings triggered by -Wmisleading-indentation.
>
> Some are simply weird formatting, some may be indicative of
> real issues -- and I have started to look into them one by
> one and submitting patches (to Wine).
>
> However, there is a pattern I noticed which, while a bit weird
> in terms of formatting, does not really strike me as something
> -Wmisleading-indentation should warn about:
>
>      VariantInit(&ret);
>      hr = IWinHttpRequest_Invoke(request, ...);
>      ok(hr == DISP_E_UNKNOWNINTERFACE, "error %#x\n", hr);
>
>      VariantInit(&ret);
> if (0) /* crashes */
>      hr = IWinHttpRequest_Invoke(request, ...);
>
>      params.cArgs = 1;
>      hr = IWinHttpRequest_Invoke(request, ...);
>      ok(hr == DISP_E_TYPEMISMATCH, "error %#x\n", hr);
>
>      VariantInit(&arg[2]);
>
> This is from the Wine testsuite, and the if (0) in colum one guards
> one invication of the function under test that would crash (so is
> the equivalent of #if 0...#endif, except that it avoids conditional
> compilation).
>
> Is this a bit unusual?  Definitely?  Does it look like a one of
> those cases a programmer would actually be tempted to misread?
> I don't think so.
>
> What do you think?
>
> Gerald
>

Maybe I am missing something, but yes, this is misleading and confusing, 
and yes, it is weird indentation that I would normally assume is a 
mistake - and I would be very happy to see the compiler warn me about it.

Is there some particularly strong reason why the Wine code has this odd 
indentation?

I would say the best idea is to fix the oddity in the Wine code, not to 
try to weaken gcc's warnings as a workaround.  The gcc warning is to 
spot unusual, misleading and possibly incorrect code - a warning on this 
code is correct.  If gcc waters it down to workaround oddities in Wine, 
then it must surely also water it down for whatever oddities are found 
in other projects - until it becomes pointless.

So my vote (which is worthless, of course - I am a mere observer, but 
one who thinks "-Wmisleading-indentation" is a fantastic idea) is that 
the Wine developers most produce evidence that this code construction is 
necessary, clear, and important to their code, and also that the same 
technique is used in a wide variety of other projects, before it makes 
sense to reduce -Wmisleading-indentation to accommodate them.  Even then 
I would make the variation require an additional flag or option to the 
-Wmisleading-indentation flag.