r293752 - clang-format: Don't force-wrap multiline RHSs for 2-operand experssions.
Daniel Jasper via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 1 01:23:39 PST 2017
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));
More information about the cfe-commits
mailing list