owenpan added a comment.

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

> In D137181#3935856 <https://reviews.llvm.org/D137181#3935856>, @goldstein.w.n 
> wrote:
>
>> 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.

My bad. I meant splitting the test cases between the two. Can you regroup them 
(lines 5137-5258) as follows?

  style.IndentPPDirectives = FormatStyle::PPDIS_AfterHash;
  style.IndentWidth = 4;
  style.PPIndentWidth = 1;
  verifyFormat("#ifdef foo\n"
               "# define bar() \\\n"
               "     if (A) {  \\\n"
               "         B();  \\\n"
               "     }         \\\n"
               "     C();\n"
               "#endif",
               style);
  verifyFormat("#if abc\n"
               "# ifdef foo\n"
               "#  define bar()    \\\n"
               "      if (A) {     \\\n"
               "          if (B) { \\\n"
               "              C(); \\\n"
               "          }        \\\n"
               "      }            \\\n"
               "      D();\n"
               "# endif\n"
               "#endif",
               style);
  verifyFormat("#ifndef foo\n"
               "#define foo\n"
               "if (emacs) {\n"
               "#ifdef is\n"
               "# define lit           \\\n"
               "     if (af) {         \\\n"
               "         return duh(); \\\n"
               "     }\n"
               "#endif\n"
               "}\n"
               "#endif",
               style);
  verifyFormat("#define X  \\\n"
               "    {      \\\n"
               "        x; \\\n"
               "        x; \\\n"
               "    }",
               style);
  
  style.IndentWidth = 8;
  style.PPIndentWidth = 2;
  verifyFormat("#ifdef foo\n"
               "#  define bar()        \\\n"
               "          if (A) {     \\\n"
               "                  B(); \\\n"
               "          }            \\\n"
               "          C();\n"
               "#endif",
               style);
  
  style.IndentWidth = 1;
  style.PPIndentWidth = 4;
  verifyFormat("#define X \\\n"
               " {        \\\n"
               "  x;      \\\n"
               "  x;      \\\n"
               " }",
               style);
  
  style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
  style.IndentWidth = 4;
  style.PPIndentWidth = 1;
  verifyFormat("if (emacs) {\n"
               "#ifdef is\n"
               " #define lit           \\\n"
               "     if (af) {         \\\n"
               "         return duh(); \\\n"
               "     }\n"
               "#endif\n"
               "}",
               style);
  verifyFormat("#if abc\n"
               " #ifdef foo\n"
               "  #define bar() \\\n"
               "      if (A) {  \\\n"
               "          B();  \\\n"
               "      }         \\\n"
               "      C();\n"
               " #endif\n"
               "#endif",
               style);
  verifyFormat("#if 1\n"
               " #define X  \\\n"
               "     {      \\\n"
               "         x; \\\n"
               "         x; \\\n"
               "     }\n"
               "#endif",
               style);
  
  style.PPIndentWidth = 2;
  verifyFormat("#ifdef foo\n"
               "  #define bar() \\\n"
               "      if (A) {  \\\n"
               "          B();  \\\n"
               "      }         \\\n"
               "      C();\n"
               "#endif",
               style);
  
  style.IndentWidth = 1;
  style.PPIndentWidth = 4;
  verifyFormat("#if 1\n"
               "    #define X \\\n"
               "     {        \\\n"
               "      x;      \\\n"
               "      x;      \\\n"
               "     }\n"
               "#endif",
               style);



================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:838
+  // If this changes PPLevel needs to be used for get correct indentation.
+  assert(!Line.InMacroBody && !InPPDirective);
   return Line.Level * Style.IndentWidth + Length <= ColumnLimit;
----------------
```
error: use of undeclared identifier 'InPPDirective'
  assert(!Line.InMacroBody && !InPPDirective);
                               ^
```
Also, can you break it into two assertions as suggested so that we know which 
one failed even without a debugger?


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1281
+  Line->PPLevel = PPBranchLevel + (IncludeGuard == IG_Defined ? 0 : 1);
   Line->InMacroBody = true;
 
----------------
In case `PPLevel` is negative.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to