r181430 - Improve line breaking in binary expressions.
Daniel Jasper
djasper at google.com
Wed May 8 07:12:05 PDT 2013
Author: djasper
Date: Wed May 8 09:12:04 2013
New Revision: 181430
URL: http://llvm.org/viewvc/llvm-project?rev=181430&view=rev
Log:
Improve line breaking in binary expressions.
If the LHS of a binary expression is broken, clang-format should also
break after the operator as otherwise:
- The RHS can be easy to miss
- It can look as if clang-format doesn't understand operator precedence
Before:
bool aaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa !=
bbbbbbbbbbbbbbbbbb && ccccccccc == ddddddddddd;
After:
bool aaaaaaaaaaaaaaaaaaaaa =
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa != bbbbbbbbbbbbbbbbbb &&
ccccccccc == ddddddddddd;
As an additional note, clang-format would also be ok with the following
formatting, it just has a higher penalty (IMO correctly so).
bool aaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa !=
bbbbbbbbbbbbbbbbbb &&
ccccccccc == ddddddddddd;
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=181430&r1=181429&r2=181430&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed May 8 09:12:04 2013
@@ -467,8 +467,9 @@ private:
if (Current.is(tok::question))
State.Stack.back().BreakBeforeParameter = true;
- if (Previous.isOneOf(tok::comma, tok::semi) &&
- !State.Stack.back().AvoidBinPacking)
+ if ((Previous.isOneOf(tok::comma, tok::semi) &&
+ !State.Stack.back().AvoidBinPacking) ||
+ Previous.Type == TT_BinaryOperator)
State.Stack.back().BreakBeforeParameter = false;
if (!DryRun) {
@@ -495,7 +496,7 @@ private:
}
const AnnotatedToken *TokenBefore = Current.getPreviousNoneComment();
if (TokenBefore && !TokenBefore->isOneOf(tok::comma, tok::semi) &&
- !TokenBefore->opensScope())
+ TokenBefore->Type != TT_BinaryOperator && !TokenBefore->opensScope())
State.Stack.back().BreakBeforeParameter = true;
// If we break after {, we should also break before the corresponding }.
@@ -565,9 +566,9 @@ private:
State.Stack.back().LastSpace = State.Column;
else if (Previous.Type == TT_InheritanceColon)
State.Stack.back().Indent = State.Column;
- else if (Previous.opensScope() && Previous.ParameterCount > 1)
- // If this function has multiple parameters, indent nested calls from
- // the start of the first parameter.
+ else if (Previous.opensScope() && !Current.FakeLParens.empty())
+ // If this function has multiple parameters or a binary expression
+ // parameter, indent nested calls from the start of the first parameter.
State.Stack.back().LastSpace = State.Column;
}
@@ -906,40 +907,45 @@ private:
/// \brief Returns \c true, if a line break after \p State is mandatory.
bool mustBreak(const LineState &State) {
- if (State.NextToken->MustBreakBefore)
+ const AnnotatedToken &Current = *State.NextToken;
+ const AnnotatedToken &Previous = *Current.Parent;
+ if (Current.MustBreakBefore || Current.Type == TT_InlineASMColon)
return true;
- if (State.NextToken->is(tok::r_brace) &&
- State.Stack.back().BreakBeforeClosingBrace)
+ if (Current.is(tok::r_brace) && State.Stack.back().BreakBeforeClosingBrace)
return true;
- if (State.NextToken->Parent->is(tok::semi) &&
- State.LineContainsContinuedForLoopSection)
+ if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection)
return true;
- if ((State.NextToken->Parent->isOneOf(tok::comma, tok::semi) ||
- State.NextToken->is(tok::question) ||
- State.NextToken->Type == TT_ConditionalExpr) &&
+ if ((Previous.isOneOf(tok::comma, tok::semi) || Current.is(tok::question) ||
+ Current.Type == TT_ConditionalExpr) &&
State.Stack.back().BreakBeforeParameter &&
- !State.NextToken->isTrailingComment() &&
- State.NextToken->isNot(tok::r_paren) &&
- State.NextToken->isNot(tok::r_brace))
+ !Current.isTrailingComment() &&
+ !Current.isOneOf(tok::r_paren, tok::r_brace))
+ return true;
+
+ // If we need to break somewhere inside the LHS of a binary expression, we
+ // should also break after the operator.
+ if (Previous.Type == TT_BinaryOperator &&
+ !Previous.isOneOf(tok::lessless, tok::question) &&
+ getPrecedence(Previous) != prec::Assignment &&
+ State.Stack.back().BreakBeforeParameter)
return true;
+
// FIXME: Comparing LongestObjCSelectorName to 0 is a hacky way of finding
// out whether it is the first parameter. Clean this up.
- if (State.NextToken->Type == TT_ObjCSelectorName &&
- State.NextToken->LongestObjCSelectorName == 0 &&
+ if (Current.Type == TT_ObjCSelectorName &&
+ Current.LongestObjCSelectorName == 0 &&
State.Stack.back().BreakBeforeParameter)
return true;
- if ((State.NextToken->Type == TT_CtorInitializerColon ||
- (State.NextToken->Parent->ClosesTemplateDeclaration &&
- State.ParenLevel == 0)))
- return true;
- if (State.NextToken->Type == TT_InlineASMColon)
+ if ((Current.Type == TT_CtorInitializerColon ||
+ (Previous.ClosesTemplateDeclaration && State.ParenLevel == 0)))
return true;
+
// This prevents breaks like:
// ...
// SomeParameter, OtherParameter).DoSomething(
// ...
// As they hide "DoSomething" and generally bad for readability.
- if (State.NextToken->isOneOf(tok::period, tok::arrow) &&
+ if (Current.isOneOf(tok::period, tok::arrow) &&
getRemainingLength(State) + State.Column > getColumnLimit() &&
State.ParenLevel < State.StartOfLineLevel)
return true;
@@ -1166,8 +1172,10 @@ public:
Lex.MeasureTokenLength(LastLoc, SourceMgr, Lex.getLangOpts()) - 1;
PreviousLineWasTouched = false;
if (TheLine.Last->is(tok::comment))
- Whitespaces.addUntouchableComment(SourceMgr.getSpellingColumnNumber(
- TheLine.Last->FormatTok.Tok.getLocation()) - 1);
+ Whitespaces.addUntouchableComment(
+ SourceMgr.getSpellingColumnNumber(
+ TheLine.Last->FormatTok.Tok.getLocation()) -
+ 1);
else
Whitespaces.alignComments();
}
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=181430&r1=181429&r2=181430&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed May 8 09:12:04 2013
@@ -320,7 +320,7 @@ private:
Tok->Type = TT_ObjCMethodExpr;
Tok->Parent->Type = TT_ObjCSelectorName;
if (Tok->Parent->FormatTok.TokenLength >
- Contexts.back().LongestObjCSelectorName)
+ Contexts.back().LongestObjCSelectorName)
Contexts.back().LongestObjCSelectorName =
Tok->Parent->FormatTok.TokenLength;
if (Contexts.back().FirstObjCSelectorName == NULL)
@@ -606,7 +606,8 @@ private:
if (Current.Parent && Current.is(tok::identifier) &&
((Current.Parent->is(tok::identifier) &&
Current.Parent->FormatTok.Tok.getIdentifierInfo()
- ->getPPKeywordID() == tok::pp_not_keyword) ||
+ ->getPPKeywordID() ==
+ tok::pp_not_keyword) ||
isSimpleTypeSpecifier(*Current.Parent) ||
Current.Parent->Type == TT_PointerOrReference ||
Current.Parent->Type == TT_TemplateCloser)) {
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=181430&r1=181429&r2=181430&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed May 8 09:12:04 2013
@@ -1606,6 +1606,24 @@ TEST_F(FormatTest, PreventConfusingInden
" ddd);");
}
+TEST_F(FormatTest, LineBreakingInBinaryExpressions) {
+ verifyFormat(
+ "bool aaaaaaa =\n"
+ " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaa).aaaaaaaaaaaaaaaaaaa() ||\n"
+ " bbbbbbbb();");
+ verifyFormat("bool aaaaaaaaaaaaaaaaaaaaa =\n"
+ " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa != bbbbbbbbbbbbbbbbbb &&\n"
+ " ccccccccc == ddddddddddd;");
+
+ verifyFormat("aaaaaa = aaaaaaa(aaaaaaa, // break\n"
+ " aaaaaa) &&\n"
+ " bbbbbb && cccccc;");
+ verifyFormat("Whitespaces.addUntouchableComment(\n"
+ " SourceMgr.getSpellingColumnNumber(\n"
+ " TheLine.Last->FormatTok.Tok.getLocation()) -\n"
+ " 1);");
+}
+
TEST_F(FormatTest, ExpressionIndentation) {
verifyFormat("bool value = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +\n"
More information about the cfe-commits
mailing list