Author: djasper Date: Wed Feb 1 03:23:39 2017 New Revision: 293752 URL: http://llvm.org/viewvc/llvm-project?rev=293752&view=rev Log: clang-format: Don't force-wrap multiline RHSs for 2-operand experssions.
This rows back on r288120, r291801 and r292110. I apologize in advance for the churn. All of those revisions where meant to make the wrapping of RHS expressions more consistent. However, now that they are consistent, we seem to be a bit too eager. The reasoning here is that I think it is generally correct that we want to line-wrap before multiline RHS expressions (or multiline arguments to a function call). However, if there are only two of such operands or arguments, there is always a clear vertical separation between them and the additional line break seems much less desirable. Somewhat good examples are expressions like: EXPECT_EQ(2, someLongExpression( orCall)); Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=293752&r1=293751&r2=293752&view=diff ============================================================================== --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original) +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Wed Feb 1 03:23:39 2017 @@ -424,7 +424,12 @@ void ContinuationIndenter::addTokenOnCur (P->is(TT_BinaryOperator) && Style.BreakBeforeBinaryOperators != FormatStyle::BOS_None) || (P->is(TT_ConditionalExpr) && Style.BreakBeforeTernaryOperators); - if (!BreakBeforeOperator || + // Don't do this if there are only two operands. In these cases, there is + // always a nice vertical separation between them and the extra line break + // does not help. + bool HasTwoOperands = + P->OperatorIndex == 0 && !P->NextOperator && !P->is(TT_ConditionalExpr); + if ((!BreakBeforeOperator && !HasTwoOperands) || (!State.Stack.back().LastOperatorWrapped && BreakBeforeOperator)) State.Stack.back().NoLineBreakInOperand = true; } Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=293752&r1=293751&r2=293752&view=diff ============================================================================== --- cfe/trunk/unittests/Format/FormatTest.cpp (original) +++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Feb 1 03:23:39 2017 @@ -4060,9 +4060,14 @@ TEST_F(FormatTest, ExpressionIndentation " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +\n" " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) {\n}"); verifyFormat("if () {\n" + "} else if (aaaaa && bbbbb > // break\n" + " ccccc) {\n" + "}"); + verifyFormat("if () {\n" "} else if (aaaaa &&\n" " bbbbb > // break\n" - " ccccc) {\n" + " ccccc &&\n" + " ddddd) {\n" "}"); // Presence of a trailing comment used to change indentation of b. @@ -4167,7 +4172,7 @@ TEST_F(FormatTest, NoOperandAlignment) { Style); verifyFormat("int a = aa\n" " + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n" - " * cccccccccccccccccccccccccccccccccccc;", + " * cccccccccccccccccccccccccccccccccccc;\n", Style); Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign; @@ -4659,9 +4664,13 @@ TEST_F(FormatTest, BreaksDesireably) { "aaaaaa(new Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaa));"); verifyFormat( + "aaaaaa(aaa, new Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaa));"); + verifyFormat( "aaaaaa(aaa,\n" " new Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaa));"); + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaa),\n" + " aaaa);"); verifyFormat("aaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); @@ -5140,11 +5149,18 @@ TEST_F(FormatTest, ParenthesesAndOperand TEST_F(FormatTest, BreaksConditionalExpressions) { verifyFormat( - "aaaa(aaaaaaaaaaaaaaaaaaaa,\n" + "aaaa(aaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " : aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat( + "aaaa(aaaaaaaaaa, aaaaaaaa,\n" " aaaaaaaaaaaaaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" " : aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); verifyFormat( - "aaaa(aaaaaaaaaaaaaaaaaaaa,\n" + "aaaa(aaaaaaaaaaaaaaaaaaaa, aaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + " : aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat( + "aaaa(aaaaaaaaa, aaaaaaaaa,\n" " aaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" " : aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); verifyFormat( @@ -5275,12 +5291,21 @@ TEST_F(FormatTest, BreaksConditionalExpr Style.BreakBeforeTernaryOperators = false; Style.ColumnLimit = 70; verifyFormat( - "aaaa(aaaaaaaaaaaaaaaaaaaa,\n" + "aaaa(aaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaa ?\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa :\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);", + Style); + verifyFormat( + "aaaa(aaaaaaaaaa, aaaaaaaa,\n" " aaaaaaaaaaaaaaaaaaaaaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa :\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);", Style); verifyFormat( - "aaaa(aaaaaaaaaaaaaaaaaaaa,\n" + "aaaa(aaaaaaaaaaaaaaaaaaaa, aaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa :\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);", + Style); + verifyFormat( + "aaaa(aaaaaaaa, aaaaaaaaaa,\n" " aaaaaaa ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa :\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);", Style); @@ -5418,9 +5443,8 @@ TEST_F(FormatTest, AlignsStringLiterals) verifyFormat("someFunction(\"Always break between multi-line\"\n" " \" string literals\",\n" " and, other, parameters);"); - EXPECT_EQ("fun +\n" - " \"1243\" /* comment */\n" - " \"5678\";", + EXPECT_EQ("fun + \"1243\" /* comment */\n" + " \"5678\";", format("fun + \"1243\" /* comment */\n" " \"5678\";", getLLVMStyleWithColumns(28))); @@ -5432,13 +5456,11 @@ TEST_F(FormatTest, AlignsStringLiterals) "\"aaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaa " "aaaaaaaaaaaaaaaaaaaaa\" " "\"aaaaaaaaaaaaaaaa\";")); - verifyFormat("a = a +\n" - " \"a\"\n" - " \"a\"\n" - " \"a\";"); - verifyFormat("f(\"a\",\n" - " \"b\"\n" - " \"c\");"); + verifyFormat("a = a + \"a\"\n" + " \"a\"\n" + " \"a\";"); + verifyFormat("f(\"a\", \"b\"\n" + " \"c\");"); verifyFormat( "#define LL_FORMAT \"ll\"\n" @@ -5623,9 +5645,8 @@ TEST_F(FormatTest, AlwaysBreakBeforeMult " \"bbbb\"\n" " \"cccc\");", Break); - verifyFormat("aaaa(qqq,\n" - " \"bbbb\"\n" - " \"cccc\");", + verifyFormat("aaaa(qqq, \"bbbb\"\n" + " \"cccc\");", NoBreak); verifyFormat("aaaa(qqq,\n" " \"bbbb\"\n" @@ -5635,9 +5656,8 @@ TEST_F(FormatTest, AlwaysBreakBeforeMult " L\"bbbb\"\n" " L\"cccc\");", Break); - verifyFormat("aaaaa(aaaaaa,\n" - " aaaaaaa(\"aaaa\"\n" - " \"bbbb\"));", + verifyFormat("aaaaa(aaaaaa, aaaaaaa(\"aaaa\"\n" + " \"bbbb\"));", Break); verifyFormat("string s = someFunction(\n" " \"abc\"\n" @@ -6497,10 +6517,17 @@ TEST_F(FormatTest, UnderstandsUsesOfStar verifyFormat("foo<b & 1>();"); verifyFormat("decltype(*::std::declval<const T &>()) void F();"); verifyFormat( + "template <class T, class = typename std::enable_if<\n" + " std::is_integral<T>::value &&\n" + " (sizeof(T) > 1 || sizeof(T) < 8)>::type>\n" + "void F();", + getLLVMStyleWithColumns(70)); + verifyFormat( "template <class T,\n" " class = typename std::enable_if<\n" " std::is_integral<T>::value &&\n" - " (sizeof(T) > 1 || sizeof(T) < 8)>::type>\n" + " (sizeof(T) > 1 || sizeof(T) < 8)>::type,\n" + " class U>\n" "void F();", getLLVMStyleWithColumns(70)); verifyFormat( @@ -7422,7 +7449,10 @@ TEST_F(FormatTest, FormatsBracedListsInC "};"); // Don't create hanging lists. - verifyFormat("someFunction(Param,\n" + verifyFormat("someFunction(Param, {List1, List2,\n" + " List3});", + getLLVMStyleWithColumns(35)); + verifyFormat("someFunction(Param, Param,\n" " {List1, List2,\n" " List3});", getLLVMStyleWithColumns(35)); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits