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();
        })) {
    }
    }

llvm-svn: 224112
This commit is contained in:
Daniel Jasper 2014-12-12 09:40:58 +00:00
parent 9e3a60769f
commit 11a0ac66e1
6 changed files with 88 additions and 90 deletions

View File

@ -324,25 +324,27 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
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) &&
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))))
(Previous.is(tok::colon) && Previous.is(TT_ObjCMethodExpr)))) {
State.Stack.back().LastSpace = State.Column;
else if ((Previous.isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
} 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))
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::addTokenOnNewLine(LineState &State,
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::getNewLineColumn(const LineState &State) {
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::moveStateToNextToken(LineState &State,
!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::moveStatePastFakeLParens(LineState &State,
}
}
// 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& State, const FormatToken &Tok) {
}
}
// 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::moveStatePastScopeOpener(LineState &State,
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::moveStatePastScopeOpener(LineState &State,
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::moveStatePastScopeCloser(LineState &State) {
}
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)
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;
}

View File

@ -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)

View File

@ -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.

View File

@ -593,6 +593,15 @@ unsigned UnwrappedLineFormatter::analyzeSolutionSpace(LineState &InitialState,
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::reconstructPath(LineState &State,
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::formatChildren(LineState &State, bool NewLine,
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 -
int AdditionalIndent = State.Stack.back().Indent -
Previous.Children[0]->Level * Style.IndentWidth;
}
Penalty += format(Previous.Children, DryRun, AdditionalIndent,
/*FixBadIndentation=*/true);

View File

@ -5912,8 +5912,7 @@ TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
" 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) {

View File

@ -206,7 +206,6 @@ 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"
@ -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, MultipleFunctionLiterals) {
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, MultipleFunctionLiterals) {
" 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) {