From 53013530467f00cc2bcdc5b25fbf55f433464566 Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Mon, 16 Jan 2017 13:13:15 +0000 Subject: [PATCH] clang-format: Always wrap before multi-line parameters/operands. Before: aaaaaaaaaaaaaaaaaa(aaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaa); After: aaaaaaaaaaaaaaaaaa(aaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaa); No new test cases, as the existing ones cover this fairly well. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@292110 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/ContinuationIndenter.cpp | 61 +++++++++++++++++------------ lib/Format/ContinuationIndenter.h | 13 ++++-- unittests/Format/FormatTest.cpp | 56 ++++++++++++++------------ 3 files changed, 78 insertions(+), 52 deletions(-) diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp index 45522a8232..778ffe30f6 100644 --- a/lib/Format/ContinuationIndenter.cpp +++ b/lib/Format/ContinuationIndenter.cpp @@ -135,6 +135,12 @@ bool ContinuationIndenter::canBreak(const LineState &State) { return false; } + // If binary operators are moved to the next line (including commas for some + // styles of constructor initializers), that's always ok. + if (!Current.isOneOf(TT_BinaryOperator, tok::comma) && + State.Stack.back().NoLineBreakInOperand) + return false; + return !State.Stack.back().NoLineBreak; } @@ -395,6 +401,27 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun, State.Stack.back().NoLineBreak = true; } + // Don't allow the RHS of an operator to be split over multiple lines unless + // there is a line-break right after the operator. + // Exclude relational operators, as there, it is always more desirable to + // have the LHS 'left' of the RHS. + const FormatToken *P = Current.getPreviousNonComment(); + if (!Current.is(tok::comment) && P && + (P->isOneOf(TT_BinaryOperator, tok::comma) || + (P->is(TT_ConditionalExpr) && P->is(tok::colon))) && + !P->isOneOf(TT_OverloadedOperator, TT_CtorInitializerComma) && + P->getPrecedence() != prec::Assignment && + P->getPrecedence() != prec::Relational) { + bool BreakBeforeOperator = + P->is(tok::lessless) || + (P->is(TT_BinaryOperator) && + Style.BreakBeforeBinaryOperators != FormatStyle::BOS_None) || + (P->is(TT_ConditionalExpr) && Style.BreakBeforeTernaryOperators); + if (!BreakBeforeOperator || + (!State.Stack.back().LastOperatorWrapped && BreakBeforeOperator)) + State.Stack.back().NoLineBreakInOperand = true; + } + State.Column += Spaces; if (Current.isNot(tok::comment) && Previous.is(tok::l_paren) && Previous.Previous && @@ -716,6 +743,8 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State, assert(State.Stack.size()); const FormatToken &Current = *State.NextToken; + if (Current.isOneOf(tok::comma, TT_BinaryOperator)) + State.Stack.back().NoLineBreakInOperand = false; if (Current.is(TT_InheritanceColon)) State.Stack.back().AvoidBinPacking = true; if (Current.is(tok::lessless) && Current.isNot(TT_OverloadedOperator)) { @@ -724,8 +753,10 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State, else State.Stack.back().LastOperatorWrapped = Newline; } - if ((Current.is(TT_BinaryOperator) && Current.isNot(tok::lessless)) || - Current.is(TT_ConditionalExpr)) + if (Current.is(TT_BinaryOperator) && Current.isNot(tok::lessless)) + State.Stack.back().LastOperatorWrapped = Newline; + if (Current.is(TT_ConditionalExpr) && Current.Previous && + !Current.Previous->is(TT_ConditionalExpr)) State.Stack.back().LastOperatorWrapped = Newline; if (Current.is(TT_ArraySubscriptLSquare) && State.Stack.back().StartOfArraySubscripts == 0) @@ -848,6 +879,8 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State, I != E; ++I) { ParenState NewParenState = State.Stack.back(); NewParenState.ContainsLineBreak = false; + NewParenState.NoLineBreak = + NewParenState.NoLineBreak || State.Stack.back().NoLineBreakInOperand; // Indent from 'LastSpace' unless these are fake parentheses encapsulating // a builder type call after 'return' or, if the alignment after opening @@ -862,24 +895,6 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State, std::max(std::max(State.Column, NewParenState.Indent), State.Stack.back().LastSpace); - // Don't allow the RHS of an operator to be split over multiple lines unless - // there is a line-break right after the operator. - // Exclude relational operators, as there, it is always more desirable to - // have the LHS 'left' of the RHS. - if (Previous && Previous->getPrecedence() != prec::Assignment && - Previous->isOneOf(TT_BinaryOperator, TT_ConditionalExpr, tok::comma) && - Previous->getPrecedence() != prec::Relational) { - bool BreakBeforeOperator = - Previous->is(tok::lessless) || - (Previous->is(TT_BinaryOperator) && - Style.BreakBeforeBinaryOperators != FormatStyle::BOS_None) || - (Previous->is(TT_ConditionalExpr) && - Style.BreakBeforeTernaryOperators); - if ((!Newline && !BreakBeforeOperator) || - (!State.Stack.back().LastOperatorWrapped && BreakBeforeOperator)) - NewParenState.NoLineBreak = true; - } - // Do not indent relative to the fake parentheses inserted for "." or "->". // This is a special case to make the following to statements consistent: // OuterFunction(InnerFunctionCall( // break @@ -1003,15 +1018,13 @@ void ContinuationIndenter::moveStatePastScopeOpener(LineState &State, // Generally inherit NoLineBreak from the current scope to nested scope. // However, don't do this for non-empty nested blocks, dict literals and // array literals as these follow different indentation rules. - const FormatToken *Previous = Current.getPreviousNonComment(); bool NoLineBreak = Current.Children.empty() && !Current.isOneOf(TT_DictLiteral, TT_ArrayInitializerLSquare) && (State.Stack.back().NoLineBreak || + State.Stack.back().NoLineBreakInOperand || (Current.is(TT_TemplateOpener) && - State.Stack.back().ContainsUnwrappedBuilder) || - (Current.is(tok::l_brace) && !Newline && Previous && - Previous->is(tok::comma))); + State.Stack.back().ContainsUnwrappedBuilder)); State.Stack.push_back(ParenState(NewIndent, NewIndentLevel, LastSpace, AvoidBinPacking, NoLineBreak)); State.Stack.back().NestedBlockIndent = NestedBlockIndent; diff --git a/lib/Format/ContinuationIndenter.h b/lib/Format/ContinuationIndenter.h index 21ad653c4f..704b2f891f 100644 --- a/lib/Format/ContinuationIndenter.h +++ b/lib/Format/ContinuationIndenter.h @@ -151,10 +151,11 @@ struct ParenState { : Indent(Indent), IndentLevel(IndentLevel), LastSpace(LastSpace), NestedBlockIndent(Indent), BreakBeforeClosingBrace(false), AvoidBinPacking(AvoidBinPacking), BreakBeforeParameter(false), - NoLineBreak(NoLineBreak), LastOperatorWrapped(true), - ContainsLineBreak(false), ContainsUnwrappedBuilder(false), - AlignColons(true), ObjCSelectorNameFound(false), - HasMultipleNestedBlocks(false), NestedBlockInlined(false) {} + NoLineBreak(NoLineBreak), NoLineBreakInOperand(false), + LastOperatorWrapped(true), ContainsLineBreak(false), + ContainsUnwrappedBuilder(false), AlignColons(true), + ObjCSelectorNameFound(false), HasMultipleNestedBlocks(false), + NestedBlockInlined(false) {} /// \brief The position to which a specific parenthesis level needs to be /// indented. @@ -224,6 +225,10 @@ struct ParenState { /// \brief Line breaking in this context would break a formatting rule. bool NoLineBreak : 1; + /// \brief Same as \c NoLineBreak, but is restricted until the end of the + /// operand (including the next ","). + bool NoLineBreakInOperand : 1; + /// \brief True if the last binary operator on this level was wrapped to the /// next line. bool LastOperatorWrapped : 1; diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 150f6077f2..9bf7fb060f 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -3353,9 +3353,10 @@ TEST_F(FormatTest, PreventConfusingIndents) { " aaaaaaaaaaaaaaaaaaaaaaaa<\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa>,\n" " aaaaaaaaaaaaaaaaaaaaaaaa>;"); - verifyFormat("int a = bbbb && ccc && fffff(\n" + verifyFormat("int a = bbbb && ccc &&\n" + " fffff(\n" "#define A Just forcing a new line\n" - " ddd);"); + " ddd);"); } TEST_F(FormatTest, LineBreakingInBinaryExpressions) { @@ -4050,8 +4051,9 @@ TEST_F(FormatTest, BreaksDesireably) { " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa));"); verifyFormat( - "aaaaaaaa(aaaaaaaaaaaaa, aaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)),\n" + "aaaaaaaa(aaaaaaaaaaaaa,\n" + " aaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)),\n" " aaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)));"); @@ -4067,8 +4069,9 @@ TEST_F(FormatTest, BreaksDesireably) { "aaaaaa(new Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaa));"); verifyFormat( - "aaaaaa(aaa, new Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaa));"); + "aaaaaa(aaa,\n" + " new Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaa));"); verifyFormat("aaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); @@ -4626,10 +4629,9 @@ TEST_F(FormatTest, BreaksConditionalExpressions) { " aaaaaaaaaaaaaaaaaaaaa\n" " : aaaaaaaaaa;"); verifyFormat( - "aaaaaa = aaaaaaaaaaaa\n" - " ? aaaaaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" - " : aaaaaaaaaaaaaaaaaaaaaa\n" - " : aaaaaaaaaaaaaaaaaaaaaaaaaaaa;"); + "aaaaaa = aaaaaaaaaaaa ? aaaaaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " : aaaaaaaaaaaaaaaaaaaaaa\n" + " : aaaaaaaaaaaaaaaaaaaaaaaaaaaa;"); FormatStyle NoBinPacking = getLLVMStyle(); NoBinPacking.BinPackArguments = false; @@ -4826,10 +4828,11 @@ TEST_F(FormatTest, AlignsStringLiterals) { verifyFormat("someFunction(\"Always break between multi-line\"\n" " \" string literals\",\n" " and, other, parameters);"); - EXPECT_EQ("fun + \"1243\" /* comment */\n" - " \"5678\";", + EXPECT_EQ("fun +\n" + " \"1243\" /* comment */\n" + " \"5678\";", format("fun + \"1243\" /* comment */\n" - " \"5678\";", + " \"5678\";", getLLVMStyleWithColumns(28))); EXPECT_EQ( "aaaaaa = \"aaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaa \"\n" @@ -4839,11 +4842,13 @@ TEST_F(FormatTest, AlignsStringLiterals) { "\"aaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaa " "aaaaaaaaaaaaaaaaaaaaa\" " "\"aaaaaaaaaaaaaaaa\";")); - verifyFormat("a = a + \"a\"\n" - " \"a\"\n" - " \"a\";"); - verifyFormat("f(\"a\", \"b\"\n" - " \"c\");"); + verifyFormat("a = a +\n" + " \"a\"\n" + " \"a\"\n" + " \"a\";"); + verifyFormat("f(\"a\",\n" + " \"b\"\n" + " \"c\");"); verifyFormat( "#define LL_FORMAT \"ll\"\n" @@ -5028,8 +5033,9 @@ TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) { " \"bbbb\"\n" " \"cccc\");", Break); - verifyFormat("aaaa(qqq, \"bbbb\"\n" - " \"cccc\");", + verifyFormat("aaaa(qqq,\n" + " \"bbbb\"\n" + " \"cccc\");", NoBreak); verifyFormat("aaaa(qqq,\n" " \"bbbb\"\n" @@ -5039,8 +5045,9 @@ TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) { " L\"bbbb\"\n" " L\"cccc\");", Break); - verifyFormat("aaaaa(aaaaaa, aaaaaaa(\"aaaa\"\n" - " \"bbbb\"));", + verifyFormat("aaaaa(aaaaaa,\n" + " aaaaaaa(\"aaaa\"\n" + " \"bbbb\"));", Break); verifyFormat("string s = someFunction(\n" " \"abc\"\n" @@ -5456,8 +5463,9 @@ TEST_F(FormatTest, WrapsAtNestedNameSpecifiers) { " aaaaaaaaaaaaaaaaaaaaaaa);"); verifyFormat( - "aaaaaaaaaaaaaaaaaa(aaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa::\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" + "aaaaaaaaaaaaaaaaaa(aaaaaaaa,\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa::\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" " aaaaaaaaaaaaaaaaaaaaa);", getLLVMStyleWithColumns(74)); -- 2.40.0