Correctly handle escaped newlines when the next token starts without a space.

[klimek]

Summary

We will need this to correctly handle conflict markers inside macro
definitions. Currently the tests as written fail directly when inserting
a space after the \\\n.

[djasper]

I am generally fine with this, as it seems irrelevant in practice. It seems weird, however, to replace an escaped newline with a newline.

unittests/Format/FormatTest.cpp

749–750 ↗

(On Diff #8472)

I find these tests easier to read if the strings are split after each \n.

Also, while I don't see a practical use here, I don't think simply removing the escaped newline is the right thing to do here. What other purpose can it have, other than being a notion of "the following comment is bound to this"? But if implementing that is extra effort, I don't care.

934 ↗

(On Diff #8472)

Same here.

I think here it is even worse as the test makes no sense whatsoever unless we also change the content of the comment. If we decided to keep this, maybe we should just remove/replace these tests?

[klimek]

Now without changes to the tests...

lib/Format/Format.cpp

unsigned Newlines =
         std::min(RootToken.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1);
     // Remove empty lines before "}" where applicable.
     if (RootToken.is(tok::r_brace) &&
         (!RootToken.Next ||
          (RootToken.Next->is(tok::semi) && !RootToken.Next->Next)))
       Newlines = std::min(Newlines, 1u);
     if (Newlines == 0 && !RootToken.IsFirst)
       Newlines = 1;
+    if (RootToken.IsFirst && !RootToken.HasUnescapedNewline)
+      Newlines = 0;
 
     // Remove empty lines after "{".
     if (!Style.KeepEmptyLinesAtTheStartOfBlocks && PreviousLine &&
         PreviousLine->Last->is(tok::l_brace) &&
         PreviousLine->First->isNot(tok::kw_namespace))
       Newlines = 1;
 
     // Insert extra new line before access specifiers.
FormatToken *getNextToken() {
     }
 
     // In case the token starts with escaped newlines, we want to
     // take them into account as whitespace - this pattern is quite frequent
     // in macro definitions.
     // FIXME: Add a more explicit test.
     while (FormatTok->TokenText.size() > 1 && FormatTok->TokenText[0] == '\\' &&
            FormatTok->TokenText[1] == '\n') {
-      // FIXME: ++FormatTok->NewlinesBefore is missing...
+      ++FormatTok->NewlinesBefore;
       WhitespaceLength += 2;
       Column = 0;
       FormatTok->TokenText = FormatTok->TokenText.substr(2);
     }
 
     FormatTok->WhitespaceRange = SourceRange(
         WhitespaceStart, WhitespaceStart.getLocWithOffset(WhitespaceLength));
 

lib/Format/TokenAnnotator.cpp

 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
                                      const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
   if (Right.is(tok::comment)) {
     return Right.Previous->BlockKind != BK_BracedInit &&
            Right.Previous->Type != TT_CtorInitializerColon &&
-           Right.NewlinesBefore > 0;
+           (Right.NewlinesBefore > 0 && Right.HasUnescapedNewline);
   } else if (Right.Previous->isTrailingComment() ||
              (Right.isStringLiteral() && Right.Previous->isStringLiteral())) {
     return true;
   } else if (Right.Previous->IsUnterminatedLiteral) {
     return true;
   } else if (Right.is(tok::lessless) && Right.Next &&
              Right.Previous->is(tok::string_literal) &&
              Right.Next->is(tok::string_literal)) {

lib/Format/UnwrappedLineParser.h

private:
   void addUnwrappedLine();
   bool eof() const;
   void nextToken();
   void readToken();
   void flushComments(bool NewlineBeforeNext);
   void pushToken(FormatToken *Tok);
   void calculateBraceTypes();
   void pushPPConditional();
+  bool isOnNewLine(const FormatToken& FormatTok);
 
   // FIXME: We are constantly running into bugs where Line.Level is incorrectly
   // subtracted from beyond 0. Introduce a method to subtract from Line.Level
   // and use that everywhere in the Parser.
   std::unique_ptr<UnwrappedLine> Line;
 
   // Comments are sorted into unwrapped lines by whether they are in the same
   // line as the previous token, or not. If not, they belong to the next token.

lib/Format/UnwrappedLineParser.cpp

for (SmallVectorImpl<UnwrappedLine>::iterator
       CurrentLines->push_back(*I);
     }
     PreprocessorDirectives.clear();
   }
 }
 
 bool UnwrappedLineParser::eof() const { return FormatTok->Tok.is(tok::eof); }
 
+bool UnwrappedLineParser::isOnNewLine(const FormatToken& FormatTok) {
+  return (Line->InPPDirective || FormatTok.HasUnescapedNewline) &&
+         FormatTok.NewlinesBefore > 0;
+}
+
 void UnwrappedLineParser::flushComments(bool NewlineBeforeNext) {
   bool JustComments = Line->Tokens.empty();
   for (SmallVectorImpl<FormatToken *>::const_iterator
            I = CommentsBeforeNextToken.begin(),
            E = CommentsBeforeNextToken.end();
        I != E; ++I) {
-    if ((*I)->NewlinesBefore && JustComments) {
+    if (isOnNewLine(**I) && JustComments) {
       addUnwrappedLine();
     }
     pushToken(*I);
   }
   if (NewlineBeforeNext && JustComments) {
     addUnwrappedLine();
   }
   CommentsBeforeNextToken.clear();
 }
 
 void UnwrappedLineParser::nextToken() {
   if (eof())
     return;
-  flushComments(FormatTok->NewlinesBefore > 0);
+  flushComments(isOnNewLine(*FormatTok));
   pushToken(FormatTok);
   readToken();
 }
 
 void UnwrappedLineParser::readToken() {
   bool CommentsInCurrentLine = true;
   do {
     FormatTok = Tokens->getNextToken();
     assert(FormatTok);
     while (!Line->InPPDirective && FormatTok->Tok.is(tok::hash) &&
            (FormatTok->HasUnescapedNewline || FormatTok->IsFirst)) {
       // If there is an unfinished unwrapped line, we flush the preprocessor
       // directives only after that unwrapped line was finished later.
       bool SwitchToPreprocessorLines =
           !Line->Tokens.empty() && CurrentLines == &Lines;
       ScopedLineState BlockState(*this, SwitchToPreprocessorLines);
       // Comments stored before the preprocessor directive need to be output
       // before the preprocessor directive, at the same level as the
       // preprocessor directive, as we consider them to apply to the directive.
-      flushComments(FormatTok->NewlinesBefore > 0);
+      flushComments(isOnNewLine(*FormatTok));
       parsePPDirective();
     }
 
     if (!PPStack.empty() && (PPStack.back() == PP_Unreachable) &&
         !Line->InPPDirective) {
       continue;
     }
 
     if (!FormatTok->Tok.is(tok::comment))
       return;
-    if (FormatTok->NewlinesBefore > 0 || FormatTok->IsFirst) {
+    if (isOnNewLine(*FormatTok) || FormatTok->IsFirst) {
       CommentsInCurrentLine = false;
     }
     if (CommentsInCurrentLine) {
       pushToken(FormatTok);
     } else {
       CommentsBeforeNextToken.push_back(FormatTok);
     }
   } while (!eof());