clang-format: Revamp builder-type call formatting.

Previously builder-type calls were only correctly recognized in
top-level calls.

This fixes llvm.org/PR16981.
Before:
  someobj->Add((new util::filetools::Handler(dir))->OnEvent1(
      NewPermanentCallback(this, &HandlerHolderClass::EventHandlerCBA))
                   ->OnEvent2(NewPermanentCallback(
                                  this, &HandlerHolderClass::EventHandlerCBB))
                   ->OnEvent3(NewPermanentCallback(
                                  this, &HandlerHolderClass::EventHandlerCBC))
                   ->OnEvent5(NewPermanentCallback(
                                  this, &HandlerHolderClass::EventHandlerCBD))
                   ->OnEvent6(NewPermanentCallback(
                         this, &HandlerHolderClass::EventHandlerCBE)));

After:
  someobj->Add((new util::filetools::Handler(dir))
                   ->OnEvent1(NewPermanentCallback(
                         this, &HandlerHolderClass::EventHandlerCBA))
                   ->OnEvent2(NewPermanentCallback(
                         this, &HandlerHolderClass::EventHandlerCBB))
                   ->OnEvent3(NewPermanentCallback(
                         this, &HandlerHolderClass::EventHandlerCBC))
                   ->OnEvent5(NewPermanentCallback(
                         this, &HandlerHolderClass::EventHandlerCBD))
                   ->OnEvent6(NewPermanentCallback(
                         this, &HandlerHolderClass::EventHandlerCBE)));

llvm-svn: 189337

clang/lib/Format/ContinuationIndenter.cpp

@@ -38,6 +38,15 @@ static unsigned getLengthToMatchingParen(const FormatToken &Tok) {
   return End->TotalLength - Tok.TotalLength + 1;
 }
 
+// Returns \c true if \c Tok starts a binary expression.
+static bool startsBinaryExpression(const FormatToken &Tok) {
+  for (unsigned i = 0, e = Tok.FakeLParens.size(); i != e; ++i) {
+    if (Tok.FakeLParens[i] > prec::Unknown)
+      return true;
+  }
+  return false;
+}
+
 ContinuationIndenter::ContinuationIndenter(const FormatStyle &Style,
                                            SourceManager &SourceMgr,
                                            const AnnotatedLine &Line,
@@ -372,8 +381,8 @@ unsigned ContinuationIndenter::addTokenToState(LineState &State, bool Newline,
               Previous.Type == TT_ConditionalExpr ||
               Previous.Type == TT_UnaryOperator ||
               Previous.Type == TT_CtorInitializerColon) &&
-             !(Previous.getPrecedence() == prec::Assignment &&
+             (Previous.getPrecedence() != prec::Assignment ||
-               Current.FakeLParens.empty()))
+              startsBinaryExpression(Current)))
       // 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.
@@ -395,7 +404,9 @@ unsigned ContinuationIndenter::addTokenToState(LineState &State, bool Newline,
         if (Next && Next->isOneOf(tok::period, tok::arrow))
           HasTrailingCall = true;
       }
-      if (HasMultipleParameters || HasTrailingCall)
+      if (HasMultipleParameters ||
+          (HasTrailingCall &&
+           State.Stack[State.Stack.size() - 2].CallContinuation == 0))
         State.Stack.back().LastSpace = State.Column;
     }
   }
@@ -420,8 +431,7 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
   if (!Current.opensScope() && !Current.closesScope())
     State.LowestLevelOnLine =
         std::min(State.LowestLevelOnLine, State.ParenLevel);
-  if (Current.isOneOf(tok::period, tok::arrow) &&
+  if (Current.isOneOf(tok::period, tok::arrow))
-      Line.Type == LT_BuilderTypeCall && State.ParenLevel == 0)
     State.Stack.back().StartOfFunctionCall =
         Current.LastInChainOfCalls ? 0 : State.Column + Current.CodePointCount;
   if (Current.Type == TT_CtorInitializerColon) {
@@ -545,7 +555,7 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
 
   State.Column += Current.CodePointCount;
   State.NextToken = State.NextToken->Next;
-  unsigned Penalty =  breakProtrudingToken(Current, State, DryRun);
+  unsigned Penalty = breakProtrudingToken(Current, State, 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

clang/lib/Format/TokenAnnotator.cpp

@@ -484,9 +484,6 @@ private:
 
 public:
   LineType parseLine() {
-    int PeriodsAndArrows = 0;
-    FormatToken *LastPeriodOrArrow = NULL;
-    bool CanBeBuilderTypeStmt = true;
     if (CurrentToken->is(tok::hash)) {
       parsePreprocessorDirective();
       return LT_PreprocessorDirective;
@@ -494,26 +491,12 @@ public:
     while (CurrentToken != NULL) {
       if (CurrentToken->is(tok::kw_virtual))
         KeywordVirtualFound = true;
-      if (CurrentToken->isOneOf(tok::period, tok::arrow)) {
-        ++PeriodsAndArrows;
-        LastPeriodOrArrow = CurrentToken;
-      }
-      FormatToken *TheToken = CurrentToken;
       if (!consumeToken())
         return LT_Invalid;
-      if (TheToken->getPrecedence() > prec::Assignment &&
-          TheToken->Type == TT_BinaryOperator)
-        CanBeBuilderTypeStmt = false;
     }
     if (KeywordVirtualFound)
       return LT_VirtualFunctionDecl;
 
-    // Assume a builder-type call if there are 2 or more "." and "->".
-    if (PeriodsAndArrows >= 2 && CanBeBuilderTypeStmt) {
-      LastPeriodOrArrow->LastInChainOfCalls = true;
-      return LT_BuilderTypeCall;
-    }
-
     if (Line.First->Type == TT_ObjCMethodSpecifier) {
       if (Contexts.back().FirstObjCSelectorName != NULL)
         Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
@@ -841,6 +824,9 @@ private:
   IdentifierInfo &Ident_in;
 };
 
+static int PrecedenceUnaryOperator = prec::PointerToMember + 1;
+static int PrecedenceArrowAndPeriod = prec::PointerToMember + 2;
+
 /// \brief Parses binary expressions by inserting fake parenthesis based on
 /// operator precedence.
 class ExpressionParser {
@@ -853,24 +839,24 @@ public:
 
   /// \brief Parse expressions with the given operatore precedence.
   void parse(int Precedence = 0) {
-    if (Current == NULL)
+    if (Current == NULL || Precedence > PrecedenceArrowAndPeriod)
       return;
 
     // Conditional expressions need to be parsed separately for proper nesting.
-    if (Precedence == prec::Conditional + 1) {
+    if (Precedence == prec::Conditional) {
       parseConditionalExpr();
       return;
     }
 
     // Parse unary operators, which all have a higher precedence than binary
     // operators.
-    if (Precedence > prec::PointerToMember) {
+    if (Precedence == PrecedenceUnaryOperator) {
       parseUnaryOperator();
       return;
     }
 
     FormatToken *Start = Current;
-    bool OperatorFound = false;
+    FormatToken *LatestOperator = NULL;
 
     while (Current) {
       // Consume operators with higher precedence.
@@ -885,9 +871,16 @@ public:
       // At the end of the line or when an operator with higher precedence is
       // found, insert fake parenthesis and return.
       if (Current == NULL || Current->closesScope() ||
-          (CurrentPrecedence != 0 && CurrentPrecedence < Precedence)) {
+          (CurrentPrecedence != -1 && CurrentPrecedence < Precedence)) {
-        if (OperatorFound)
+        if (LatestOperator) {
-          addFakeParenthesis(Start, prec::Level(Precedence - 1));
+          if (Precedence == PrecedenceArrowAndPeriod) {
+            LatestOperator->LastInChainOfCalls = true;
+            // Call expressions don't have a binary operator precedence.
+            addFakeParenthesis(Start, prec::Unknown);
+          } else {
+            addFakeParenthesis(Start, prec::Level(Precedence));
+          }
+        }
         return;
       }
 
@@ -901,7 +894,7 @@ public:
       } else {
         // Operator found.
         if (CurrentPrecedence == Precedence)
-          OperatorFound = true;
+          LatestOperator = Current;
 
         next();
       }
@@ -914,15 +907,17 @@ private:
   int getCurrentPrecedence() {
     if (Current) {
       if (Current->Type == TT_ConditionalExpr)
-        return 1 + (int)prec::Conditional;
+        return prec::Conditional;
       else if (Current->is(tok::semi) || Current->Type == TT_InlineASMColon)
-        return 1;
+        return 0;
       else if (Current->Type == TT_BinaryOperator || Current->is(tok::comma))
-        return 1 + (int)Current->getPrecedence();
+        return Current->getPrecedence();
       else if (Current->Type == TT_ObjCSelectorName)
-        return 1 + (int)prec::Assignment;
+        return prec::Assignment;
+      else if (Current->isOneOf(tok::period, tok::arrow))
+        return PrecedenceArrowAndPeriod;
     }
-    return 0;
+    return -1;
   }
 
   void addFakeParenthesis(FormatToken *Start, prec::Level Precedence) {
@@ -934,36 +929,26 @@ private:
   /// \brief Parse unary operator expressions and surround them with fake
   /// parentheses if appropriate.
   void parseUnaryOperator() {
-    if (Current == NULL || Current->Type != TT_UnaryOperator)
+    if (Current == NULL || Current->Type != TT_UnaryOperator) {
+      parse(PrecedenceArrowAndPeriod);
       return;
+    }
 
     FormatToken *Start = Current;
     next();
+    parseUnaryOperator();
 
-    while (Current) {
-      if (Current->opensScope()) {
-        while (Current && !Current->closesScope()) {
-          next();
-          parse();
-        }
-        next();
-      } else if (getCurrentPrecedence() == 0 && !Current->closesScope()) {
-        next();
-      } else {
-        break;
-      }
-    }
     // The actual precedence doesn't matter.
-    addFakeParenthesis(Start, prec::Level(0));
+    addFakeParenthesis(Start, prec::Unknown);
   }
 
   void parseConditionalExpr() {
     FormatToken *Start = Current;
-    parse(prec::LogicalOr + 1);
+    parse(prec::LogicalOr);
     if (!Current || !Current->is(tok::question))
       return;
     next();
-    parse(prec::LogicalOr + 1);
+    parse(prec::LogicalOr);
     if (!Current || Current->Type != TT_ConditionalExpr)
       return;
     next();

clang/lib/Format/TokenAnnotator.h

@@ -28,7 +28,6 @@ namespace format {
 enum LineType {
   LT_Invalid,
   LT_Other,
-  LT_BuilderTypeCall,
   LT_PreprocessorDirective,
   LT_VirtualFunctionDecl,
   LT_ObjCDecl, // An @interface, @implementation, or @protocol line.

clang/unittests/Format/FormatTest.cpp

@@ -2835,10 +2835,11 @@ TEST_F(FormatTest, AdaptiveOnePerLineFormatting) {
 TEST_F(FormatTest, FormatsBuilderPattern) {
   verifyFormat(
       "return llvm::StringSwitch<Reference::Kind>(name)\n"
-      "    .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n"
+      "           .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n"
-      "    .StartsWith(\".eh_frame\", ORDER_EH_FRAME).StartsWith(\".init\", ORDER_INIT)\n"
+      "           .StartsWith(\".eh_frame\", ORDER_EH_FRAME)\n"
-      "    .StartsWith(\".fini\", ORDER_FINI).StartsWith(\".hash\", ORDER_HASH)\n"
+      "           .StartsWith(\".init\", ORDER_INIT).StartsWith(\".fini\", "
-      "    .Default(ORDER_TEXT);\n");
+      "ORDER_FINI)\n"
+      "           .StartsWith(\".hash\", ORDER_HASH).Default(ORDER_TEXT);\n");
 
   verifyFormat("return aaaaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa() <\n"
                "       aaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa();");
@@ -2850,10 +2851,23 @@ TEST_F(FormatTest, FormatsBuilderPattern) {
       "aaaaaaaaaaaaaaaaaaa()->aaaaaa(bbbbb)->aaaaaaaaaaaaaaaaaaa( // break\n"
       "    aaaaaaaaaaaaaa);");
   verifyFormat(
-      "aaaaaaaaaaaaaaaaaaaaaaa *aaaaaaaaa = aaaaaa->aaaaaaaaaaaa()\n"
+      "aaaaaaaaaaaaaaaaaaaaaaa *aaaaaaaaa =\n"
-      "    ->aaaaaaaaaaaaaaaa(\n"
+      "    aaaaaa->aaaaaaaaaaaa()\n"
-      "          aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
+      "        ->aaaaaaaaaaaaaaaa(\n"
-      "    ->aaaaaaaaaaaaaaaaa();");
+      "              aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
+      "        ->aaaaaaaaaaaaaaaaa();");
+  verifyFormat(
+      "someobj->Add((new util::filetools::Handler(dir))\n"
+      "                 ->OnEvent1(NewPermanentCallback(\n"
+      "                       this, &HandlerHolderClass::EventHandlerCBA))\n"
+      "                 ->OnEvent2(NewPermanentCallback(\n"
+      "                       this, &HandlerHolderClass::EventHandlerCBB))\n"
+      "                 ->OnEvent3(NewPermanentCallback(\n"
+      "                       this, &HandlerHolderClass::EventHandlerCBC))\n"
+      "                 ->OnEvent5(NewPermanentCallback(\n"
+      "                       this, &HandlerHolderClass::EventHandlerCBD))\n"
+      "                 ->OnEvent6(NewPermanentCallback(\n"
+      "                       this, &HandlerHolderClass::EventHandlerCBE)));");
 }
 
 TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) {
@@ -2886,8 +2900,8 @@ TEST_F(FormatTest, BreaksAfterAssignments) {
       "    Line.Tokens.front().Tok.getLo(), Line.Tokens.back().Tok.getLoc());");
 
   verifyFormat(
-      "aaaaaaaaaaaaaaaaaaaaaaaaaa aaaa = aaaaaaaaaaaaaa(0)\n"
+      "aaaaaaaaaaaaaaaaaaaaaaaaaa aaaa = aaaaaaaaaaaaaa(0).aaaa().aaaaaaaaa(\n"
-      "    .aaaa().aaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa::aaaaaaaaaaaaaaaaaaaaa);");
+      "    aaaaaaaaaaaaaaaaaaa::aaaaaaaaaaaaaaaaaaaaa);");
   verifyFormat("unsigned OriginalStartColumn =\n"
                "    SourceMgr.getSpellingColumnNumber(\n"
                "        Current.FormatTok.getStartOfNonWhitespace()) -\n"
@@ -5210,7 +5224,7 @@ TEST_F(FormatTest, BreakStringLiterals) {
                    getLLVMStyleWithColumns(20)));
   EXPECT_EQ(
       "f(\"one two\".split(\n"
-      "    variable));",
+      "      variable));",
       format("f(\"one two\".split(variable));", getLLVMStyleWithColumns(20)));
   EXPECT_EQ("f(\"one two three four five six \"\n"
             "  \"seven\".split(\n"