Several small changes in formatting decisions.

1. Use a hanging ident for function calls nested in binary expressions.
   E.g.:
   int aaaaa = aaaaaaaaa && aaaaaaaaaa(
                                aaaaaaaaaa);

2. Slightly improve heuristic for builder type expressions and reduce
   penalty for breaking before "." and "->" in those.

3. Remove  mostly obsolete metric of decreasing indent level. This
   fixes: llvm.org/PR14931.

Changes #1 and #2 were necessary to keep tests passing after #3.

llvm-svn: 173680

3 files changed, 20 insertions, 28 deletions

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 99b2d545c2a..089035a3560 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -158,6 +158,11 @@ static prec::Level getPrecedence(const AnnotatedToken &Tok) {
   return getBinOpPrecedence(Tok.FormatTok.Tok.getKind(), true, true);
 }
 
+bool isBinaryOperator(const AnnotatedToken &Tok) {
+  // Comma is a binary operator, but does not behave as such wrt. formatting.
+  return getPrecedence(Tok) > prec::Comma;
+}
+
 FormatStyle getLLVMStyle() {
   FormatStyle LLVMStyle;
   LLVMStyle.ColumnLimit = 80;
@@ -200,7 +205,6 @@ FormatStyle getChromiumStyle() {
 
 struct OptimizationParameters {
   unsigned PenaltyIndentLevel;
-  unsigned PenaltyLevelDecrease;
   unsigned PenaltyExcessCharacter;
 };
 
@@ -345,7 +349,6 @@ public:
         FirstIndent(FirstIndent), RootToken(RootToken),
         Whitespaces(Whitespaces) {
     Parameters.PenaltyIndentLevel = 20;
-    Parameters.PenaltyLevelDecrease = 30;
     Parameters.PenaltyExcessCharacter = 1000000;
   }
 
@@ -361,7 +364,6 @@ public:
     State.Stack.push_back(ParenState(FirstIndent + 4, FirstIndent));
     State.ForLoopVariablePos = 0;
     State.LineContainsContinuedForLoopSection = false;
-    State.StartOfLineLevel = 1;
 
     DEBUG({
       DebugTokenState(*State.NextToken);
@@ -493,9 +495,6 @@ private:
     /// \brief The token that needs to be next formatted.
     const AnnotatedToken *NextToken;
 
-    /// \brief The parenthesis level of the first token on the current line.
-    unsigned StartOfLineLevel;
-
     /// \brief The column of the first variable in a for-loop declaration.
     ///
     /// Used to align the second variable if necessary.
@@ -514,8 +513,6 @@ private:
         return Other.NextToken > NextToken;
       if (Other.Column != Column)
         return Other.Column > Column;
-      if (Other.StartOfLineLevel != StartOfLineLevel)
-        return Other.StartOfLineLevel > StartOfLineLevel;
       if (Other.ForLoopVariablePos != ForLoopVariablePos)
         return Other.ForLoopVariablePos < ForLoopVariablePos;
       if (Other.LineContainsContinuedForLoopSection !=
@@ -569,10 +566,6 @@ private:
         State.Column = State.Stack[ParenLevel].Indent;
       }
 
-      // A line starting with a closing brace is assumed to be correct for the
-      // same level as before the opening brace.
-      State.StartOfLineLevel = ParenLevel + (Current.is(tok::r_brace) ? 0 : 1);
-
       if (RootToken.is(tok::kw_for))
         State.LineContainsContinuedForLoopSection = Previous.isNot(tok::semi);
 
@@ -622,6 +615,9 @@ private:
       else if (Previous.is(tok::comma) && ParenLevel != 0)
         // Top-level spaces are exempt as that mostly leads to better results.
         State.Stack.back().LastSpace = State.Column;
+      else if (Previous.Type == TT_BinaryOperator &&
+               getPrecedence(Previous) != prec::Assignment)
+        State.Stack.back().LastSpace = State.Column;
       else if (Previous.ParameterCount > 1 &&
                (Previous.is(tok::l_paren) || Previous.is(tok::l_square) ||
                 Previous.Type == TT_TemplateOpener))
@@ -739,7 +735,7 @@ private:
 
     if (Right.is(tok::arrow) || Right.is(tok::period)) {
       if (Left.is(tok::r_paren) && Line.Type == LT_BuilderTypeCall)
-        return 15; // Should be smaller than breaking at a nested comma.
+        return 5; // Should be smaller than breaking at a nested comma.
       return 150;
     }
 
@@ -792,15 +788,9 @@ private:
       return UINT_MAX;
 
     unsigned CurrentPenalty = 0;
-    if (NewLine) {
+    if (NewLine)
       CurrentPenalty += Parameters.PenaltyIndentLevel * State.Stack.size() +
                         splitPenalty(*State.NextToken->Parent);
-    } else {
-      if (State.Stack.size() < State.StartOfLineLevel &&
-          State.NextToken->is(tok::identifier))
-        CurrentPenalty += Parameters.PenaltyLevelDecrease *
-                          (State.StartOfLineLevel - State.Stack.size());
-    }
 
     addTokenToState(NewLine, true, State);
 
@@ -1200,6 +1190,7 @@ public:
 
     LineType parseLine() {
       int PeriodsAndArrows = 0;
+      bool CanBeBuilderTypeStmt = true;
       if (CurrentToken->is(tok::hash)) {
         parsePreprocessorDirective();
         return LT_PreprocessorDirective;
@@ -1210,6 +1201,9 @@ public:
           KeywordVirtualFound = true;
         if (CurrentToken->is(tok::period) || CurrentToken->is(tok::arrow))
           ++PeriodsAndArrows;
+        if (getPrecedence(*CurrentToken) > prec::Assignment &&
+            CurrentToken->isNot(tok::less) && CurrentToken->isNot(tok::greater))
+          CanBeBuilderTypeStmt = false;
         if (!consumeToken())
           return LT_Invalid;
       }
@@ -1217,7 +1211,7 @@ public:
         return LT_VirtualFunctionDecl;
 
       // Assume a builder-type call if there are 2 or more "." and "->".
-      if (PeriodsAndArrows >= 2)
+      if (PeriodsAndArrows >= 2 && CanBeBuilderTypeStmt)
         return LT_BuilderTypeCall;
 
       return LT_Other;
@@ -1354,11 +1348,6 @@ private:
       determineTokenTypes(Current.Children[0], IsExpression);
   }
 
-  bool isBinaryOperator(const AnnotatedToken &Tok) {
-    // Comma is a binary operator, but does not behave as such wrt. formatting.
-    return getPrecedence(Tok) > prec::Comma;
-  }
-
   /// \brief Returns the previous token ignoring comments.
   const AnnotatedToken *getPreviousToken(const AnnotatedToken &Tok) {
     const AnnotatedToken *PrevToken = Tok.Parent;
diff --git a/clang/test/Index/overriding-ftemplate-comments.cpp b/clang/test/Index/overriding-ftemplate-comments.cpp
index f5f45be16c6..0bc3c2f0ae8 100644
--- a/clang/test/Index/overriding-ftemplate-comments.cpp
+++ b/clang/test/Index/overriding-ftemplate-comments.cpp
@@ -82,5 +82,5 @@ void comment_to_html_conversion_22();
 template<class CCC1, template<class CCC2, template<class CCC3, class CCC4> class QQQ> class PPP>
 void comment_to_html_conversion_22();
 
-// CHECK: FullCommentAsXML=[<Function templateKind="template" file="{{[^"]+}}overriding-ftemplate-comments.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_22</Name><USR>c:@FT@&gt;2#T#t&gt;2#T#t&gt;2#T#Tcomment_to_html_conversion_22#</USR><Declaration>template &lt;class CCC1,\n          template &lt;class CCC2, template &lt;class CCC3, class CCC4&gt; class QQQ&gt;\n      class PPP&gt;\nvoid comment_to_html_conversion_22()</Declaration><TemplateParameters><Parameter><Name>CCC1</Name><Index>0</Index><Discussion><Para> Ccc 1 </Para></Discussion></Parameter><Parameter><Name>PPP</Name><Index>1</Index><Discussion><Para> Zzz </Para></Discussion></Parameter><Parameter><Name>CCC2</Name><Discussion><Para> Ccc 2 </Para></Discussion></Parameter><Parameter><Name>CCC3</Name><Discussion><Para> Ccc 3 </Para></Discussion></Parameter><Parameter><Name>CCC4</Name><Discussion><Para> Ccc 4 </Para></Discussion></Parameter><Parameter><Name>QQQ</Name><Discussion><Para> Bbb</Para></Discussion></Parameter></TemplateParameters></Function>]
+// CHECK: FullCommentAsXML=[<Function templateKind="template" file="{{[^"]+}}overriding-ftemplate-comments.cpp" line="[[@LINE-2]]" column="6"><Name>comment_to_html_conversion_22</Name><USR>c:@FT@&gt;2#T#t&gt;2#T#t&gt;2#T#Tcomment_to_html_conversion_22#</USR><Declaration>template &lt;class CCC1, template &lt;class CCC2, template &lt;class CCC3, class CCC4&gt;\n                            class QQQ&gt; class PPP&gt;\nvoid comment_to_html_conversion_22()</Declaration><TemplateParameters><Parameter><Name>CCC1</Name><Index>0</Index><Discussion><Para> Ccc 1 </Para></Discussion></Parameter><Parameter><Name>PPP</Name><Index>1</Index><Discussion><Para> Zzz </Para></Discussion></Parameter><Parameter><Name>CCC2</Name><Discussion><Para> Ccc 2 </Para></Discussion></Parameter><Parameter><Name>CCC3</Name><Discussion><Para> Ccc 3 </Para></Discussion></Parameter><Parameter><Name>CCC4</Name><Discussion><Para> Ccc 4 </Para></Discussion></Parameter><Parameter><Name>QQQ</Name><Discussion><Para> Bbb</Para></Discussion></Parameter></TemplateParameters></Function>]
 
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index e47c7a5196c..57252f547d6 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -856,7 +856,7 @@ TEST_F(FormatTest, FormatsAwesomeMethodCall) {
       "                   SecondLongCall(parameter));");
 }
 
-TEST_F(FormatTest, HigherIndentsForDeeperNestedParameters) {
+TEST_F(FormatTest, PreventConfusingIndents) {
   verifyFormat(
       "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
       "    aaaaaaaaaaaaaaaaaaaaaaaa(\n"
@@ -872,6 +872,9 @@ TEST_F(FormatTest, HigherIndentsForDeeperNestedParameters) {
       "    aaaaaaaaaaaaaaaaaaaaaaaa<\n"
       "        aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>,\n"
       "    aaaaaaaaaaaaaaaaaaaaaaaa>;");
+  verifyFormat("int a = bbbb && ccc && fffff(\n"
+               "#define A Just forcing a new line\n"
+               "                           ddd);");
 }
 
 TEST_F(FormatTest, ConstructorInitializers) {