[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added a comment.

In D137181#3914292 <https://reviews.llvm.org/D137181#3914292>, @owenpan wrote:

> In D137181#3914100 <https://reviews.llvm.org/D137181#3914100>, @goldstein.w.n wrote:
>
>> Err I'm not sure that's right. I thought we where going for the C-code should start at the column that the 'd' in define is.
>> So it would be:
>>
>>   #define X       \
>>    switch (x) {   \
>>        case 0:    \
>>            break; \
>>        default:   \
>>            break; \
>>    }
>
> That doesn't make sense IMO. Here is an example:
>
>   $ cat foo.cpp
>   #if X
>   #define FOO \
>       if (a)  \
>           b;
>   #endif
>   $ clang-format -style="{BasedOnStyle: Chromium, IndentWidth: 4}" foo.cpp > out1
>   $ diff foo.cpp out1
>   $ clang-format -style="{BasedOnStyle: Chromium, IndentWidth: 4, PPIndentWidth: 1}" foo.cpp > out2
>
> Now what should out2 look like? Well, it should be no different than out1 (and foo.cpp) because `IndentPPDirectives` is still `None` by default. However, the current patch outputs the following instead:
>
>   #if X
>   #define FOO \
>    if (a)     \
>        b;
>   #endif
>
> So I really think the first line of a macro definition body should be indented `IndentWidth - 1` relative to the letter `d` in `define` no matter what `IndentPPDirectives` and `PPIndentWidth` are set to.

Fair enough. Adding something like:

  if (PPIndentWidth != Style.IndentWidth)
    Indent += Style.IndentWidth - 1;

Works for that and all existing tests pass + added a few tests with varied indentwidth / ppindentwidth.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137181/new/

https://reviews.llvm.org/D137181