Hi djasper,

Still needs some work, sending out for feedback of the design before I polish it
up.

The main change is in how we handle changes in the WhitespaceManager.
Instead of storing some changes, and applying others directly, we now
consistently store the changes in a uniform format, and do all layouting
in original source order in the end.
This allows us to correctly format unwrapped lines that are split up
by blocks, like:
f({
  g();
})

To support this, handling of protruding tokens had to be switched to an
a-priori annotation, which simplified the code (removing the need for
recursion).

This change also prepares more cleanup; I'm planning to have all alignments
done in the WhitespaceManager, which will also fix the problem of inconsistent
whitespace for indentation between comment alignments and other indents when
using tabs.

http://llvm-reviews.chandlerc.com/D840

Files:
  include/clang/Format/Format.h
  lib/Format/BreakableToken.cpp
  lib/Format/BreakableToken.h
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/TokenAnnotator.h
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -137,10 +137,8 @@
            PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
            PointerBindsToType == R.PointerBindsToType &&
            SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&
-           Standard == R.Standard &&
-           IndentWidth == R.IndentWidth &&
-           UseTab == R.UseTab &&
-           BreakBeforeBraces == R.BreakBeforeBraces;
+           Standard == R.Standard && IndentWidth == R.IndentWidth &&
+           UseTab == R.UseTab && BreakBeforeBraces == R.BreakBeforeBraces;
   }
 
 };
Index: lib/Format/BreakableToken.cpp
===================================================================
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -56,13 +56,10 @@
     AdditionalPrefix = "";
   }
 
-  unsigned WhitespaceStartColumn =
-      getContentStartColumn(LineIndex, TailOffset) + Split.first;
   unsigned BreakOffset = Text.data() - TokenText.data() + Split.first;
   unsigned CharsToRemove = Split.second;
   Whitespaces.breakToken(Tok, BreakOffset, CharsToRemove, "", AdditionalPrefix,
-                         InPPDirective, IndentAtLineBreak,
-                         WhitespaceStartColumn);
+                         InPPDirective, IndentAtLineBreak);
 }
 
 BreakableBlockComment::BreakableBlockComment(const SourceManager &SourceMgr,
@@ -144,16 +141,17 @@
   if (LineIndex == Lines.size() - 1)
     return;
   StringRef Text = Lines[LineIndex].substr(TailOffset);
+
+  // FIXME: This seems wrong...
   if (!Text.endswith(" ") && !InPPDirective)
     return;
 
   StringRef TrimmedLine = Text.rtrim();
-  unsigned WhitespaceStartColumn =
-      getLineLengthAfterSplit(LineIndex, TailOffset);
   unsigned BreakOffset = TrimmedLine.end() - TokenText.data();
   unsigned CharsToRemove = Text.size() - TrimmedLine.size() + 1;
+  // FIXME: The 0 is bad.
   Whitespaces.breakToken(Tok, BreakOffset, CharsToRemove, "", "", InPPDirective,
-                         0, WhitespaceStartColumn);
+                         0);
 }
 
 BreakableLineComment::BreakableLineComment(const SourceManager &SourceMgr,
Index: lib/Format/BreakableToken.h
===================================================================
--- lib/Format/BreakableToken.h
+++ lib/Format/BreakableToken.h
@@ -78,7 +78,10 @@
     if (ColumnLimit <= getDecorationLength())
       return Split(StringRef::npos, 0);
     unsigned MaxSplit = ColumnLimit - getDecorationLength();
-    assert(MaxSplit < Text.size());
+    // FIXME: Reduce unit test case.
+    if (Text.empty())
+      return Split(StringRef::npos, 0);
+    MaxSplit = std::min<unsigned>(MaxSplit, Text.size() - 1);
     StringRef::size_type SpaceOffset = Text.rfind(' ', MaxSplit);
     if (SpaceOffset != StringRef::npos && SpaceOffset != 0)
       return Split(SpaceOffset + 1, 0);
@@ -94,10 +97,8 @@
 
   virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
                            bool InPPDirective, WhitespaceManager &Whitespaces) {
-    unsigned WhitespaceStartColumn = StartColumn + Split.first + 2;
     Whitespaces.breakToken(Tok, 1 + TailOffset + Split.first, Split.second,
-                           "\"", "\"", InPPDirective, StartColumn,
-                           WhitespaceStartColumn);
+                           "\"", "\"", InPPDirective, StartColumn);
   }
 
 private:
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -143,7 +143,7 @@
   GoogleStyle.AlignEscapedNewlinesLeft = true;
   GoogleStyle.AllowAllParametersOfDeclarationOnNextLine = true;
   GoogleStyle.AllowShortIfStatementsOnASingleLine = true;
-  GoogleStyle.AllowShortLoopsOnASingleLine= true;
+  GoogleStyle.AllowShortLoopsOnASingleLine = true;
   GoogleStyle.BinPackParameters = true;
   GoogleStyle.ColumnLimit = 80;
   GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
@@ -242,10 +242,7 @@
         Whitespaces(Whitespaces), Count(0) {}
 
   /// \brief Formats an \c UnwrappedLine.
-  ///
-  /// \returns The column after the last token in the last line of the
-  /// \c UnwrappedLine.
-  unsigned format(const AnnotatedLine *NextLine) {
+  void format(const AnnotatedLine *NextLine) {
     // Initialize state dependent on indent.
     LineState State;
     State.Column = FirstIndent;
@@ -270,16 +267,15 @@
       while (State.NextToken != NULL) {
         addTokenToState(false, false, State);
       }
-      return State.Column;
     }
 
     // If the ObjC method declaration does not fit on a line, we should format
     // it with one arg per line.
     if (Line.Type == LT_ObjCMethodDecl)
       State.Stack.back().BreakBeforeParameter = true;
 
     // Find best solution in solution space.
-    return analyzeSolutionSpace(State);
+    analyzeSolutionSpace(State);
   }
 
 private:
@@ -465,7 +461,6 @@
     unsigned ContinuationIndent =
         std::max(State.Stack.back().LastSpace, State.Stack.back().Indent) + 4;
     if (Newline) {
-      unsigned WhitespaceStartColumn = State.Column;
       if (Current.is(tok::r_brace)) {
         State.Column = Line.Level * Style.IndentWidth;
       } else if (Current.is(tok::string_literal) &&
@@ -527,12 +522,8 @@
           NewLines =
               std::max(NewLines, std::min(Current.FormatTok.NewlinesBefore,
                                           Style.MaxEmptyLinesToKeep + 1));
-        if (!Line.InPPDirective)
-          Whitespaces.replaceWhitespace(Current, NewLines, State.Column,
-                                        WhitespaceStartColumn);
-        else
-          Whitespaces.replacePPWhitespace(Current, NewLines, State.Column,
-                                          WhitespaceStartColumn);
+        Whitespaces.replaceWhitespace(Current, NewLines, State.Column,
+                                      State.Column, Line.InPPDirective);
       }
 
       State.Stack.back().LastSpace = State.Column;
@@ -585,7 +576,8 @@
       unsigned Spaces = State.NextToken->SpacesRequiredBefore;
 
       if (!DryRun)
-        Whitespaces.replaceWhitespace(Current, 0, Spaces, State.Column);
+        Whitespaces.replaceWhitespace(Current, 0, Spaces,
+                                      State.Column + Spaces);
 
       if (Current.Type == TT_ObjCSelectorName &&
           State.Stack.back().ColonPos == 0) {
@@ -780,11 +772,10 @@
   /// already handled in \c addNextStateToQueue; the returned penalty will only
   /// cover the cost of the additional line breaks.
   unsigned breakProtrudingToken(const AnnotatedToken &Current, LineState &State,
-                                bool DryRun,
-                                unsigned UnbreakableTailLength = 0) {
+                                bool DryRun) {
+    unsigned UnbreakableTailLength = Current.UnbreakableSequenceLength;
     llvm::OwningPtr<BreakableToken> Token;
-    unsigned StartColumn = State.Column - Current.FormatTok.TokenLength -
-                           UnbreakableTailLength;
+    unsigned StartColumn = State.Column - Current.FormatTok.TokenLength;
     if (Current.is(tok::string_literal) &&
         Current.Type != TT_ImplicitStringLiteral) {
       // Only break up default narrow strings.
@@ -806,15 +797,7 @@
                 Current.Parent->Type != TT_ImplicitStringLiteral)) {
       Token.reset(new BreakableLineComment(SourceMgr, Current, StartColumn));
     } else {
-      // If a token that we cannot breaks protrudes, it means we were unable to
-      // break a sequence of tokens due to disallowed breaks between the tokens.
-      // Thus, we recursively search backwards to try to find a breakable token.
-      if (State.Column <= getColumnLimit() ||
-          Current.CanBreakBefore || !Current.Parent)
-        return 0;
-      return breakProtrudingToken(
-          *Current.Parent, State, DryRun,
-          UnbreakableTailLength + Current.FormatTok.TokenLength);
+      return 0;
     }
     if (UnbreakableTailLength >= getColumnLimit())
       return 0;
@@ -830,7 +813,7 @@
           Token->getLineLengthAfterSplit(LineIndex, TailOffset);
       while (RemainingTokenLength > RemainingSpace) {
         BreakableToken::Split Split =
-            Token->getSplit(LineIndex, TailOffset, RemainingSpace);
+            Token->getSplit(LineIndex, TailOffset, getColumnLimit());
         if (Split.first == StringRef::npos)
           break;
         assert(Split.first != 0);
@@ -854,7 +837,7 @@
     }
 
     if (BreakInserted) {
-      State.Column = PositionAfterLastLineInToken + UnbreakableTailLength;
+      State.Column = PositionAfterLastLineInToken;
       for (unsigned i = 0, e = State.Stack.size(); i != e; ++i)
         State.Stack[i].BreakBeforeParameter = true;
       State.Stack.back().LastSpace = StartColumn;
@@ -898,7 +881,7 @@
   /// the solution space (\c LineStates are the nodes). The algorithm tries to
   /// find the shortest path (the one with lowest penalty) from \p InitialState
   /// to a state where all tokens are placed.
-  unsigned analyzeSolutionSpace(LineState &InitialState) {
+  void analyzeSolutionSpace(LineState &InitialState) {
     std::set<LineState> Seen;
 
     // Insert start element into queue.
@@ -928,15 +911,15 @@
     if (Queue.empty())
       // We were unable to find a solution, do nothing.
       // FIXME: Add diagnostic?
-      return 0;
+      return;
 
     // Reconstruct the solution.
     reconstructPath(InitialState, Queue.top().second);
     DEBUG(llvm::dbgs() << "Total number of analyzed states: " << Count << "\n");
     DEBUG(llvm::dbgs() << "---\n");
 
     // Return the column after the last token of the solution.
-    return Queue.top().second->State.Column;
+    return;
   }
 
   void reconstructPath(LineState &State, StateNode *Current) {
@@ -1180,7 +1163,6 @@
     LexerBasedFormatTokenSource Tokens(Lex, SourceMgr);
     UnwrappedLineParser Parser(Style, Tokens, *this);
     bool StructuralError = Parser.parse();
-    unsigned PreviousEndOfLineColumn = 0;
     TokenAnnotator Annotator(Style, SourceMgr, Lex,
                              Tokens.getIdentTable().get("in"));
     for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
@@ -1236,7 +1218,7 @@
         if (PreviousLineWasTouched) {
           unsigned NewLines = std::min(FirstTok.NewlinesBefore, 1u);
           Whitespaces.replaceWhitespace(TheLine.First, NewLines, /*Indent*/ 0,
-                                        /*WhitespaceStartColumn*/ 0);
+                                        /*TargetColumn*/ 0);
         }
       } else if (TheLine.Type != LT_Invalid &&
                  (WasMoved || FormatPPDirective || touchesLine(TheLine))) {
@@ -1246,25 +1228,28 @@
             // we break apart a line consisting of multiple unwrapped lines.
             (FirstTok.NewlinesBefore == 0 || !StructuralError)) {
           formatFirstToken(TheLine.First, PreviousLineLastToken, Indent,
-                           TheLine.InPPDirective, PreviousEndOfLineColumn);
+                           TheLine.InPPDirective);
         } else {
           Indent = LevelIndent =
               SourceMgr.getSpellingColumnNumber(FirstTok.Tok.getLocation()) - 1;
         }
         UnwrappedLineFormatter Formatter(Style, SourceMgr, TheLine, Indent,
                                          TheLine.First, Whitespaces);
-        PreviousEndOfLineColumn =
-            Formatter.format(I + 1 != E ? &*(I + 1) : NULL);
+        Formatter.format(I + 1 != E ? &*(I + 1) : NULL);
         IndentForLevel[TheLine.Level] = LevelIndent;
         PreviousLineWasTouched = true;
       } else {
         if (FirstTok.NewlinesBefore > 0 || FirstTok.IsFirst) {
           unsigned LevelIndent =
               SourceMgr.getSpellingColumnNumber(FirstTok.Tok.getLocation()) - 1;
           // Remove trailing whitespace of the previous line if it was touched.
-          if (PreviousLineWasTouched || touchesEmptyLineBefore(TheLine))
+          if (PreviousLineWasTouched || touchesEmptyLineBefore(TheLine)) {
             formatFirstToken(TheLine.First, PreviousLineLastToken, LevelIndent,
-                             TheLine.InPPDirective, PreviousEndOfLineColumn);
+                             TheLine.InPPDirective);
+          } else {
+            Whitespaces.addUntouchableToken(TheLine.First,
+                                            TheLine.InPPDirective);
+          }
 
           if (static_cast<int>(LevelIndent) - Offset >= 0)
             LevelIndent -= Offset;
@@ -1274,18 +1259,9 @@
         // If we did not reformat this unwrapped line, the column at the end of
         // the last token is unchanged - thus, we can calculate the end of the
         // last token.
-        SourceLocation LastLoc = TheLine.Last->FormatTok.Tok.getLocation();
-        PreviousEndOfLineColumn =
-            SourceMgr.getSpellingColumnNumber(LastLoc) +
-            Lex.MeasureTokenLength(LastLoc, SourceMgr, Lex.getLangOpts()) - 1;
         PreviousLineWasTouched = false;
-        if (TheLine.Last->is(tok::comment))
-          Whitespaces.addUntouchableComment(
-              SourceMgr.getSpellingColumnNumber(
-                  TheLine.Last->FormatTok.Tok.getLocation()) -
-              1);
-        else
-          Whitespaces.alignComments();
+        if (&TheLine.First != TheLine.Last)
+          Whitespaces.addUntouchableToken(*TheLine.Last, TheLine.InPPDirective);
       }
       PreviousLineLastToken = I->Last;
     }
@@ -1545,25 +1521,21 @@
   /// Returns the indent level of the \c UnwrappedLine.
   void formatFirstToken(const AnnotatedToken &RootToken,
                         const AnnotatedToken *PreviousToken, unsigned Indent,
-                        bool InPPDirective, unsigned PreviousEndOfLineColumn) {
+                        bool InPPDirective) {
     const FormatToken &Tok = RootToken.FormatTok;
 
     unsigned Newlines =
         std::min(Tok.NewlinesBefore, Style.MaxEmptyLinesToKeep + 1);
     if (Newlines == 0 && !Tok.IsFirst)
       Newlines = 1;
 
-    if (!InPPDirective || Tok.HasUnescapedNewline) {
-      // Insert extra new line before access specifiers.
-      if (PreviousToken && PreviousToken->isOneOf(tok::semi, tok::r_brace) &&
-          RootToken.isAccessSpecifier() && Tok.NewlinesBefore == 1)
-        ++Newlines;
+    // Insert extra new line before access specifiers.
+    if (PreviousToken && PreviousToken->isOneOf(tok::semi, tok::r_brace) &&
+        RootToken.isAccessSpecifier() && Tok.NewlinesBefore == 1)
+      ++Newlines;
 
-      Whitespaces.replaceWhitespace(RootToken, Newlines, Indent, 0);
-    } else {
-      Whitespaces.replacePPWhitespace(RootToken, Newlines, Indent,
-                                      PreviousEndOfLineColumn);
-    }
+    Whitespaces.replaceWhitespace(RootToken, Newlines, Indent, Indent,
+                                  InPPDirective && !Tok.HasUnescapedNewline);
   }
 
   FormatStyle Style;
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -645,9 +645,9 @@
         else
           Current.Type = TT_BlockComment;
       } else if (Current.is(tok::r_paren)) {
-        bool ParensNotExpr = !Current.Parent ||
-                             Current.Parent->Type == TT_PointerOrReference ||
-                             Current.Parent->Type == TT_TemplateCloser;
+        bool ParensNotExpr =
+            !Current.Parent || Current.Parent->Type == TT_PointerOrReference ||
+            Current.Parent->Type == TT_TemplateCloser;
         bool ParensCouldEndDecl =
             !Current.Children.empty() &&
             Current.Children[0].isOneOf(tok::equal, tok::semi, tok::l_brace);
@@ -922,11 +922,29 @@
     Current = Current->Children.empty() ? NULL : &Current->Children[0];
   }
 
+  calculateUnbreakableSequenceLengths(Line);
   DEBUG({
     printDebugInfo(Line);
   });
 }
 
+void TokenAnnotator::calculateUnbreakableSequenceLengths(AnnotatedLine &Line) {
+  unsigned UnbreakableSequenceLength = 0;
+  AnnotatedToken *Current = Line.Last;
+  while (Current != NULL) {
+    Current->UnbreakableSequenceLength = UnbreakableSequenceLength;
+    if (!Current->CanBreakBefore &&
+        Current->FormatTok.Tok.isNot(tok::comment) &&
+        Current->FormatTok.Tok.isNot(tok::string_literal)) {
+      UnbreakableSequenceLength +=
+          Current->FormatTok.TokenLength + Current->SpacesRequiredBefore;
+    } else {
+      UnbreakableSequenceLength = 0;
+    }
+    Current = Current->Parent;
+  }
+}
+
 unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line,
                                       const AnnotatedToken &Tok) {
   const AnnotatedToken &Left = *Tok.Parent;
Index: lib/Format/TokenAnnotator.h
===================================================================
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -76,10 +76,10 @@
         CanBreakBefore(false), MustBreakBefore(false),
         ClosesTemplateDeclaration(false), MatchingParen(NULL),
         ParameterCount(0), TotalLength(FormatTok.TokenLength),
-        BindingStrength(0), SplitPenalty(0), LongestObjCSelectorName(0),
-        DefinesFunctionType(false), Parent(NULL), FakeRParens(0),
-        LastInChainOfCalls(false), PartOfMultiVariableDeclStmt(false),
-        NoMoreTokensOnLevel(false) {}
+        UnbreakableSequenceLength(0), BindingStrength(0), SplitPenalty(0),
+        LongestObjCSelectorName(0), DefinesFunctionType(false), Parent(NULL),
+        FakeRParens(0), LastInChainOfCalls(false),
+        PartOfMultiVariableDeclStmt(false), NoMoreTokensOnLevel(false) {}
 
   bool is(tok::TokenKind Kind) const { return FormatTok.Tok.is(Kind); }
 
@@ -154,6 +154,10 @@
   /// \brief The total length of the line up to and including this token.
   unsigned TotalLength;
 
+  /// \brief The length of following tokens until the next natural split point,
+  /// or the next token that can be broken.
+  unsigned UnbreakableSequenceLength;
+
   // FIXME: Come up with a 'cleaner' concept.
   /// \brief The binding strength of a token. This is a combined value of
   /// operator precedence, parenthesis nesting, etc.
@@ -282,6 +286,8 @@
 
   void printDebugInfo(const AnnotatedLine &Line);
 
+  void calculateUnbreakableSequenceLengths(AnnotatedLine &Line);
+
   const FormatStyle &Style;
   SourceManager &SourceMgr;
   Lexer &Lex;
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -36,6 +36,7 @@
     else
       Line.MustBeDeclaration = true;
   }
+
 private:
   UnwrappedLine &Line;
   std::vector<bool> &Stack;
Index: lib/Format/WhitespaceManager.cpp
===================================================================
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -18,93 +18,94 @@
 namespace clang {
 namespace format {
 
+WhitespaceManager::Change WhitespaceManager::Change::replaceTokenWhitespace(
+    const FormatToken &Tok, unsigned Newlines, unsigned Spaces,
+    unsigned StartOfTokenColumn, bool InPPDirective, StringRef PreviousPostfix,
+    StringRef CurrentPrefix) {
+  Change C;
+  C.OriginalWhitespaceRange =
+      SourceRange(Tok.WhiteSpaceStart,
+                  Tok.WhiteSpaceStart.getLocWithOffset(Tok.WhiteSpaceLength));
+  C.NewlinesBefore = Newlines;
+  C.Spaces = Spaces;
+  C.StartOfTokenColumn = StartOfTokenColumn;
+  C.PreviousLinePostfix = PreviousPostfix;
+  C.CurrentLinePrefix = CurrentPrefix;
+  C.ContinuesPPDirective = InPPDirective && !Tok.IsFirst;
+  C.IsComment = Tok.Tok.is(tok::comment);
+  C.IsEOF = Tok.Tok.is(tok::eof);
+  C.EscapedNewlineColumn = 0;
+  C.CreateReplacement = true;
+  return C;
+}
+
 void WhitespaceManager::replaceWhitespace(const AnnotatedToken &Tok,
                                           unsigned NewLines, unsigned Spaces,
-                                          unsigned WhitespaceStartColumn) {
-  if (NewLines > 0)
-    alignEscapedNewlines();
-
-  // 2+ newlines mean an empty line separating logic scopes.
-  if (NewLines >= 2)
-    alignComments();
+                                          unsigned StartOfTokenColumn,
+                                          bool InPPDirective,
+                                          StringRef PreviousPostfix,
+                                          StringRef CurrentPrefix) {
+  Changes.push_back(Change::replaceTokenWhitespace(
+      Tok.FormatTok, NewLines, Spaces, StartOfTokenColumn, InPPDirective,
+      PreviousPostfix, CurrentPrefix));
 
   // Align line comments if they are trailing or if they continue other
   // trailing comments.
+  // FIXME: Pull this out and generalize so it works the same way in broken
+  // comments and unbroken comments with trailing whitespace.
   if (Tok.isTrailingComment()) {
     SourceLocation TokenEndLoc = Tok.FormatTok.getStartOfNonWhitespace()
         .getLocWithOffset(Tok.FormatTok.TokenLength);
     // Remove the comment's trailing whitespace.
     if (Tok.FormatTok.TrailingWhiteSpaceLength != 0)
       Replaces.insert(tooling::Replacement(
           SourceMgr, TokenEndLoc, Tok.FormatTok.TrailingWhiteSpaceLength, ""));
-
-    bool LineExceedsColumnLimit =
-        Spaces + WhitespaceStartColumn + Tok.FormatTok.TokenLength >
-        Style.ColumnLimit;
-    // Align comment with other comments.
-    if ((Tok.Parent != NULL || !Comments.empty()) &&
-        !LineExceedsColumnLimit) {
-      unsigned MinColumn =
-          NewLines > 0 ? Spaces : WhitespaceStartColumn + Spaces;
-      unsigned MaxColumn = Style.ColumnLimit - Tok.FormatTok.TokenLength;
-      Comments.push_back(StoredToken(
-          Tok.FormatTok.WhiteSpaceStart, Tok.FormatTok.WhiteSpaceLength,
-          MinColumn, MaxColumn, NewLines, Spaces));
-      return;
-    }
   }
-
-  // If this line does not have a trailing comment, align the stored comments.
-  if (Tok.Children.empty() && !Tok.isTrailingComment())
-    alignComments();
-
-  storeReplacement(Tok.FormatTok.WhiteSpaceStart,
-                   Tok.FormatTok.WhiteSpaceLength,
-                   getNewLineText(NewLines, Spaces));
 }
 
 void WhitespaceManager::replacePPWhitespace(const AnnotatedToken &Tok,
                                             unsigned NewLines, unsigned Spaces,
                                             unsigned WhitespaceStartColumn) {
-  if (NewLines == 0) {
-    replaceWhitespace(Tok, NewLines, Spaces, WhitespaceStartColumn);
-  } else {
-    // The earliest position for "\" is 2 after the last token.
-    unsigned MinColumn = WhitespaceStartColumn + 2;
-    unsigned MaxColumn = Style.ColumnLimit;
-    EscapedNewlines.push_back(StoredToken(
-        Tok.FormatTok.WhiteSpaceStart, Tok.FormatTok.WhiteSpaceLength,
-        MinColumn, MaxColumn, NewLines, Spaces));
-  }
+  replaceWhitespace(Tok, NewLines, Spaces, WhitespaceStartColumn, true);
+}
+
+void WhitespaceManager::addUntouchableToken(const AnnotatedToken &Tok,
+                                            bool InPPDirective) {
+  Change C(Change::replaceTokenWhitespace(
+      Tok.FormatTok, Tok.FormatTok.NewlinesBefore,
+      Tok.FormatTok.WhiteSpaceLength - Tok.FormatTok.NewlinesBefore,
+      SourceMgr.getSpellingColumnNumber(Tok.FormatTok.Tok.getLocation()) - 1,
+      InPPDirective));
+  C.CreateReplacement = false;
+  Changes.push_back(C);
 }
 
 void WhitespaceManager::breakToken(const FormatToken &Tok, unsigned Offset,
                                    unsigned ReplaceChars, StringRef Prefix,
                                    StringRef Postfix, bool InPPDirective,
-                                   unsigned Spaces,
-                                   unsigned WhitespaceStartColumn) {
-  SourceLocation Location =
-      Tok.getStartOfNonWhitespace().getLocWithOffset(Offset);
-  if (InPPDirective) {
-    // The earliest position for "\" is 2 after the last token.
-    unsigned MinColumn = WhitespaceStartColumn + 2;
-    unsigned MaxColumn = Style.ColumnLimit;
-    StoredToken StoredTok = StoredToken(Location, ReplaceChars, MinColumn,
-                                        MaxColumn, /*NewLines=*/ 1, Spaces);
-    StoredTok.Prefix = Prefix;
-    StoredTok.Postfix = Postfix;
-    EscapedNewlines.push_back(StoredTok);
-  } else {
-    std::string ReplacementText =
-        (Prefix + getNewLineText(1, Spaces) + Postfix).str();
-    Replaces.insert(tooling::Replacement(SourceMgr, Location, ReplaceChars,
-                                         ReplacementText));
-  }
+                                   unsigned Spaces) {
+  Change C(Change::replaceTokenWhitespace(Tok, 1, Spaces, Spaces, InPPDirective,
+                                          Prefix, Postfix));
+  C.OriginalWhitespaceRange = SourceRange(
+      Tok.getStartOfNonWhitespace().getLocWithOffset(Offset),
+      Tok.getStartOfNonWhitespace().getLocWithOffset(Offset + ReplaceChars));
+  // FIXME: Unify token adjustment, so we don't split it between BreakableToken
+  // and the WhitespaceManager. That would also allow us to store a
+  // tok::TokenKind instead of bools inside the Change.
+  C.IsComment = false;
+  Changes.push_back(C);
 }
 
 const tooling::Replacements &WhitespaceManager::generateReplacements() {
-  alignComments();
+  if (Changes.empty())
+    return Replaces;
+
+  std::sort(Changes.begin(), Changes.end(), IsBeforeInFile(SourceMgr));
+  calculateMissingInformation();
+  alignTrailingComments();
   alignEscapedNewlines();
+  generateChanges();
+
   return Replaces;
 }
 
@@ -114,10 +115,44 @@
       tooling::Replacement(SourceMgr, SourceLoc, ReplaceChars, Text));
 }
 
-void WhitespaceManager::addUntouchableComment(unsigned Column) {
-  StoredToken Tok = StoredToken(SourceLocation(), 0, Column, Column, 0, 0);
-  Tok.Untouchable = true;
-  Comments.push_back(Tok);
+void WhitespaceManager::calculateMissingInformation() {
+  Changes[0].PreviousEndOfTokenColumn = 0;
+  for (unsigned i = 1, e = Changes.size(); i != e; ++i) {
+    unsigned OriginalWhitespaceStart =
+        SourceMgr.getFileOffset(Changes[i].OriginalWhitespaceRange.getBegin());
+    unsigned PreviousOriginalWhitespaceEnd = SourceMgr.getFileOffset(
+        Changes[i - 1].OriginalWhitespaceRange.getEnd());
+    Changes[i - 1].TokenLength =
+        OriginalWhitespaceStart - PreviousOriginalWhitespaceEnd +
+        Changes[i].PreviousLinePostfix.size() +
+        Changes[i - 1].CurrentLinePrefix.size();
+
+    Changes[i].PreviousEndOfTokenColumn =
+        Changes[i - 1].StartOfTokenColumn + Changes[i - 1].TokenLength;
+
+    Changes[i - 1].IsTrailingComment =
+        (Changes[i].NewlinesBefore > 0 || Changes[i].IsEOF) &&
+        Changes[i - 1].IsComment;
+  }
+  // FIXME: Reduce a unit test where the last token is not and eof token,
+  // and the trailing comment before it needs to be aligned.
+}
+
+void WhitespaceManager::generateChanges() {
+  for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
+    const Change &C = Changes[i];
+    if (C.CreateReplacement) {
+      std::string ReplacementText =
+          C.PreviousLinePostfix +
+          (C.ContinuesPPDirective
+               ? getNewLineText(C.NewlinesBefore, C.Spaces,
+                                C.PreviousEndOfTokenColumn,
+                                C.EscapedNewlineColumn)
+               : getNewLineText(C.NewlinesBefore, C.Spaces)) +
+          C.CurrentLinePrefix;
+      storeReplacement(C.OriginalWhitespaceRange, ReplacementText);
+    }
+  }
 }
 
 std::string WhitespaceManager::getNewLineText(unsigned NewLines,
@@ -150,69 +185,99 @@
          std::string(Spaces % Style.IndentWidth, ' ');
 }
 
-void WhitespaceManager::alignComments() {
+void WhitespaceManager::alignTrailingComments() {
   unsigned MinColumn = 0;
   unsigned MaxColumn = UINT_MAX;
-  token_iterator Start = Comments.begin();
-  for (token_iterator I = Start, E = Comments.end(); I != E; ++I) {
-    if (I->MinColumn > MaxColumn || I->MaxColumn < MinColumn) {
-      alignComments(Start, I, MinColumn);
-      MinColumn = I->MinColumn;
-      MaxColumn = I->MaxColumn;
-      Start = I;
-    } else {
-      MinColumn = std::max(MinColumn, I->MinColumn);
-      MaxColumn = std::min(MaxColumn, I->MaxColumn);
+  unsigned StartOfSequence = 0;
+  bool BreakBeforeNext = false;
+  unsigned Newlines = 0;
+  for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
+    unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
+    // FIXME: Correctly handle in PP directives.
+    unsigned ChangeMaxColumn = Style.ColumnLimit - Changes[i].TokenLength;
+    Newlines += Changes[i].NewlinesBefore;
+    if (Changes[i].IsTrailingComment) {
+      if (BreakBeforeNext ||
+          (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
+          Newlines > 1 ||
+          // Break the comment sequence if the previous line did not end
+          // in a trailing comment.
+          (Changes[i].NewlinesBefore == 1 && i > 0 &&
+           !Changes[i - 1].IsTrailingComment)) {
+        alignTrailingComments(StartOfSequence, i, MinColumn);
+        MinColumn = ChangeMinColumn;
+        MaxColumn = ChangeMaxColumn;
+        StartOfSequence = i;
+      } else {
+        MinColumn = std::max(MinColumn, ChangeMinColumn);
+        MaxColumn = std::min(MaxColumn, ChangeMaxColumn);
+      }
+      BreakBeforeNext =
+          (i == 0) || (Changes[i].NewlinesBefore > 1) ||
+          (Changes[i].NewlinesBefore == 1 && !Changes[i - 1].IsTrailingComment);
+      Newlines = 0;
     }
   }
-  alignComments(Start, Comments.end(), MinColumn);
-  Comments.clear();
+  alignTrailingComments(StartOfSequence, Changes.size(), MinColumn);
 }
 
-void WhitespaceManager::alignComments(token_iterator I, token_iterator E,
-                                      unsigned Column) {
-  while (I != E) {
-    if (!I->Untouchable) {
-      unsigned Spaces = I->Spaces + Column - I->MinColumn;
-      storeReplacement(I->ReplacementLoc, I->ReplacementLength,
-                       getNewLineText(I->NewLines, Spaces));
+void WhitespaceManager::alignTrailingComments(unsigned Start, unsigned End,
+                                              unsigned Column) {
+  for (unsigned i = Start; i != End; ++i) {
+    if (Changes[i].IsTrailingComment) {
+      assert(Column >= Changes[i].StartOfTokenColumn);
+      Changes[i].Spaces += Column - Changes[i].StartOfTokenColumn;
+      Changes[i].StartOfTokenColumn = Column;
     }
-    ++I;
   }
 }
 
 void WhitespaceManager::alignEscapedNewlines() {
-  unsigned MinColumn;
-  if (Style.AlignEscapedNewlinesLeft) {
-    MinColumn = 0;
-    for (token_iterator I = EscapedNewlines.begin(), E = EscapedNewlines.end();
-         I != E; ++I) {
-      if (I->MinColumn > MinColumn)
-        MinColumn = I->MinColumn;
+  unsigned MaxEndOfLine = 0;
+  unsigned StartOfMacro = 0;
+  for (unsigned i = 1, e = Changes.size(); i < e; ++i) {
+    Change &C = Changes[i];
+    if (C.NewlinesBefore > 0) {
+      if (C.ContinuesPPDirective) {
+        if (Style.AlignEscapedNewlinesLeft)
+          MaxEndOfLine = std::max(C.PreviousEndOfTokenColumn + 2, MaxEndOfLine);
+        else
+          MaxEndOfLine = Style.ColumnLimit;
+      } else {
+        alignEscapedNewlines(StartOfMacro + 1, i, MaxEndOfLine);
+        MaxEndOfLine = 0;
+        StartOfMacro = i;
+      }
     }
-  } else {
-    MinColumn = Style.ColumnLimit;
   }
+  alignEscapedNewlines(StartOfMacro + 1, Changes.size(), MaxEndOfLine);
+}
 
-  for (token_iterator I = EscapedNewlines.begin(), E = EscapedNewlines.end();
-       I != E; ++I) {
-    // I->MinColumn - 2 is the end of the previous token (i.e. the
-    // WhitespaceStartColumn).
-    storeReplacement(
-        I->ReplacementLoc, I->ReplacementLength,
-        I->Prefix + getNewLineText(I->NewLines, I->Spaces, I->MinColumn - 2,
-                                   MinColumn) + I->Postfix);
-
+void WhitespaceManager::alignEscapedNewlines(unsigned Start, unsigned End,
+                                             unsigned MaxEndOfLine) {
+  for (unsigned i = Start; i < End; ++i) {
+    Change &C = Changes[i];
+    if (C.NewlinesBefore > 0) {
+      assert(C.ContinuesPPDirective);
+      if (C.PreviousEndOfTokenColumn + 1 > MaxEndOfLine)
+        C.EscapedNewlineColumn = 0;
+      else
+        C.EscapedNewlineColumn = MaxEndOfLine;
+    }
   }
-  EscapedNewlines.clear();
 }
 
-void WhitespaceManager::storeReplacement(SourceLocation Loc, unsigned Length,
-                                         const std::string Text) {
+void WhitespaceManager::storeReplacement(const SourceRange &Range,
+                                         StringRef Text) {
+  unsigned WhitespaceLength = SourceMgr.getFileOffset(Range.getEnd()) -
+                              SourceMgr.getFileOffset(Range.getBegin());
   // Don't create a replacement, if it does not change anything.
-  if (StringRef(SourceMgr.getCharacterData(Loc), Length) == Text)
+  if (StringRef(SourceMgr.getCharacterData(Range.getBegin()),
+                WhitespaceLength) ==
+      Text)
     return;
-  Replaces.insert(tooling::Replacement(SourceMgr, Loc, Length, Text));
+  Replaces.insert(tooling::Replacement(
+      SourceMgr, CharSourceRange::getCharRange(Range), Text));
 }
 
 } // namespace format
Index: lib/Format/WhitespaceManager.h
===================================================================
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -19,6 +19,7 @@
 #include "TokenAnnotator.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
+#include "clang/Tooling/Tooling.h"
 #include <string>
 
 namespace clang {
@@ -36,7 +37,10 @@
   /// \brief Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   void replaceWhitespace(const AnnotatedToken &Tok, unsigned NewLines,
-                         unsigned Spaces, unsigned WhitespaceStartColumn);
+                         unsigned Spaces, unsigned WhitespaceStartColumn,
+                         bool InPPDirective = false,
+                         StringRef PreviousPostfix = "",
+                         StringRef CurrentPrefix = "");
 
   /// \brief Like \c replaceWhitespace, but additionally adds right-aligned
   /// backslashes to escape newlines inside a preprocessor directive.
@@ -55,22 +59,16 @@
   /// used to generate the correct line break.
   void breakToken(const FormatToken &Tok, unsigned Offset,
                   unsigned ReplaceChars, StringRef Prefix, StringRef Postfix,
-                  bool InPPDirective, unsigned Spaces,
-                  unsigned WhitespaceStartColumn);
+                  bool InPPDirective, unsigned Spaces);
 
   /// \brief Returns all the \c Replacements created during formatting.
   const tooling::Replacements &generateReplacements();
 
+  void addUntouchableToken(const AnnotatedToken &Tok, bool InPPDirective);
+
   void addReplacement(const SourceLocation &SourceLoc, unsigned ReplaceChars,
                       StringRef Text);
 
-  void addUntouchableComment(unsigned Column);
-
-  /// \brief Try to align all stashed comments.
-  void alignComments();
-  /// \brief Try to align all stashed escaped newlines.
-  void alignEscapedNewlines();
-
 private:
   std::string getNewLineText(unsigned NewLines, unsigned Spaces);
 
@@ -80,35 +78,64 @@
 
   std::string getIndentText(unsigned Spaces);
 
-  /// \brief Structure to store tokens for later layout and alignment.
-  struct StoredToken {
-    StoredToken(SourceLocation ReplacementLoc, unsigned ReplacementLength,
-                unsigned MinColumn, unsigned MaxColumn, unsigned NewLines,
-                unsigned Spaces)
-        : ReplacementLoc(ReplacementLoc), ReplacementLength(ReplacementLength),
-          MinColumn(MinColumn), MaxColumn(MaxColumn), NewLines(NewLines),
-          Spaces(Spaces), Untouchable(false) {}
-    SourceLocation ReplacementLoc;
-    unsigned ReplacementLength;
-    unsigned MinColumn;
-    unsigned MaxColumn;
-    unsigned NewLines;
+  // Represents a change before a token, or a break inside a token.
+  struct Change {
+    static Change replaceTokenWhitespace(const FormatToken &Tok,
+                                         unsigned Newlines, unsigned Spaces,
+                                         unsigned StartOfTokenColumn,
+                                         bool InPPDirective,
+                                         StringRef PreviousPostfix = "",
+                                         StringRef CurrentPrefix = "");
+
+    bool CreateReplacement;
+
+    // Changes might be in the middle of a token, so we cannot just keep the
+    // FormatToken around to query its information.
+    SourceRange OriginalWhitespaceRange;
     unsigned Spaces;
-    bool Untouchable;
-    std::string Prefix;
-    std::string Postfix;
+    unsigned StartOfTokenColumn;
+    unsigned NewlinesBefore;
+    std::string PreviousLinePostfix;
+    std::string CurrentLinePrefix;
+
+    bool IsComment;
+    bool IsEOF;
+    bool IsTrailingComment;
+
+    bool ContinuesPPDirective;
+
+
+    unsigned TokenLength;
+    unsigned PreviousEndOfTokenColumn;
+    unsigned EscapedNewlineColumn;
+  };
+  SmallVector<Change, 16> Changes;
+
+  class IsBeforeInFile {
+  public:
+    IsBeforeInFile(const SourceManager &SM) : SM(SM) {}
+    bool operator()(const Change &C1, const Change &C2) const {
+      return SM.getFileOffset(C1.OriginalWhitespaceRange.getBegin()) <
+             SM.getFileOffset(C2.OriginalWhitespaceRange.getBegin());
+    }
+
+  private:
+    const SourceManager &SM;
   };
-  SmallVector<StoredToken, 16> Comments;
-  SmallVector<StoredToken, 16> EscapedNewlines;
-  typedef SmallVector<StoredToken, 16>::iterator token_iterator;
 
+  void calculateMissingInformation();
+  /// \brief Try to align all stashed comments.
+  void alignTrailingComments();
   /// \brief Put all the comments between \p I and \p E into \p Column.
-  void alignComments(token_iterator I, token_iterator E, unsigned Column);
+  void alignTrailingComments(unsigned Start, unsigned End, unsigned Column);
+  /// \brief Try to align all stashed escaped newlines.
+  void alignEscapedNewlines();
+  void alignEscapedNewlines(unsigned Start, unsigned End,
+                            unsigned MaxEndOfLine);
 
-  /// \brief Stores \p Text as the replacement for the whitespace in front of
-  /// \p Tok.
-  void storeReplacement(SourceLocation Loc, unsigned Length,
-                        const std::string Text);
+  /// \brief Stores \p Text as the replacement for the whitespace in \p Range.
+  void storeReplacement(const SourceRange &Range, StringRef Text);
+  void generateChanges();
 
   SourceManager &SourceMgr;
   tooling::Replacements Replaces;
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1528,6 +1528,13 @@
                    "};"));
 }
 
+TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
+  verifyFormat("#define A \\\n"
+               "  f({     \\\n"
+               "    g();  \\\n"
+               "  });", getLLVMStyleWithColumns(11));
+}
+
 TEST_F(FormatTest, IndentPreprocessorDirectivesAtZero) {
   EXPECT_EQ("{\n  {\n#define A\n  }\n}", format("{{\n#define A\n}}"));
 }
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to