Better support for constructor initializers.

We used to format initializers like this (with a sort of hacky implementation):
Constructor()
    : Val1(A),
      Val2(B) {

and now format like this (with a somewhat better solution):
Constructor()
    : Val1(A), Val2(B) {

assuming this would not fit on a single line. Also added tests.

As a side effect we now first analyze whether an UnwrappedLine needs to be
split at all. If not, not splitting it is the best solution by definition. As
this should be a very common case in normal code, not exploring the entire
solution space can provide significant speedup.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@170457 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 44 insertions, 49 deletions

diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 46af06fb41..45bd97d88d 100644
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -37,6 +37,7 @@ struct TokenAnnotation {
     TT_OverloadedOperator,
     TT_PointerOrReference,
     TT_ConditionalExpr,
+    TT_CtorInitializerColon,
     TT_LineComment,
     TT_BlockComment
   };
@@ -73,7 +74,6 @@ FormatStyle getGoogleStyle() {
 }
 
 struct OptimizationParameters {
-  unsigned PenaltyExtraLine;
   unsigned PenaltyIndentLevel;
 };
 
@@ -83,13 +83,9 @@ public:
                          const UnwrappedLine &Line,
                          const std::vector<TokenAnnotation> &Annotations,
                          tooling::Replacements &Replaces, bool StructuralError)
-      : Style(Style),
-        SourceMgr(SourceMgr),
-        Line(Line),
-        Annotations(Annotations),
-        Replaces(Replaces),
+      : Style(Style), SourceMgr(SourceMgr), Line(Line),
+        Annotations(Annotations), Replaces(Replaces),
         StructuralError(StructuralError) {
-    Parameters.PenaltyExtraLine = 100;
     Parameters.PenaltyIndentLevel = 5;
   }
 
@@ -100,8 +96,6 @@ public:
     // Initialize state dependent on indent.
     IndentState State;
     State.Column = Indent;
-    State.CtorInitializerOnNewLine = false;
-    State.InCtorInitializer = false;
     State.ConsumedTokens = 0;
     State.Indent.push_back(Indent + 4);
     State.LastSpace.push_back(Indent);
@@ -110,11 +104,33 @@ public:
     // The first token has already been indented and thus consumed.
     moveStateToNextToken(State);
 
+    // Check whether the UnwrappedLine can be put onto a single line. If so,
+    // this is bound to be the optimal solution (by definition) and we don't
+    // need to analyze the entire solution space. 
+    unsigned Columns = State.Column;
+    bool FitsOnALine = true;
+    for (unsigned i = 1, n = Line.Tokens.size(); i != n; ++i) {
+      Columns += (Annotations[i].SpaceRequiredBefore ? 1 : 0) +
+          Line.Tokens[i].Tok.getLength();
+      // A special case for the colon of a constructor initializer as this only
+      // needs to be put on a new line if the line needs to be split.
+      if (Columns > Style.ColumnLimit ||
+          (Annotations[i].MustBreakBefore &&
+           Annotations[i].Type != TokenAnnotation::TT_CtorInitializerColon)) {
+        FitsOnALine = false;
+        break;
+      }
+    }
+
     // Start iterating at 1 as we have correctly formatted of Token #0 above.
     for (unsigned i = 1, n = Line.Tokens.size(); i != n; ++i) {
-      unsigned NoBreak = calcPenalty(State, false, UINT_MAX);
-      unsigned Break = calcPenalty(State, true, NoBreak);
-      addTokenToState(Break < NoBreak, false, State);
+      if (FitsOnALine) {
+        addTokenToState(false, false, State);
+      } else {
+        unsigned NoBreak = calcPenalty(State, false, UINT_MAX);
+        unsigned Break = calcPenalty(State, true, NoBreak);
+        addTokenToState(Break < NoBreak, false, State);
+      }
     }
   }
 
@@ -146,9 +162,6 @@ private:
     /// on a level.
     std::vector<unsigned> FirstLessLess;
 
-    bool CtorInitializerOnNewLine;
-    bool InCtorInitializer;
-
     /// \brief Comparison operator to be able to used \c IndentState in \c map.
     bool operator<(const IndentState &Other) const {
       if (Other.ConsumedTokens != ConsumedTokens)
@@ -212,11 +225,8 @@ private:
 
       State.LastSpace[ParenLevel] = State.Indent[ParenLevel];
       if (Current.Tok.is(tok::colon) &&
-          Annotations[Index].Type != TokenAnnotation::TT_ConditionalExpr) {
+          Annotations[Index].Type != TokenAnnotation::TT_ConditionalExpr)
         State.Indent[ParenLevel] += 2;
-        State.CtorInitializerOnNewLine = true;
-        State.InCtorInitializer = true;
-      }
     } else {
       unsigned Spaces = Annotations[Index].SpaceRequiredBefore ? 1 : 0;
       if (Annotations[Index].Type == TokenAnnotation::TT_LineComment)
@@ -228,10 +238,7 @@ private:
       if (Previous.Tok.is(tok::l_paren) ||
           Annotations[Index - 1].Type == TokenAnnotation::TT_TemplateOpener)
         State.Indent[ParenLevel] = State.Column;
-      if (Current.Tok.is(tok::colon)) {
-        State.Indent[ParenLevel] = State.Column + 3;
-        State.InCtorInitializer = true;
-      }
+
       // Top-level spaces are exempt as that mostly leads to better results.
       State.Column += Spaces;
       if (Spaces > 0 && ParenLevel != 0)
@@ -279,10 +286,8 @@ private:
            "Tried to calculate penalty for splitting after the last token");
     const FormatToken &Left = Line.Tokens[Index];
     const FormatToken &Right = Line.Tokens[Index + 1];
-    if (Left.Tok.is(tok::semi))
+    if (Left.Tok.is(tok::semi) || Left.Tok.is(tok::comma))
       return 0;
-    if (Left.Tok.is(tok::comma))
-      return 1;
     if (Left.Tok.is(tok::equal) || Left.Tok.is(tok::l_paren) ||
         Left.Tok.is(tok::pipepipe) || Left.Tok.is(tok::ampamp))
       return 2;
@@ -313,18 +318,10 @@ private:
     if (NewLine && !Annotations[State.ConsumedTokens].CanBreakBefore)
       return UINT_MAX;
 
-    if (State.ConsumedTokens > 0 && !NewLine &&
-        State.CtorInitializerOnNewLine &&
-        Line.Tokens[State.ConsumedTokens - 1].Tok.is(tok::comma))
-      return UINT_MAX;
-
-    if (NewLine && State.InCtorInitializer && !State.CtorInitializerOnNewLine)
-      return UINT_MAX;
-
     unsigned CurrentPenalty = 0;
     if (NewLine) {
       CurrentPenalty += Parameters.PenaltyIndentLevel * State.Indent.size() +
-          Parameters.PenaltyExtraLine + splitPenalty(State.ConsumedTokens - 1);
+          splitPenalty(State.ConsumedTokens - 1);
     }
 
     addTokenToState(NewLine, true, State);
@@ -413,9 +410,7 @@ class TokenAnnotator {
 public:
   TokenAnnotator(const UnwrappedLine &Line, const FormatStyle &Style,
                  SourceManager &SourceMgr)
-      : Line(Line),
-        Style(Style),
-        SourceMgr(SourceMgr) {
+      : Line(Line), Style(Style), SourceMgr(SourceMgr) {
   }
 
   /// \brief A parser that gathers additional information about tokens.
@@ -427,9 +422,7 @@ public:
   public:
     AnnotatingParser(const SmallVector<FormatToken, 16> &Tokens,
                      std::vector<TokenAnnotation> &Annotations)
-        : Tokens(Tokens),
-          Annotations(Annotations),
-          Index(0) {
+        : Tokens(Tokens), Annotations(Annotations), Index(0) {
     }
 
     bool parseAngle() {
@@ -496,6 +489,10 @@ public:
       switch (Tokens[CurrentIndex].Tok.getKind()) {
       case tok::l_paren:
         parseParens();
+        if (Index < Tokens.size() && Tokens[Index].Tok.is(tok::colon)) {
+          Annotations[Index].Type = TokenAnnotation::TT_CtorInitializerColon;
+          next();
+        }
         break;
       case tok::l_square:
         parseSquare();
@@ -557,7 +554,10 @@ public:
       Annotation.CanBreakBefore =
           canBreakBetween(Line.Tokens[i - 1], Line.Tokens[i]);
 
-      if (Line.Tokens[i].Tok.is(tok::colon)) {
+      if (Annotation.Type == TokenAnnotation::TT_CtorInitializerColon) {
+        Annotation.MustBreakBefore = true;
+        Annotation.SpaceRequiredBefore = true;
+      } else if (Line.Tokens[i].Tok.is(tok::colon)) {
         Annotation.SpaceRequiredBefore =
             Line.Tokens[0].Tok.isNot(tok::kw_case) && i != e - 1;
       } else if (Annotations[i - 1].Type == TokenAnnotation::TT_UnaryOperator) {
@@ -769,9 +769,7 @@ private:
 class LexerBasedFormatTokenSource : public FormatTokenSource {
 public:
   LexerBasedFormatTokenSource(Lexer &Lex, SourceManager &SourceMgr)
-      : GreaterStashed(false),
-        Lex(Lex),
-        SourceMgr(SourceMgr),
+      : GreaterStashed(false), Lex(Lex), SourceMgr(SourceMgr),
         IdentTable(Lex.getLangOpts()) {
     Lex.SetKeepWhitespaceMode(true);
   }
@@ -831,10 +829,7 @@ class Formatter : public UnwrappedLineConsumer {
 public:
   Formatter(const FormatStyle &Style, Lexer &Lex, SourceManager &SourceMgr,
             const std::vector<CharSourceRange> &Ranges)
-      : Style(Style),
-        Lex(Lex),
-        SourceMgr(SourceMgr),
-        Ranges(Ranges),
+      : Style(Style), Lex(Lex), SourceMgr(SourceMgr), Ranges(Ranges),
         StructuralError(false) {
   }