From: Daniel Jasper Date: Tue, 27 Aug 2013 11:09:05 +0000 (+0000) Subject: clang-format: Revamp builder-type call formatting. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d489f8ceb735458b0e1f814e3f952b154f49c025;p=clang 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))); git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@189337 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp index 4f6fca9a2b..ed5098bf48 100644 --- a/lib/Format/ContinuationIndenter.cpp +++ b/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 && - Current.FakeLParens.empty())) + (Previous.getPrecedence() != prec::Assignment || + 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) && - Line.Type == LT_BuilderTypeCall && State.ParenLevel == 0) + if (Current.isOneOf(tok::period, tok::arrow)) 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 diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp index 890b4f1c98..c8dfb13d78 100644 --- a/lib/Format/TokenAnnotator.cpp +++ b/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)) { - if (OperatorFound) - addFakeParenthesis(Start, prec::Level(Precedence - 1)); + (CurrentPrecedence != -1 && CurrentPrecedence < Precedence)) { + if (LatestOperator) { + 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(); diff --git a/lib/Format/TokenAnnotator.h b/lib/Format/TokenAnnotator.h index 28d55a007c..480681ed2f 100644 --- a/lib/Format/TokenAnnotator.h +++ b/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. diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 19a764cc6c..99a703a03d 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -2835,10 +2835,11 @@ TEST_F(FormatTest, AdaptiveOnePerLineFormatting) { TEST_F(FormatTest, FormatsBuilderPattern) { verifyFormat( "return llvm::StringSwitch(name)\n" - " .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n" - " .StartsWith(\".eh_frame\", ORDER_EH_FRAME).StartsWith(\".init\", ORDER_INIT)\n" - " .StartsWith(\".fini\", ORDER_FINI).StartsWith(\".hash\", ORDER_HASH)\n" - " .Default(ORDER_TEXT);\n"); + " .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n" + " .StartsWith(\".eh_frame\", ORDER_EH_FRAME)\n" + " .StartsWith(\".init\", ORDER_INIT).StartsWith(\".fini\", " + "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" - " ->aaaaaaaaaaaaaaaa(\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" - " ->aaaaaaaaaaaaaaaaa();"); + "aaaaaaaaaaaaaaaaaaaaaaa *aaaaaaaaa =\n" + " aaaaaa->aaaaaaaaaaaa()\n" + " ->aaaaaaaaaaaaaaaa(\n" + " 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" - " .aaaa().aaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa::aaaaaaaaaaaaaaaaaaaaa);"); + "aaaaaaaaaaaaaaaaaaaaaaaaaa aaaa = aaaaaaaaaaaaaa(0).aaaa().aaaaaaaaa(\n" + " 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"