[llvm-dev] -Wmisleading-indentation violations

[Dan Liew via llvm-dev]

On 1 June 2016 at 23:02, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
>> On 2016-Jun-01, at 22:51, Dan Liew <dan at su-root.co.uk> wrote:
>>
>> ```
>> /home/dsl11/dev/llvm-upstream/src/lib/AsmParser/LLParser.cpp: In
>> member function ‘bool llvm::LLParser::ParseTopLevelEntities()’:
>>
>> /home/dsl11/dev/llvm-upstream/src/lib/AsmParser/LLParser.cpp:260:34:
>> warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
>>
>>                                 if (ParseUseListOrderBB()) return true; break;
>>
>>                                 ^~
>>
>> /home/dsl11/dev/llvm-upstream/src/lib/AsmParser/LLParser.cpp:260:74:
>> note: ...this statement, but the latter is misleadingly indented as if
>> it
>>
>> is guarded by the ‘if’
>>
>>                                 if (ParseUseListOrderBB()) return true; break;
>>
>>                                                                         ^~~~~
>> ```
>
> LLParser::ParseTopLevelEntities() hasn't been clang-formatted.  The
> style is one case per line (and every case uses the exact same pattern:
> `if`, `return true`, `break`).

I think GCC is warning about this particular one and not the others
because the ``case`` and the ``if`` are on their own lines. I.e.

```
    case lltok::kw_uselistorder: if (ParseUseListOrder()) return true; break;
    case lltok::kw_uselistorder_bb:
                                 if (ParseUseListOrderBB()) return true; break;
```

GCC does not warn about the ``lltok::kw_uselistorder`` case (and
similarly formatted cases above) but it is warning about the
``lltok::kw_uselistorder_bb`` case.


> I think the density this provides makes the code easier to read than
> clang-format's preference, but maybe a local macro would be better?
> --
>  #define DISPATCH_TOP_LEVEL_ENTITY(token, parser) \
>    case lltok::token:                             \
>      if (parser)                                  \
>        return true;                               \
>      break;
>  DISPATCH_TOP_LEVEL_ENTITY(kw_declare, ParseDeclare())
>  DISPATCH_TOP_LEVEL_ENTITY(kw_define, ParseDefine())
>  ...
>  #undef DISPATCH_TOP_LEVEL_ENTITY
> --

Up to you. Personally I dislike using macros, especially here as the
only reason for doing it would be to suppress a warning.

To suppress this particular warning you could reindent or use braces
for the case that doesn't fit on a single line. You could also use a
``#pragma GCC ...`` to push and pop ignoring
``-Wmisleading-indentation`` in this particular region.

Whatever you find most tasteful I guess.

Dan.