clang-format: Revamp nested block formatting.

This fixed llvm.org/PR21804 and hopefully a few other strange cases.

Before:
    if (blah_blah(whatever, whatever, [] {
      doo_dah();
      doo_dah();
    })) {
    }
    }

After:
    if (blah_blah(whatever, whatever, [] {
          doo_dah();
          doo_dah();
        })) {
    }
    }

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

diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index 8a57b09f5d48254de87754c0dc28d0706c23fb7a..e709cfbfc8512f206f5185c80091c7c3a9ba82b8 100644 (file)
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -324,25 +324,27 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
 
   State.Column += Spaces;
   if (Current.isNot(tok::comment) && Previous.is(tok::l_paren) &&
-      Previous.Previous && Previous.Previous->isOneOf(tok::kw_if, tok::kw_for))
+      Previous.Previous &&
+      Previous.Previous->isOneOf(tok::kw_if, tok::kw_for)) {
     // Treat the condition inside an if as if it was a second function
     // parameter, i.e. let nested calls have a continuation indent.
     State.Stack.back().LastSpace = State.Column;
-  else if (!Current.isOneOf(tok::comment, tok::caret) &&
-           (Previous.is(tok::comma) ||
-            (Previous.is(tok::colon) && Previous.is(TT_ObjCMethodExpr))))
+    State.Stack.back().NestedBlockIndent = State.Column;
+  } else if (!Current.isOneOf(tok::comment, tok::caret) &&
+             (Previous.is(tok::comma) ||
+              (Previous.is(tok::colon) && Previous.is(TT_ObjCMethodExpr)))) {
     State.Stack.back().LastSpace = State.Column;
-  else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
-                             TT_CtorInitializerColon)) &&
-           ((Previous.getPrecedence() != prec::Assignment &&
-             (Previous.isNot(tok::lessless) || Previous.OperatorIndex != 0 ||
-              !Previous.LastOperator)) ||
-            Current.StartsBinaryExpression))
+  } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
+                               TT_CtorInitializerColon)) &&
+             ((Previous.getPrecedence() != prec::Assignment &&
+               (Previous.isNot(tok::lessless) || Previous.OperatorIndex != 0 ||
+                !Previous.LastOperator)) ||
+              Current.StartsBinaryExpression)) {
     // Always indent relative to the RHS of the expression unless this is a
     // simple assignment without binary expression on the RHS. Also indent
     // relative to unary operators and the colons of constructor initializers.
     State.Stack.back().LastSpace = State.Column;
-  else if (Previous.is(TT_InheritanceColon)) {
+  } else if (Previous.is(TT_InheritanceColon)) {
     State.Stack.back().Indent = State.Column;
     State.Stack.back().LastSpace = State.Column;
   } else if (Previous.opensScope()) {
@@ -393,6 +395,7 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
     Penalty += Style.PenaltyBreakFirstLessLess;
 
   State.Column = getNewLineColumn(State);
+  State.Stack.back().NestedBlockIndent = State.Column;
   if (NextNonComment->isMemberAccess()) {
     if (State.Stack.back().CallContinuation == 0)
       State.Stack.back().CallContinuation = State.Column;
@@ -513,12 +516,10 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
     return Current.NestingLevel == 0 ? State.FirstIndent
                                      : State.Stack.back().Indent;
   if (Current.isOneOf(tok::r_brace, tok::r_square)) {
-    if (State.Stack.size() > 1 &&
-        State.Stack[State.Stack.size() - 2].NestedBlockInlined)
-      return State.FirstIndent;
-    if (Current.closesBlockTypeList(Style) ||
-        (Current.MatchingParen &&
-         Current.MatchingParen->BlockKind == BK_BracedInit))
+    if (Current.closesBlockTypeList(Style))
+      return State.Stack[State.Stack.size() - 2].NestedBlockIndent;
+    if (Current.MatchingParen &&
+        Current.MatchingParen->BlockKind == BK_BracedInit)
       return State.Stack[State.Stack.size() - 2].LastSpace;
     return State.FirstIndent;
   }
@@ -664,7 +665,7 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
       !Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
     State.Stack.back().NestedBlockInlined =
         !Newline &&
-        (Previous->isNot(tok::l_paren) || Previous->ParameterCount > 1) && !Newline;
+        (Previous->isNot(tok::l_paren) || Previous->ParameterCount > 1);
   }
 
   moveStatePastFakeLParens(State, Newline);
@@ -777,9 +778,8 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
   }
 }
 
-// Remove the fake r_parens after 'Tok'.
-static void consumeRParens(LineState& State, const FormatToken &Tok) {
-  for (unsigned i = 0, e = Tok.FakeRParens; i != e; ++i) {
+void ContinuationIndenter::moveStatePastFakeRParens(LineState &State) {
+  for (unsigned i = 0, e = State.NextToken->FakeRParens; i != e; ++i) {
     unsigned VariablePos = State.Stack.back().VariablePos;
     assert(State.Stack.size() > 1);
     if (State.Stack.size() == 1) {
@@ -791,45 +791,6 @@ static void consumeRParens(LineState& State, const FormatToken &Tok) {
   }
 }
 
-// Returns whether 'Tok' opens or closes a scope requiring special handling
-// of the subsequent fake r_parens.
-//
-// For example, if this is an l_brace starting a nested block, we pretend (wrt.
-// to indentation) that we already consumed the corresponding r_brace. Thus, we
-// remove all ParenStates caused by fake parentheses that end at the r_brace.
-// The net effect of this is that we don't indent relative to the l_brace, if
-// the nested block is the last parameter of a function. This formats:
-//
-//   SomeFunction(a, [] {
-//     f();  // break
-//   });
-//
-// instead of:
-//   SomeFunction(a, [] {
-//                     f();  // break
-//                   });
-static bool fakeRParenSpecialCase(const LineState &State) {
-  const FormatToken &Tok = *State.NextToken;
-  if (!Tok.MatchingParen)
-    return false;
-  const FormatToken *Left = &Tok;
-  if (Tok.isOneOf(tok::r_brace, tok::r_square))
-    Left = Tok.MatchingParen;
-  return !State.Stack.back().HasMultipleNestedBlocks &&
-         Left->isOneOf(tok::l_brace, tok::l_square) &&
-         (Left->BlockKind == BK_Block ||
-          Left->isOneOf(TT_ArrayInitializerLSquare, TT_DictLiteral));
-}
-
-void ContinuationIndenter::moveStatePastFakeRParens(LineState &State) {
-  // Don't remove FakeRParens attached to r_braces that surround nested blocks
-  // as they will have been removed early (see above).
-  if (fakeRParenSpecialCase(State))
-    return;
-
-  consumeRParens(State, *State.NextToken);
-}
-
 void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
                                                     bool Newline) {
   const FormatToken &Current = *State.NextToken;
@@ -846,16 +807,12 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
   bool AvoidBinPacking;
   bool BreakBeforeParameter = false;
   if (Current.isOneOf(tok::l_brace, TT_ArrayInitializerLSquare)) {
-    if (fakeRParenSpecialCase(State))
-      consumeRParens(State, *Current.MatchingParen);
-
-    NewIndent = State.Stack.back().LastSpace;
     if (Current.opensBlockTypeList(Style)) {
-      NewIndent += Style.IndentWidth;
+      NewIndent = State.Stack.back().NestedBlockIndent + Style.IndentWidth;
       NewIndent = std::min(State.Column + 2, NewIndent);
       ++NewIndentLevel;
     } else {
-      NewIndent += Style.ContinuationIndentWidth;
+      NewIndent = State.Stack.back().LastSpace + Style.ContinuationIndentWidth;
       NewIndent = std::min(State.Column + 1, NewIndent);
     }
     const FormatToken *NextNoComment = Current.getNextNonComment();
@@ -884,9 +841,11 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
   bool NoLineBreak = State.Stack.back().NoLineBreak ||
                      (Current.is(TT_TemplateOpener) &&
                       State.Stack.back().ContainsUnwrappedBuilder);
+  unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent;
   State.Stack.push_back(ParenState(NewIndent, NewIndentLevel,
                                    State.Stack.back().LastSpace,
                                    AvoidBinPacking, NoLineBreak));
+  State.Stack.back().NestedBlockIndent = NestedBlockIndent;
   State.Stack.back().BreakBeforeParameter = BreakBeforeParameter;
   State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount > 1;
 }
@@ -913,20 +872,17 @@ void ContinuationIndenter::moveStatePastScopeCloser(LineState &State) {
 }
 
 void ContinuationIndenter::moveStateToNewBlock(LineState &State) {
-  // If we have already found more than one lambda introducers on this level, we
-  // opt out of this because similarity between the lambdas is more important.
-  if (fakeRParenSpecialCase(State))
-    consumeRParens(State, *State.NextToken->MatchingParen);
-
+  unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent;
   // ObjC block sometimes follow special indentation rules.
   unsigned NewIndent =
-      State.Stack.back().LastSpace + (State.NextToken->is(TT_ObjCBlockLBrace)
-                                          ? Style.ObjCBlockIndentWidth
-                                          : Style.IndentWidth);
+      NestedBlockIndent + (State.NextToken->is(TT_ObjCBlockLBrace)
+                               ? Style.ObjCBlockIndentWidth
+                               : Style.IndentWidth);
   State.Stack.push_back(ParenState(
       NewIndent, /*NewIndentLevel=*/State.Stack.back().IndentLevel + 1,
       State.Stack.back().LastSpace, /*AvoidBinPacking=*/true,
       State.Stack.back().NoLineBreak));
+  State.Stack.back().NestedBlockIndent = NestedBlockIndent;
   State.Stack.back().BreakBeforeParameter = true;
 }
 

diff --git a/lib/Format/ContinuationIndenter.h b/lib/Format/ContinuationIndenter.h
index 004ddebac4a26f4411054a7d3a3e5fef6b7464dd..36691d945b4faa6228ce1db0f12a56ef7fdc6b8d 100644 (file)
--- a/lib/Format/ContinuationIndenter.h
+++ b/lib/Format/ContinuationIndenter.h
@@ -148,7 +148,8 @@ struct ParenState {
   ParenState(unsigned Indent, unsigned IndentLevel, unsigned LastSpace,
              bool AvoidBinPacking, bool NoLineBreak)
       : Indent(Indent), IndentLevel(IndentLevel), LastSpace(LastSpace),
-        FirstLessLess(0), BreakBeforeClosingBrace(false), QuestionColumn(0),
+        NestedBlockIndent(Indent), FirstLessLess(0),
+        BreakBeforeClosingBrace(false), QuestionColumn(0),
         AvoidBinPacking(AvoidBinPacking), BreakBeforeParameter(false),
         NoLineBreak(NoLineBreak), LastOperatorWrapped(true), ColonPos(0),
         StartOfFunctionCall(0), StartOfArraySubscripts(0),
@@ -171,6 +172,10 @@ struct ParenState {
   ///                             OtherParameter));
   unsigned LastSpace;
 
+  /// \brief If a block relative to this parenthesis level gets wrapped, indent
+  /// it this much.
+  unsigned NestedBlockIndent;
+
   /// \brief The position the first "<<" operator encountered on each level.
   ///
   /// Used to align "<<" operators. 0 if no such operator has been encountered
@@ -265,6 +270,8 @@ struct ParenState {
       return Indent < Other.Indent;
     if (LastSpace != Other.LastSpace)
       return LastSpace < Other.LastSpace;
+    if (NestedBlockIndent != Other.NestedBlockIndent)
+      return NestedBlockIndent < Other.NestedBlockIndent;
     if (FirstLessLess != Other.FirstLessLess)
       return FirstLessLess < Other.FirstLessLess;
     if (BreakBeforeClosingBrace != Other.BreakBeforeClosingBrace)

diff --git a/lib/Format/FormatToken.h b/lib/Format/FormatToken.h
index df4742101225e1eb32ee1bd9a7885fd5f82afe46..7ffbfbd4eba067217029eab7dd047db2bcd0b6f9 100644 (file)
--- a/lib/Format/FormatToken.h
+++ b/lib/Format/FormatToken.h
@@ -409,8 +409,9 @@ struct FormatToken {
   /// list that should be indented with a block indent.
   bool opensBlockTypeList(const FormatStyle &Style) const {
     return is(TT_ArrayInitializerLSquare) ||
-           (is(tok::l_brace) && (BlockKind == BK_Block || is(TT_DictLiteral) ||
-                                 !Style.Cpp11BracedListStyle));
+           (is(tok::l_brace) &&
+            (BlockKind == BK_Block || is(TT_DictLiteral) ||
+             (!Style.Cpp11BracedListStyle && NestingLevel == 0)));
   }
 
   /// \brief Same as opensBlockTypeList, but for the closing token.

diff --git a/lib/Format/UnwrappedLineFormatter.cpp b/lib/Format/UnwrappedLineFormatter.cpp
index fe9cf5939995709098d20b2890a8fe30f2bf4570..56feaf6e54ef192dc36f09a3d1e919e4a41785db 100644 (file)
--- a/lib/Format/UnwrappedLineFormatter.cpp
+++ b/lib/Format/UnwrappedLineFormatter.cpp
@@ -593,6 +593,15 @@ unsigned UnwrappedLineFormatter::analyzeSolutionSpace(LineState &InitialState,
   return Penalty;
 }
 
+static void printLineState(const LineState &State) {
+  llvm::dbgs() << "State: ";
+  for (const ParenState &P : State.Stack) {
+    llvm::dbgs() << P.Indent << "|" << P.LastSpace << "|" << P.NestedBlockIndent
+                 << " ";
+  }
+  llvm::dbgs() << State.NextToken->TokenText << "\n";
+}
+
 void UnwrappedLineFormatter::reconstructPath(LineState &State,
                                              StateNode *Current) {
   std::deque<StateNode *> Path;
@@ -608,6 +617,7 @@ void UnwrappedLineFormatter::reconstructPath(LineState &State,
     Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
 
     DEBUG({
+      printLineState((*I)->Previous->State);
       if ((*I)->NewLine) {
         llvm::dbgs() << "Penalty for placing "
                      << (*I)->Previous->State.NextToken->Tok.getName() << ": "
@@ -648,13 +658,8 @@ bool UnwrappedLineFormatter::formatChildren(LineState &State, bool NewLine,
     return true;
 
   if (NewLine) {
-    int AdditionalIndent =
-        State.FirstIndent - State.Line->Level * Style.IndentWidth;
-    if (State.Stack.size() < 2 ||
-        !State.Stack[State.Stack.size() - 2].NestedBlockInlined) {
-      AdditionalIndent = State.Stack.back().Indent -
-                         Previous.Children[0]->Level * Style.IndentWidth;
-    }
+    int AdditionalIndent = State.Stack.back().Indent -
+                           Previous.Children[0]->Level * Style.IndentWidth;
 
     Penalty += format(Previous.Children, DryRun, AdditionalIndent,
                       /*FixBadIndentation=*/true);

diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index fdcfe79d135770163dfc75ab38f3ae3381144fb4..2b4553cefcd14510287635174f0640f26c37ba82 100644 (file)
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -5912,8 +5912,7 @@ TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
       "             BracedList{ // comment 1 (Forcing interesting break)\n"
       "                         param1, param2,\n"
       "                         // comment 2\n"
-      "                         param3, param4\n"
-      "             });",
+      "                         param3, param4 });",
       ExtraSpaces);
   verifyFormat(
       "std::this_thread::sleep_for(\n"
@@ -9421,6 +9420,12 @@ TEST_F(FormatTest, FormatsLambdas) {
   verifyFormat("void f() {\n"
                "  MACRO((const AA &a) { return 1; });\n"
                "}");
+
+  verifyFormat("if (blah_blah(whatever, whatever, [] {\n"
+               "      doo_dah();\n"
+               "      doo_dah();\n"
+               "    })) {\n"
+               "}");
 }
 
 TEST_F(FormatTest, FormatsBlocks) {

diff --git a/unittests/Format/FormatTestJS.cpp b/unittests/Format/FormatTestJS.cpp
index 23654a88483c82e5212a73cd746f1005cab50abd..1d12d32044a6a1e84203ec5e1b83f0b4aacb6b4b 100644 (file)
--- a/unittests/Format/FormatTestJS.cpp
+++ b/unittests/Format/FormatTestJS.cpp
@@ -206,14 +206,13 @@ TEST_F(FormatTestJS, FunctionLiterals) {
                "    style: {direction: ''}\n"
                "  }\n"
                "};");
-  // FIXME: The formatting here probably isn't ideal.
   EXPECT_EQ("abc = xyz ?\n"
             "          function() {\n"
             "            return 1;\n"
             "          } :\n"
             "          function() {\n"
-            "  return -1;\n"
-            "};",
+            "            return -1;\n"
+            "          };",
             format("abc=xyz?function(){return 1;}:function(){return -1;};"));
 
   verifyFormat("var closure = goog.bind(\n"
@@ -254,6 +253,23 @@ TEST_F(FormatTestJS, FunctionLiterals) {
                "    return 1;\n"
                "  }\n"
                "};");
+  verifyFormat("this.someObject.doSomething(aaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
+               "    .then(goog.bind(function(aaaaaaaaaaa) {\n"
+               "      someFunction();\n"
+               "      someFunction();\n"
+               "    }, this), aaaaaaaaaaaaaaaaa);");
+
+  // FIXME: This is not ideal yet.
+  verifyFormat("someFunction(goog.bind(\n"
+               "                 function() {\n"
+               "                   doSomething();\n"
+               "                   doSomething();\n"
+               "                 },\n"
+               "                 this),\n"
+               "             goog.bind(function() {\n"
+               "               doSomething();\n"
+               "               doSomething();\n"
+               "             }, this));");
 }
 
 TEST_F(FormatTestJS, InliningFunctionLiterals) {
@@ -341,7 +357,10 @@ TEST_F(FormatTestJS, MultipleFunctionLiterals) {
 
   verifyFormat("getSomeLongPromise()\n"
                "    .then(function(value) { body(); })\n"
-               "    .thenCatch(function(error) { body(); });");
+               "    .thenCatch(function(error) {\n"
+               "      body();\n"
+               "      body();\n"
+               "    });");
   verifyFormat("getSomeLongPromise()\n"
                "    .then(function(value) {\n"
                "      body();\n"
@@ -351,6 +370,11 @@ TEST_F(FormatTestJS, MultipleFunctionLiterals) {
                "      body();\n"
                "      body();\n"
                "    });");
+
+  // FIXME: This is bad, but it used to be formatted correctly by accident.
+  verifyFormat("getSomeLongPromise().then(function(value) {\n"
+               "  body();\n"
+               "}).thenCatch(function(error) { body(); });");
 }
 
 TEST_F(FormatTestJS, ReturnStatements) {