Add experimental flag for adaptive parameter bin-packing.

This is not activated for any style, might change or go away
completely.

For those that want to play around with it, set
ExperimentalAutoDetectBinPacking to true.

clang-format will then:
Look at whether function calls/declarations/definitions are currently
formatted with one parameter per line (on a case-by-case basis). If so,
clang-format will avoid bin-packing the parameters. If all parameters
are on one line (thus that line is "inconclusive"), clang-format will
make the choice dependent on whether there are other bin-packed
calls/declarations in the same file.

The reason for this change is that bin-packing in some situations can be
really bad and an author might opt to put one parameter on each line. If
the author does that, he might want clang-format not to mess with that.
If the author is unhappy with the one-per-line formatting, clang-format
can easily be convinced to bin-pack by putting any two parameters on the
same line.

llvm-svn: 186003

3 files changed, 62 insertions, 12 deletions

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index b6b726cfb21..ad390c23dcd 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -94,6 +94,8 @@ template <> struct MappingTraits<clang::format::FormatStyle> {
     IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
                    Style.ConstructorInitializerAllOnOneLineOrOnePerLine);
     IO.mapOptional("DerivePointerBinding", Style.DerivePointerBinding);
+    IO.mapOptional("ExperimentalAutoDetectBinPacking",
+                   Style.ExperimentalAutoDetectBinPacking);
     IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
     IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
     IO.mapOptional("ObjCSpaceBeforeProtocolList",
@@ -134,6 +136,7 @@ FormatStyle getLLVMStyle() {
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
   LLVMStyle.DerivePointerBinding = false;
+  LLVMStyle.ExperimentalAutoDetectBinPacking = false;
   LLVMStyle.IndentCaseLabels = false;
   LLVMStyle.MaxEmptyLinesToKeep = 1;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
@@ -165,6 +168,7 @@ FormatStyle getGoogleStyle() {
   GoogleStyle.ColumnLimit = 80;
   GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
   GoogleStyle.DerivePointerBinding = true;
+  GoogleStyle.ExperimentalAutoDetectBinPacking = false;
   GoogleStyle.IndentCaseLabels = true;
   GoogleStyle.MaxEmptyLinesToKeep = 1;
   GoogleStyle.ObjCSpaceBeforeProtocolList = false;
@@ -260,10 +264,12 @@ public:
                          const AnnotatedLine &Line, unsigned FirstIndent,
                          const FormatToken *RootToken,
                          WhitespaceManager &Whitespaces,
-                         encoding::Encoding Encoding)
+                         encoding::Encoding Encoding,
+                         bool BinPackInconclusiveFunctions)
       : Style(Style), SourceMgr(SourceMgr), Line(Line),
         FirstIndent(FirstIndent), RootToken(RootToken),
-        Whitespaces(Whitespaces), Count(0), Encoding(Encoding) {}
+        Whitespaces(Whitespaces), Count(0), Encoding(Encoding),
+        BinPackInconclusiveFunctions(BinPackInconclusiveFunctions) {}
 
   /// \brief Formats an \c UnwrappedLine.
   void format(const AnnotatedLine *NextLine) {
@@ -782,7 +788,11 @@ private:
       } else {
         NewIndent =
             4 + std::max(LastSpace, State.Stack.back().StartOfFunctionCall);
-        AvoidBinPacking = !Style.BinPackParameters;
+        AvoidBinPacking = !Style.BinPackParameters ||
+                          (Style.ExperimentalAutoDetectBinPacking &&
+                           (Current.PackingKind == PPK_OnePerLine ||
+                            (!BinPackInconclusiveFunctions &&
+                             Current.PackingKind == PPK_Inconclusive)));
       }
 
       State.Stack.push_back(ParenState(NewIndent, LastSpace, AvoidBinPacking,
@@ -1157,6 +1167,7 @@ private:
   // to create a deterministic order independent of the container.
   unsigned Count;
   encoding::Encoding Encoding;
+  bool BinPackInconclusiveFunctions;
 };
 
 class FormatTokenLexer {
@@ -1363,7 +1374,8 @@ public:
               1;
         }
         UnwrappedLineFormatter Formatter(Style, SourceMgr, TheLine, Indent,
-                                         TheLine.First, Whitespaces, Encoding);
+                                         TheLine.First, Whitespaces, Encoding,
+                                         BinPackInconclusiveFunctions);
         Formatter.format(I + 1 != E ? &*(I + 1) : NULL);
         IndentForLevel[TheLine.Level] = LevelIndent;
         PreviousLineWasTouched = true;
@@ -1408,6 +1420,8 @@ private:
     unsigned CountBoundToVariable = 0;
     unsigned CountBoundToType = 0;
     bool HasCpp03IncompatibleFormat = false;
+    bool HasBinPackedFunction = false;
+    bool HasOnePerLineFunction = false;
     for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
       if (!AnnotatedLines[i].First->Next)
         continue;
@@ -1428,6 +1442,12 @@ private:
             Tok->Previous->Type == TT_TemplateCloser &&
             Tok->WhitespaceRange.getBegin() == Tok->WhitespaceRange.getEnd())
           HasCpp03IncompatibleFormat = true;
+
+        if (Tok->PackingKind == PPK_BinPacked)
+          HasBinPackedFunction = true;
+        if (Tok->PackingKind == PPK_OnePerLine)
+          HasOnePerLineFunction = true;
+
         Tok = Tok->Next;
       }
     }
@@ -1441,6 +1461,8 @@ private:
       Style.Standard = HasCpp03IncompatibleFormat ? FormatStyle::LS_Cpp11
                                                   : FormatStyle::LS_Cpp03;
     }
+    BinPackInconclusiveFunctions =
+        HasBinPackedFunction || !HasOnePerLineFunction;
   }
 
   /// \brief Get the indent of \p Level from \p IndentForLevel.
@@ -1691,6 +1713,7 @@ private:
   std::vector<AnnotatedLine> AnnotatedLines;
 
   encoding::Encoding Encoding;
+  bool BinPackInconclusiveFunctions;
 };
 
 } // end anonymous namespace
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 1de4b8964fe..03f097000f4 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -64,6 +64,13 @@ enum BraceBlockKind {
   BK_BracedInit
 };
 
+// The packing kind of a function's parameters.
+enum ParameterPackingKind {
+  PPK_BinPacked,
+  PPK_OnePerLine,
+  PPK_Inconclusive
+};
+
 /// \brief A wrapper around a \c Token storing information about the
 /// whitespace characters preceeding it.
 struct FormatToken {
@@ -72,9 +79,9 @@ struct FormatToken {
         CodePointCount(0), IsFirst(false), MustBreakBefore(false),
         BlockKind(BK_Unknown), Type(TT_Unknown), SpacesRequiredBefore(0),
         CanBreakBefore(false), ClosesTemplateDeclaration(false),
-        ParameterCount(0), TotalLength(0), UnbreakableTailLength(0),
-        BindingStrength(0), SplitPenalty(0), LongestObjCSelectorName(0),
-        FakeRParens(0), LastInChainOfCalls(false),
+        ParameterCount(0), PackingKind(PPK_Inconclusive), TotalLength(0),
+        UnbreakableTailLength(0), BindingStrength(0), SplitPenalty(0),
+        LongestObjCSelectorName(0), FakeRParens(0), LastInChainOfCalls(false),
         PartOfMultiVariableDeclStmt(false), MatchingParen(NULL), Previous(NULL),
         Next(NULL) {}
 
@@ -143,6 +150,9 @@ struct FormatToken {
   /// the number of commas.
   unsigned ParameterCount;
 
+  /// \brief If this is an opening parenthesis, how are the parameters packed?
+  ParameterPackingKind PackingKind;
+
   /// \brief The total length of the line up to and including this token.
   unsigned TotalLength;
 
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index ce2db6d9c0b..f5b6fbde7c9 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -99,6 +99,8 @@ private:
     }
 
     bool MightBeFunctionType = CurrentToken->is(tok::star);
+    bool HasMultipleLines = false;
+    bool HasMultipleParametersOnALine = false;
     while (CurrentToken != NULL) {
       // LookForDecls is set when "if (" has been seen. Check for
       // 'identifier' '*' 'identifier' followed by not '=' -- this
@@ -133,6 +135,13 @@ private:
           }
         }
 
+        if (!HasMultipleLines)
+          Left->PackingKind = PPK_Inconclusive;
+        else if (HasMultipleParametersOnALine)
+          Left->PackingKind = PPK_BinPacked;
+        else
+          Left->PackingKind = PPK_OnePerLine;
+
         next();
         return true;
       }
@@ -143,8 +152,14 @@ private:
                                                     tok::coloncolon))
         MightBeFunctionType = true;
       updateParameterCount(Left, CurrentToken);
+      if (CurrentToken->is(tok::comma) && CurrentToken->Next &&
+          !CurrentToken->Next->HasUnescapedNewline &&
+          !CurrentToken->Next->isTrailingComment())
+        HasMultipleParametersOnALine = true;
       if (!consumeToken())
         return false;
+      if (CurrentToken && CurrentToken->HasUnescapedNewline)
+        HasMultipleLines = true;
     }
     return false;
   }
@@ -245,10 +260,11 @@ private:
   }
 
   void updateParameterCount(FormatToken *Left, FormatToken *Current) {
-    if (Current->is(tok::comma))
+    if (Current->is(tok::comma)) {
       ++Left->ParameterCount;
-    else if (Left->ParameterCount == 0 && Current->isNot(tok::comment))
+    } else if (Left->ParameterCount == 0 && Current->isNot(tok::comment)) {
       Left->ParameterCount = 1;
+    }
   }
 
   bool parseConditional() {
@@ -1283,9 +1299,10 @@ void TokenAnnotator::printDebugInfo(const AnnotatedLine &Line) {
   const FormatToken *Tok = Line.First;
   while (Tok) {
     llvm::errs() << " M=" << Tok->MustBreakBefore
-                 << " C=" << Tok->CanBreakBefore << " T=" << Tok->Type << " S="
-                 << Tok->SpacesRequiredBefore << " P=" << Tok->SplitPenalty
-                 << " Name=" << Tok->Tok.getName() << " FakeLParens=";
+                 << " C=" << Tok->CanBreakBefore << " T=" << Tok->Type
+                 << " S=" << Tok->SpacesRequiredBefore
+                 << " P=" << Tok->SplitPenalty << " Name=" << Tok->Tok.getName()
+                 << " PPK=" << Tok->PackingKind << " FakeLParens=";
     for (unsigned i = 0, e = Tok->FakeLParens.size(); i != e; ++i)
       llvm::errs() << Tok->FakeLParens[i] << "/";
     llvm::errs() << " FakeRParens=" << Tok->FakeRParens << "\n";