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

Owen Pan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 17 23:22:16 PST 2022


owenpan added a comment.

In D137181#3935856 <https://reviews.llvm.org/D137181#3935856>, @goldstein.w.n wrote:

> In D137181#3935752 <https://reviews.llvm.org/D137181#3935752>, @owenpan wrote:
>
>> Please mark review comments as done if you have addressed them. Can you also clean up the test cases, removing overlapping/redundant ones, making sure a test case doesn't end with a newline (e.g., line 5380), etc?
>
> Regarding newlines. I have new line before changing the style. Otherwise done. (let me know if you want me to remove those).

Sorry, it's line 5379 and several other places that have an ending newline. (Just search for `\n",`.) You only need to remove them in the new test cases. We can fix the existing ones in a separate NFC patch.

> Regarding removing redundant test cases. I think I've had each of them fail independently at some point (although many when I was doing the previous PPLevel tracking) so I wouldn't call any of them truly redundant.

I know, but IMO some of the failures (e.g. the `switch` statement) during development shouldn't automatically go in. This file already has a humongous size. Maybe do the best you can here.

> I could remove either the `PPDIS_BeforeHash` or `PPDIS_AfterHash` though as the they really no independent logic in this commit.

Yes, please. Or better yet, alternate a little bit between the two.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181



More information about the cfe-commits mailing list