r224112 - clang-format: Revamp nested block formatting.
Daniel Jasper
djasper at google.com
Fri Dec 12 01:40:58 PST 2014
Author: djasper
Date: Fri Dec 12 03:40:58 2014
New Revision: 224112
URL: http://llvm.org/viewvc/llvm-project?rev=224112&view=rev
Log:
clang-format: Revamp nested block formatting.
This fixed llvm.org/PR21804 and hopefully a few other strange cases.
Before:
if (blah_blah(whatever, whatever, [] {
doo_dah();
doo_dah();
})) {
}
}
After:
if (blah_blah(whatever, whatever, [] {
doo_dah();
doo_dah();
})) {
}
}
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/ContinuationIndenter.h
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=224112&r1=224111&r2=224112&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Fri Dec 12 03:40:58 2014
@@ -324,25 +324,27 @@ void ContinuationIndenter::addTokenOnCur
State.Column += Spaces;
if (Current.isNot(tok::comment) && Previous.is(tok::l_paren) &&
- Previous.Previous && Previous.Previous->isOneOf(tok::kw_if, tok::kw_for))
+ Previous.Previous &&
+ Previous.Previous->isOneOf(tok::kw_if, tok::kw_for)) {
// Treat the condition inside an if as if it was a second function
// parameter, i.e. let nested calls have a continuation indent.
State.Stack.back().LastSpace = State.Column;
- else if (!Current.isOneOf(tok::comment, tok::caret) &&
- (Previous.is(tok::comma) ||
- (Previous.is(tok::colon) && Previous.is(TT_ObjCMethodExpr))))
+ State.Stack.back().NestedBlockIndent = State.Column;
+ } else if (!Current.isOneOf(tok::comment, tok::caret) &&
+ (Previous.is(tok::comma) ||
+ (Previous.is(tok::colon) && Previous.is(TT_ObjCMethodExpr)))) {
State.Stack.back().LastSpace = State.Column;
- else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
- TT_CtorInitializerColon)) &&
- ((Previous.getPrecedence() != prec::Assignment &&
- (Previous.isNot(tok::lessless) || Previous.OperatorIndex != 0 ||
- !Previous.LastOperator)) ||
- Current.StartsBinaryExpression))
+ } else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
+ TT_CtorInitializerColon)) &&
+ ((Previous.getPrecedence() != prec::Assignment &&
+ (Previous.isNot(tok::lessless) || Previous.OperatorIndex != 0 ||
+ !Previous.LastOperator)) ||
+ Current.StartsBinaryExpression)) {
// 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.
State.Stack.back().LastSpace = State.Column;
- else if (Previous.is(TT_InheritanceColon)) {
+ } else if (Previous.is(TT_InheritanceColon)) {
State.Stack.back().Indent = State.Column;
State.Stack.back().LastSpace = State.Column;
} else if (Previous.opensScope()) {
@@ -393,6 +395,7 @@ unsigned ContinuationIndenter::addTokenO
Penalty += Style.PenaltyBreakFirstLessLess;
State.Column = getNewLineColumn(State);
+ State.Stack.back().NestedBlockIndent = State.Column;
if (NextNonComment->isMemberAccess()) {
if (State.Stack.back().CallContinuation == 0)
State.Stack.back().CallContinuation = State.Column;
@@ -513,12 +516,10 @@ unsigned ContinuationIndenter::getNewLin
return Current.NestingLevel == 0 ? State.FirstIndent
: State.Stack.back().Indent;
if (Current.isOneOf(tok::r_brace, tok::r_square)) {
- if (State.Stack.size() > 1 &&
- State.Stack[State.Stack.size() - 2].NestedBlockInlined)
- return State.FirstIndent;
- if (Current.closesBlockTypeList(Style) ||
- (Current.MatchingParen &&
- Current.MatchingParen->BlockKind == BK_BracedInit))
+ if (Current.closesBlockTypeList(Style))
+ return State.Stack[State.Stack.size() - 2].NestedBlockIndent;
+ if (Current.MatchingParen &&
+ Current.MatchingParen->BlockKind == BK_BracedInit)
return State.Stack[State.Stack.size() - 2].LastSpace;
return State.FirstIndent;
}
@@ -664,7 +665,7 @@ unsigned ContinuationIndenter::moveState
!Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
State.Stack.back().NestedBlockInlined =
!Newline &&
- (Previous->isNot(tok::l_paren) || Previous->ParameterCount > 1) && !Newline;
+ (Previous->isNot(tok::l_paren) || Previous->ParameterCount > 1);
}
moveStatePastFakeLParens(State, Newline);
@@ -777,9 +778,8 @@ void ContinuationIndenter::moveStatePast
}
}
-// Remove the fake r_parens after 'Tok'.
-static void consumeRParens(LineState& State, const FormatToken &Tok) {
- for (unsigned i = 0, e = Tok.FakeRParens; i != e; ++i) {
+void ContinuationIndenter::moveStatePastFakeRParens(LineState &State) {
+ for (unsigned i = 0, e = State.NextToken->FakeRParens; i != e; ++i) {
unsigned VariablePos = State.Stack.back().VariablePos;
assert(State.Stack.size() > 1);
if (State.Stack.size() == 1) {
@@ -791,45 +791,6 @@ static void consumeRParens(LineState& St
}
}
-// Returns whether 'Tok' opens or closes a scope requiring special handling
-// of the subsequent fake r_parens.
-//
-// For example, if this is an l_brace starting a nested block, we pretend (wrt.
-// to indentation) that we already consumed the corresponding r_brace. Thus, we
-// remove all ParenStates caused by fake parentheses that end at the r_brace.
-// The net effect of this is that we don't indent relative to the l_brace, if
-// the nested block is the last parameter of a function. This formats:
-//
-// SomeFunction(a, [] {
-// f(); // break
-// });
-//
-// instead of:
-// SomeFunction(a, [] {
-// f(); // break
-// });
-static bool fakeRParenSpecialCase(const LineState &State) {
- const FormatToken &Tok = *State.NextToken;
- if (!Tok.MatchingParen)
- return false;
- const FormatToken *Left = &Tok;
- if (Tok.isOneOf(tok::r_brace, tok::r_square))
- Left = Tok.MatchingParen;
- return !State.Stack.back().HasMultipleNestedBlocks &&
- Left->isOneOf(tok::l_brace, tok::l_square) &&
- (Left->BlockKind == BK_Block ||
- Left->isOneOf(TT_ArrayInitializerLSquare, TT_DictLiteral));
-}
-
-void ContinuationIndenter::moveStatePastFakeRParens(LineState &State) {
- // Don't remove FakeRParens attached to r_braces that surround nested blocks
- // as they will have been removed early (see above).
- if (fakeRParenSpecialCase(State))
- return;
-
- consumeRParens(State, *State.NextToken);
-}
-
void ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
bool Newline) {
const FormatToken &Current = *State.NextToken;
@@ -846,16 +807,12 @@ void ContinuationIndenter::moveStatePast
bool AvoidBinPacking;
bool BreakBeforeParameter = false;
if (Current.isOneOf(tok::l_brace, TT_ArrayInitializerLSquare)) {
- if (fakeRParenSpecialCase(State))
- consumeRParens(State, *Current.MatchingParen);
-
- NewIndent = State.Stack.back().LastSpace;
if (Current.opensBlockTypeList(Style)) {
- NewIndent += Style.IndentWidth;
+ NewIndent = State.Stack.back().NestedBlockIndent + Style.IndentWidth;
NewIndent = std::min(State.Column + 2, NewIndent);
++NewIndentLevel;
} else {
- NewIndent += Style.ContinuationIndentWidth;
+ NewIndent = State.Stack.back().LastSpace + Style.ContinuationIndentWidth;
NewIndent = std::min(State.Column + 1, NewIndent);
}
const FormatToken *NextNoComment = Current.getNextNonComment();
@@ -884,9 +841,11 @@ void ContinuationIndenter::moveStatePast
bool NoLineBreak = State.Stack.back().NoLineBreak ||
(Current.is(TT_TemplateOpener) &&
State.Stack.back().ContainsUnwrappedBuilder);
+ unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent;
State.Stack.push_back(ParenState(NewIndent, NewIndentLevel,
State.Stack.back().LastSpace,
AvoidBinPacking, NoLineBreak));
+ State.Stack.back().NestedBlockIndent = NestedBlockIndent;
State.Stack.back().BreakBeforeParameter = BreakBeforeParameter;
State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount > 1;
}
@@ -913,20 +872,17 @@ void ContinuationIndenter::moveStatePast
}
void ContinuationIndenter::moveStateToNewBlock(LineState &State) {
- // If we have already found more than one lambda introducers on this level, we
- // opt out of this because similarity between the lambdas is more important.
- if (fakeRParenSpecialCase(State))
- consumeRParens(State, *State.NextToken->MatchingParen);
-
+ unsigned NestedBlockIndent = State.Stack.back().NestedBlockIndent;
// ObjC block sometimes follow special indentation rules.
unsigned NewIndent =
- State.Stack.back().LastSpace + (State.NextToken->is(TT_ObjCBlockLBrace)
- ? Style.ObjCBlockIndentWidth
- : Style.IndentWidth);
+ NestedBlockIndent + (State.NextToken->is(TT_ObjCBlockLBrace)
+ ? Style.ObjCBlockIndentWidth
+ : Style.IndentWidth);
State.Stack.push_back(ParenState(
NewIndent, /*NewIndentLevel=*/State.Stack.back().IndentLevel + 1,
State.Stack.back().LastSpace, /*AvoidBinPacking=*/true,
State.Stack.back().NoLineBreak));
+ State.Stack.back().NestedBlockIndent = NestedBlockIndent;
State.Stack.back().BreakBeforeParameter = true;
}
Modified: cfe/trunk/lib/Format/ContinuationIndenter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.h?rev=224112&r1=224111&r2=224112&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.h (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.h Fri Dec 12 03:40:58 2014
@@ -148,7 +148,8 @@ struct ParenState {
ParenState(unsigned Indent, unsigned IndentLevel, unsigned LastSpace,
bool AvoidBinPacking, bool NoLineBreak)
: Indent(Indent), IndentLevel(IndentLevel), LastSpace(LastSpace),
- FirstLessLess(0), BreakBeforeClosingBrace(false), QuestionColumn(0),
+ NestedBlockIndent(Indent), FirstLessLess(0),
+ BreakBeforeClosingBrace(false), QuestionColumn(0),
AvoidBinPacking(AvoidBinPacking), BreakBeforeParameter(false),
NoLineBreak(NoLineBreak), LastOperatorWrapped(true), ColonPos(0),
StartOfFunctionCall(0), StartOfArraySubscripts(0),
@@ -171,6 +172,10 @@ struct ParenState {
/// OtherParameter));
unsigned LastSpace;
+ /// \brief If a block relative to this parenthesis level gets wrapped, indent
+ /// it this much.
+ unsigned NestedBlockIndent;
+
/// \brief The position the first "<<" operator encountered on each level.
///
/// Used to align "<<" operators. 0 if no such operator has been encountered
@@ -265,6 +270,8 @@ struct ParenState {
return Indent < Other.Indent;
if (LastSpace != Other.LastSpace)
return LastSpace < Other.LastSpace;
+ if (NestedBlockIndent != Other.NestedBlockIndent)
+ return NestedBlockIndent < Other.NestedBlockIndent;
if (FirstLessLess != Other.FirstLessLess)
return FirstLessLess < Other.FirstLessLess;
if (BreakBeforeClosingBrace != Other.BreakBeforeClosingBrace)
Modified: cfe/trunk/lib/Format/FormatToken.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=224112&r1=224111&r2=224112&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Fri Dec 12 03:40:58 2014
@@ -409,8 +409,9 @@ struct FormatToken {
/// list that should be indented with a block indent.
bool opensBlockTypeList(const FormatStyle &Style) const {
return is(TT_ArrayInitializerLSquare) ||
- (is(tok::l_brace) && (BlockKind == BK_Block || is(TT_DictLiteral) ||
- !Style.Cpp11BracedListStyle));
+ (is(tok::l_brace) &&
+ (BlockKind == BK_Block || is(TT_DictLiteral) ||
+ (!Style.Cpp11BracedListStyle && NestingLevel == 0)));
}
/// \brief Same as opensBlockTypeList, but for the closing token.
Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=224112&r1=224111&r2=224112&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp Fri Dec 12 03:40:58 2014
@@ -593,6 +593,15 @@ unsigned UnwrappedLineFormatter::analyze
return Penalty;
}
+static void printLineState(const LineState &State) {
+ llvm::dbgs() << "State: ";
+ for (const ParenState &P : State.Stack) {
+ llvm::dbgs() << P.Indent << "|" << P.LastSpace << "|" << P.NestedBlockIndent
+ << " ";
+ }
+ llvm::dbgs() << State.NextToken->TokenText << "\n";
+}
+
void UnwrappedLineFormatter::reconstructPath(LineState &State,
StateNode *Current) {
std::deque<StateNode *> Path;
@@ -608,6 +617,7 @@ void UnwrappedLineFormatter::reconstruct
Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
DEBUG({
+ printLineState((*I)->Previous->State);
if ((*I)->NewLine) {
llvm::dbgs() << "Penalty for placing "
<< (*I)->Previous->State.NextToken->Tok.getName() << ": "
@@ -648,13 +658,8 @@ bool UnwrappedLineFormatter::formatChild
return true;
if (NewLine) {
- int AdditionalIndent =
- State.FirstIndent - State.Line->Level * Style.IndentWidth;
- if (State.Stack.size() < 2 ||
- !State.Stack[State.Stack.size() - 2].NestedBlockInlined) {
- AdditionalIndent = State.Stack.back().Indent -
- Previous.Children[0]->Level * Style.IndentWidth;
- }
+ int AdditionalIndent = State.Stack.back().Indent -
+ Previous.Children[0]->Level * Style.IndentWidth;
Penalty += format(Previous.Children, DryRun, AdditionalIndent,
/*FixBadIndentation=*/true);
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=224112&r1=224111&r2=224112&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Dec 12 03:40:58 2014
@@ -5912,8 +5912,7 @@ TEST_F(FormatTest, LayoutCxx11BraceIniti
" BracedList{ // comment 1 (Forcing interesting break)\n"
" param1, param2,\n"
" // comment 2\n"
- " param3, param4\n"
- " });",
+ " param3, param4 });",
ExtraSpaces);
verifyFormat(
"std::this_thread::sleep_for(\n"
@@ -9421,6 +9420,12 @@ TEST_F(FormatTest, FormatsLambdas) {
verifyFormat("void f() {\n"
" MACRO((const AA &a) { return 1; });\n"
"}");
+
+ verifyFormat("if (blah_blah(whatever, whatever, [] {\n"
+ " doo_dah();\n"
+ " doo_dah();\n"
+ " })) {\n"
+ "}");
}
TEST_F(FormatTest, FormatsBlocks) {
Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=224112&r1=224111&r2=224112&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Dec 12 03:40:58 2014
@@ -206,14 +206,13 @@ TEST_F(FormatTestJS, FunctionLiterals) {
" style: {direction: ''}\n"
" }\n"
"};");
- // FIXME: The formatting here probably isn't ideal.
EXPECT_EQ("abc = xyz ?\n"
" function() {\n"
" return 1;\n"
" } :\n"
" function() {\n"
- " return -1;\n"
- "};",
+ " return -1;\n"
+ " };",
format("abc=xyz?function(){return 1;}:function(){return -1;};"));
verifyFormat("var closure = goog.bind(\n"
@@ -254,6 +253,23 @@ TEST_F(FormatTestJS, FunctionLiterals) {
" return 1;\n"
" }\n"
"};");
+ verifyFormat("this.someObject.doSomething(aaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
+ " .then(goog.bind(function(aaaaaaaaaaa) {\n"
+ " someFunction();\n"
+ " someFunction();\n"
+ " }, this), aaaaaaaaaaaaaaaaa);");
+
+ // FIXME: This is not ideal yet.
+ verifyFormat("someFunction(goog.bind(\n"
+ " function() {\n"
+ " doSomething();\n"
+ " doSomething();\n"
+ " },\n"
+ " this),\n"
+ " goog.bind(function() {\n"
+ " doSomething();\n"
+ " doSomething();\n"
+ " }, this));");
}
TEST_F(FormatTestJS, InliningFunctionLiterals) {
@@ -341,7 +357,10 @@ TEST_F(FormatTestJS, MultipleFunctionLit
verifyFormat("getSomeLongPromise()\n"
" .then(function(value) { body(); })\n"
- " .thenCatch(function(error) { body(); });");
+ " .thenCatch(function(error) {\n"
+ " body();\n"
+ " body();\n"
+ " });");
verifyFormat("getSomeLongPromise()\n"
" .then(function(value) {\n"
" body();\n"
@@ -351,6 +370,11 @@ TEST_F(FormatTestJS, MultipleFunctionLit
" body();\n"
" body();\n"
" });");
+
+ // FIXME: This is bad, but it used to be formatted correctly by accident.
+ verifyFormat("getSomeLongPromise().then(function(value) {\n"
+ " body();\n"
+ "}).thenCatch(function(error) { body(); });");
}
TEST_F(FormatTestJS, ReturnStatements) {
More information about the cfe-commits
mailing list