[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
Wed Nov 2 13:13:50 PDT 2022


owenpan added inline comments.


================
Comment at: clang/unittests/Format/FormatTest.cpp:5047
+  style.IndentPPDirectives = FormatStyle::PPDIS_None;
+  verifyFormat("#ifdef foo\n"
+               "#define bar() \\\n"
----------------
goldstein.w.n wrote:
> owenpan wrote:
> > Do you need to enclose the macro definition in `#ifdef`/`#endif`?
> No, but figured since this change is related to PP indentation the tests should have it. You would prefer I make the test just:
> 
> ```
>   verifyFormat("#define bar() \\\n"
>                "    if (A) {  \\\n"
>                "        B();  \\\n"
>                "    }         \\\n"
>                "    C();\n",
>                "#define bar() if (A) { B(); } C();\n",
>                style);
> ```
> 
> ?
My bad. Please ignore my comment.


================
Comment at: clang/unittests/Format/FormatTest.cpp:5054-5056
+               "#ifdef foo\n"
+               "#define bar() if (A) { B(); } C();\n"
+               "#endif",
----------------
goldstein.w.n wrote:
> owenpan wrote:
> > You can delete them.
> Them being the ifdef/endif? If so will do for V2.
Them being lines 5054-5056. (If you mouse over the comment, the lines will be highlighted.)


================
Comment at: clang/unittests/Format/FormatTest.cpp:5066-5068
+               "#ifdef foo\n"
+               "#define bar() if (A) { B(); } C();\n"
+               "#endif",
----------------
goldstein.w.n wrote:
> owenpan wrote:
> > Delete them or change to multiline format like lines 5048-5052.
> Delete the test of the ifndef/endif? Can do either for V2.
Actually, delete lines 5066-5068. There are mainly two ways to use `verifyFormat`: `verifyFormat(Expected, Input, ...)` and `verifyFormat(Input, ...)`. Here we can use the latter, in which `Input` is also expected output.


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