[clang-format] Handle trailing comment sections in import statement lines

[krasimir]

Summary

This patch updates the handling of multiline trailing comment sections in
import statement lines to make it more consistent with the case in general.
This includes updating the parsing logic to collect the trailing comment
sections and the formatting logic to not insert escaped newlines at the end of
comment lines in import statement lines.

Specifically, before this patch this code:

#include <a> // line 1
             // line 2

will be turned into two unwrapped lines, whereas this code:

int i; // line 1
       // line 2

is turned into a single unwrapped line, enabling reflowing across comments.

An example where the old behaviour is bad is when partially formatting the lines
3 to 4 of this code:

#include <a> // line 1
             // line 2

int i;

which gets turned into:

#include <a> // line 1
             // line 2

             int i;

because the two comment lines were independent and the indent was copied.

[djasper]

nice

lib/Format/ContinuationIndenter.cpp

590 ↗

(On Diff #99536)

Call this "ContinuePPDirective".

lib/Format/TokenAnnotator.cpp

713 ↗

(On Diff #99536)

wat?

[krasimir]

• Address review comments

cfe/trunk/lib/Format/ContinuationIndenter.cpp

if (NextNonComment->is(tok::question) ||
       (PreviousNonComment && PreviousNonComment->is(tok::question)))
     State.Stack.back().BreakBeforeParameter = true;
   if (Current.is(TT_BinaryOperator) && Current.CanBreakBefore)
     State.Stack.back().BreakBeforeParameter = false;
 
   if (!DryRun) {
     unsigned Newlines = std::max(
         1u, std::min(Current.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1));
+    bool ContinuePPDirective =
+        State.Line->InPPDirective && State.Line->Type != LT_ImportStatement;
     Whitespaces.replaceWhitespace(Current, Newlines, State.Column, State.Column,
-                                  State.Line->InPPDirective);
+                                  ContinuePPDirective);
   }
 
   if (!Current.isTrailingComment())
     State.Stack.back().LastSpace = State.Column;
   if (Current.is(tok::lessless))
     // If we are breaking before a "<<", we always want to indent relative to
     // RHS. This is necessary only for "<<", as we special-case it and don't
     // always indent relative to the RHS.

cfe/trunk/lib/Format/TokenAnnotator.cpp

bool consumeToken() {
     default:
       break;
     }
     return true;
   }
 
   void parseIncludeDirective() {
     if (CurrentToken && CurrentToken->is(tok::less)) {
-      next();
-      while (CurrentToken) {
-        if (CurrentToken->isNot(tok::comment) || CurrentToken->Next)
+        // Mark tokens up to the trailing line comments as implicit string
+        // literals.
+        if (CurrentToken->isNot(tok::comment) &&
+            !CurrentToken->TokenText.startswith("//"))
           CurrentToken->Type = TT_ImplicitStringLiteral;
         next();
       }
     }
   }
 
   void parseWarningOrError() {
     next();

cfe/trunk/lib/Format/UnwrappedLineParser.cpp

else
       Line.MustBeDeclaration = true;
   }
 
 private:
   UnwrappedLine &Line;
   std::vector<bool> &Stack;
 };
 
+static bool isLineComment(const FormatToken &FormatTok) {
+  return FormatTok.is(tok::comment) &&
+         FormatTok.TokenText.startswith("//");
+}
+
 class ScopedMacroState : public FormatTokenSource {
 public:
   ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
                    FormatToken *&ResetToken)
       : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
         PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
-        Token(nullptr) {
+        Token(nullptr), PreviousToken(nullptr) {
     TokenSource = this;
     Line.Level = 0;
     Line.InPPDirective = true;
   }
 
   ~ScopedMacroState() override {
     TokenSource = PreviousTokenSource;
     ResetToken = Token;
     Line.InPPDirective = false;
     Line.Level = PreviousLineLevel;
   }
 
   FormatToken *getNextToken() override {
     // The \c UnwrappedLineParser guards against this by never calling
     // \c getNextToken() after it has encountered the first eof token.
     assert(!eof());
+    PreviousToken = Token;
     Token = PreviousTokenSource->getNextToken();
     if (eof())
       return getFakeEOF();
     return Token;
   }
 
   unsigned getPosition() override { return PreviousTokenSource->getPosition(); }
 
   FormatToken *setPosition(unsigned Position) override {
+    PreviousToken = nullptr;
     Token = PreviousTokenSource->setPosition(Position);
     return Token;
   }
 
 private:
-  bool eof() { return Token && Token->HasUnescapedNewline; }
+  bool eof() {
+    return Token && Token->HasUnescapedNewline &&
+           !(PreviousToken && isLineComment(*PreviousToken) &&
+             isLineComment(*Token) && Token->NewlinesBefore == 1);
+  }
 
   FormatToken *getFakeEOF() {
     static bool EOFInitialized = false;
     static FormatToken FormatTok;
     if (!EOFInitialized) {
       FormatTok.Tok.startToken();
       FormatTok.Tok.setKind(tok::eof);
       EOFInitialized = true;
     }
     return &FormatTok;
   }
 
   UnwrappedLine &Line;
   FormatTokenSource *&TokenSource;
   FormatToken *&ResetToken;
   unsigned PreviousLineLevel;
   FormatTokenSource *PreviousTokenSource;
 
   FormatToken *Token;
+  FormatToken *PreviousToken;
 };
 
 } // end anonymous namespace
 
 class ScopedLineState {
 public:
   ScopedLineState(UnwrappedLineParser &Parser,
                   bool SwitchToPreprocessorLines = false)

 
 bool UnwrappedLineParser::eof() const { return FormatTok->Tok.is(tok::eof); }
 
 bool UnwrappedLineParser::isOnNewLine(const FormatToken &FormatTok) {
   return (Line->InPPDirective || FormatTok.HasUnescapedNewline) &&
          FormatTok.NewlinesBefore > 0;
 }
 
-static bool isLineComment(const FormatToken &FormatTok) {
-  return FormatTok.is(tok::comment) &&
-         FormatTok.TokenText.startswith("//");
-}
-
 // Checks if \p FormatTok is a line comment that continues the line comment
 // section on \p Line.
 static bool continuesLineComment(const FormatToken &FormatTok,
                                  const UnwrappedLine &Line,
                                  llvm::Regex &CommentPragmasRegex) {
   if (Line.Tokens.empty())
     return false;
 

cfe/trunk/unittests/Format/FormatTestSelective.cpp

EXPECT_EQ(
       "var x = 'a';\n"
       "var x = \"a\";",
       format("var x = \"a\";\n"
              "var x = \"a\";\n"
              "var x = \"a\";",
              20, 0));
 }
 
+TEST_F(FormatTestSelective, KeepsIndentAfterCommentSectionImport) {
+  std::string Code = "#include <a> // line 1\n" // 23 chars long
+                     "             // line 2\n" // 23 chars long
+                     "\n"                       // this newline is char 47
+                     "int i;";                  // this line is not indented
+  EXPECT_EQ(Code, format(Code, 47, 1));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang