[clang-format] Keep level of comment before an empty line

[krasimir]

Summary

This patch fixes bug https://bugs.llvm.org/show_bug.cgi?id=3313: a comment line
was aligned with the next #ifdef even in the presence of an empty line between
them.

[djasper]

I still think we should make this dependent on the current indentation. Why do you think this is better?

[krasimir]

• Switch to original column-based detection

[krasimir]

• Update switch tests

[djasper]

Some small comments, otherwise looks good.

lib/Format/TokenAnnotator.cpp

1706

Add a comment a long the lines of:

// If the comment is currently aligned with the line immediately following it,
// that's probably intentional and we should keep it.

And maybe rename s/UpdateLevel/AlignedWithNextLine/

unittests/Format/FormatTest.cpp

855 ↗

(On Diff #106205)

Please format this with clang-format.

[krasimir]

• Address review comments

lib/Format/TokenAnnotator.cpp

 } // end anonymous namespace
 
 void TokenAnnotator::setCommentLineLevels(
     SmallVectorImpl<AnnotatedLine *> &Lines) {
   const AnnotatedLine *NextNonCommentLine = nullptr;
   for (SmallVectorImpl<AnnotatedLine *>::reverse_iterator I = Lines.rbegin(),
                                                           E = Lines.rend();
        I != E; ++I) {
-    bool CommentLine = (*I)->First;
+    bool CommentLine = true;
     for (const FormatToken *Tok = (*I)->First; Tok; Tok = Tok->Next) {
       if (!Tok->is(tok::comment)) {
         CommentLine = false;
         break;
       }
     }
-    if (NextNonCommentLine && CommentLine)
+
+    if (NextNonCommentLine && CommentLine) {
+      bool UpdateLevel = NextNonCommentLine->First->NewlinesBefore <= 1;
+      if (UpdateLevel)
         (*I)->Level = NextNonCommentLine->Level;
-    else
+    } else {
       NextNonCommentLine = (*I)->First->isNot(tok::r_brace) ? (*I) : nullptr;
+    }
 
     setCommentLineLevels((*I)->Children);
   }
 }
 
 static unsigned maxNestingDepth(const AnnotatedLine &Line) {
   unsigned Result = 0;
   for (const auto* Tok = Line.First; Tok != nullptr; Tok = Tok->Next)

unittests/Format/FormatTestComments.cpp

TEST_F(FormatTestComments, ParsesCommentsAdjacentToPPDirectives) {
   EXPECT_EQ("namespace {}\n// Test\n#define A",
             format("namespace {}\n   // Test\n#define A"));
   EXPECT_EQ("namespace {}\n/* Test */\n#define A",
             format("namespace {}\n   /* Test */\n#define A"));
   EXPECT_EQ("namespace {}\n/* Test */ #define A",
             format("namespace {}\n   /* Test */    #define A"));
 }
 
+TEST_F(FormatTestComments, KeepsLevelOfCommentBeforePPDirective) {
+  EXPECT_EQ("void f() {\n"
+            "  int i;\n"
+            "  /* comment */\n"
+            "\n"
+            "#ifdef A\n"
+            "  int j;\n"
+            "}",
+            format("void f() {\n"
+                   "  int i;\n"
+                   "  /* comment */\n"
+                   "\n"
+                   "#ifdef A\n"
+                   "  int j;\n"
+                   "}"));
+}
+
 TEST_F(FormatTestComments, SplitsLongLinesInComments) {
   EXPECT_EQ("/* This is a long\n"
             " * comment that\n"
             " * doesn't\n"
             " * fit on one line.\n"
             " */",
             format("/* "
                    "This is a long                                         "