clang-format: Some tweaks to braces list formatting:

- Format a braced list with one element per line if it has nested
  braced lists.
- Use a column layout only when the list has 6+ elements (instead of the
  current 4+ elements).

llvm-svn: 198869

3 files changed, 46 insertions, 19 deletions

diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 2812a405b87..acd670b945a 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -721,13 +721,15 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
     Penalty += Style.PenaltyExcessCharacter * ExcessCharacters;
   }
 
+  if (Current.Role)
+    Current.Role->formatFromToken(State, this, DryRun);
   // If the previous has a special role, let it consume tokens as appropriate.
   // It is necessary to start at the previous token for the only implemented
   // role (comma separated list). That way, the decision whether or not to break
   // after the "{" is already done and both options are tried and evaluated.
   // FIXME: This is ugly, find a better way.
   if (Previous && Previous->Role)
-    Penalty += Previous->Role->format(State, this, DryRun);
+    Penalty += Previous->Role->formatAfterToken(State, this, DryRun);
 
   return Penalty;
 }
diff --git a/clang/lib/Format/FormatToken.cpp b/clang/lib/Format/FormatToken.cpp
index 16600f24ee8..0816668b578 100644
--- a/clang/lib/Format/FormatToken.cpp
+++ b/clang/lib/Format/FormatToken.cpp
@@ -26,11 +26,10 @@ TokenRole::~TokenRole() {}
 
 void TokenRole::precomputeFormattingInfos(const FormatToken *Token) {}
 
-unsigned CommaSeparatedList::format(LineState &State,
-                                    ContinuationIndenter *Indenter,
-                                    bool DryRun) {
-  if (!State.NextToken->Previous || !State.NextToken->Previous->Previous ||
-      Commas.size() <= 2)
+unsigned CommaSeparatedList::formatAfterToken(LineState &State,
+                                              ContinuationIndenter *Indenter,
+                                              bool DryRun) {
+  if (!State.NextToken->Previous || !State.NextToken->Previous->Previous)
     return 0;
 
   // Ensure that we start on the opening brace.
@@ -82,6 +81,14 @@ unsigned CommaSeparatedList::format(LineState &State,
   return Penalty;
 }
 
+unsigned CommaSeparatedList::formatFromToken(LineState &State,
+                                             ContinuationIndenter *Indenter,
+                                             bool DryRun) {
+  if (HasNestedBracedList)
+    State.Stack.back().AvoidBinPacking = true;
+  return 0;
+}
+
 // Returns the lengths in code points between Begin and End (both included),
 // assuming that the entire sequence is put on a single line.
 static unsigned CodePointsBetween(const FormatToken *Begin,
@@ -92,8 +99,7 @@ static unsigned CodePointsBetween(const FormatToken *Begin,
 
 void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) {
   // FIXME: At some point we might want to do this for other lists, too.
-  if (!Token->MatchingParen || Token->isNot(tok::l_brace) ||
-      Token->NestingLevel != 0)
+  if (!Token->MatchingParen || Token->isNot(tok::l_brace))
     return;
 
   FormatToken *ItemBegin = Token->Next;
@@ -103,7 +109,6 @@ void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) {
   // trailing comments which are otherwise ignored for column alignment.
   SmallVector<unsigned, 8> EndOfLineItemLength;
 
-  bool HasNestedBracedList = false;
   for (unsigned i = 0, e = Commas.size() + 1; i != e; ++i) {
     // Skip comments on their own line.
     while (ItemBegin->HasUnescapedNewline && ItemBegin->isTrailingComment())
@@ -143,6 +148,13 @@ void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) {
     ItemBegin = ItemEnd->Next;
   }
 
+  // If this doesn't have a nested list, we require at least 6 elements in order
+  // create a column layout. If it has a nested list, column layout ensures one
+  // list element per line.
+  if (HasNestedBracedList || Commas.size() < 5 ||
+      Token->NestingLevel != 0)
+    return;
+
   // We can never place more than ColumnLimit / 3 items in a row (because of the
   // spaces and the comma).
   for (unsigned Columns = 1; Columns <= Style.ColumnLimit / 3; ++Columns) {
@@ -179,11 +191,6 @@ void CommaSeparatedList::precomputeFormattingInfos(const FormatToken *Token) {
     if (Format.TotalWidth > Style.ColumnLimit)
       continue;
 
-    // If this braced list has nested braced list, we format it either with one
-    // element per line or with all elements on one line.
-    if (HasNestedBracedList && Columns > 1 && Format.LineCount > 1)
-      continue;
-
     Formats.push_back(Format);
   }
 }
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 391f9ee8000..dff2942c004 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -398,10 +398,21 @@ public:
 
   /// \brief Apply the special formatting that the given role demands.
   ///
+  /// Assumes that the token having this role is already formatted.
+  ///
   /// Continues formatting from \p State leaving indentation to \p Indenter and
   /// returns the total penalty that this formatting incurs.
-  virtual unsigned format(LineState &State, ContinuationIndenter *Indenter,
-                          bool DryRun) {
+  virtual unsigned formatFromToken(LineState &State,
+                                   ContinuationIndenter *Indenter,
+                                   bool DryRun) {
+    return 0;
+  }
+
+  /// \brief Same as \c formatFromToken, but assumes that the first token has
+  /// already been set thereby deciding on the first line break.
+  virtual unsigned formatAfterToken(LineState &State,
+                                    ContinuationIndenter *Indenter,
+                                    bool DryRun) {
     return 0;
   }
 
@@ -414,12 +425,17 @@ protected:
 
 class CommaSeparatedList : public TokenRole {
 public:
-  CommaSeparatedList(const FormatStyle &Style) : TokenRole(Style) {}
+  CommaSeparatedList(const FormatStyle &Style)
+      : TokenRole(Style), HasNestedBracedList(false) {}
 
   virtual void precomputeFormattingInfos(const FormatToken *Token);
 
-  virtual unsigned format(LineState &State, ContinuationIndenter *Indenter,
-                          bool DryRun);
+  virtual unsigned formatAfterToken(LineState &State,
+                                    ContinuationIndenter *Indenter,
+                                    bool DryRun);
+
+  virtual unsigned formatFromToken(LineState &State,
+                                   ContinuationIndenter *Indenter, bool DryRun);
 
   /// \brief Adds \p Token as the next comma to the \c CommaSeparated list.
   virtual void CommaFound(const FormatToken *Token) { Commas.push_back(Token); }
@@ -454,6 +470,8 @@ private:
 
   /// \brief Precomputed formats that can be used for this list.
   SmallVector<ColumnFormat, 4> Formats;
+
+  bool HasNestedBracedList;
 };
 
 } // namespace format