clang-format: Fix various problems in formatting ObjC blocks.

Among other things, this fixes llvm.org/PR15269.

llvm-svn: 197900
This commit is contained in:
Daniel Jasper 2013-12-23 07:29:06 +00:00
parent a650116adb
commit b88b25feec
7 changed files with 177 additions and 31 deletions

View File

@ -177,10 +177,8 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
State.Stack.back().FirstLessLess == 0)
return true;
// FIXME: Comparing LongestObjCSelectorName to 0 is a hacky way of finding
// out whether it is the first parameter. Clean this up.
if (Current.Type == TT_ObjCSelectorName &&
Current.LongestObjCSelectorName == 0 &&
State.Stack.back().ObjCSelectorNameFound &&
State.Stack.back().BreakBeforeParameter)
return true;
if (Current.Type == TT_CtorInitializerColon &&
@ -258,9 +256,12 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
Whitespaces.replaceWhitespace(Current, /*Newlines=*/0, /*IndentLevel=*/0,
Spaces, State.Column + Spaces);
if (Current.Type == TT_ObjCSelectorName && State.Stack.back().ColonPos == 0) {
if (State.Stack.back().Indent + Current.LongestObjCSelectorName >
State.Column + Spaces + Current.ColumnWidth)
if (Current.Type == TT_ObjCSelectorName &&
!State.Stack.back().ObjCSelectorNameFound) {
if (Current.LongestObjCSelectorName == 0)
State.Stack.back().AlignColons = false;
else if (State.Stack.back().Indent + Current.LongestObjCSelectorName >
State.Column + Spaces + Current.ColumnWidth)
State.Stack.back().ColonPos =
State.Stack.back().Indent + Current.LongestObjCSelectorName;
else
@ -280,7 +281,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
// 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 + 1; // 1 is length of "(".
else if (Previous.is(tok::comma) || Previous.Type == TT_ObjCMethodExpr)
else if (Current.isNot(tok::comment) &&
(Previous.is(tok::comma) ||
(Previous.is(tok::colon) && Previous.Type == TT_ObjCMethodExpr)))
State.Stack.back().LastSpace = State.Column;
else if ((Previous.Type == TT_BinaryOperator ||
Previous.Type == TT_ConditionalExpr ||
@ -381,10 +384,17 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
State.Column =
std::max(State.Stack.back().LastSpace, State.Stack.back().Indent);
} else if (Current.Type == TT_ObjCSelectorName) {
if (State.Stack.back().ColonPos == 0) {
State.Stack.back().ColonPos =
State.Stack.back().Indent + Current.LongestObjCSelectorName;
State.Column = State.Stack.back().ColonPos - Current.ColumnWidth;
if (!State.Stack.back().ObjCSelectorNameFound) {
if (Current.LongestObjCSelectorName == 0) {
State.Column = State.Stack.back().Indent;
State.Stack.back().AlignColons = false;
} else {
State.Stack.back().ColonPos =
State.Stack.back().Indent + Current.LongestObjCSelectorName;
State.Column = State.Stack.back().ColonPos - Current.ColumnWidth;
}
} else if (!State.Stack.back().AlignColons) {
State.Column = State.Stack.back().Indent;
} else if (State.Stack.back().ColonPos > Current.ColumnWidth) {
State.Column = State.Stack.back().ColonPos - Current.ColumnWidth;
} else {
@ -397,9 +407,21 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
else
State.Column = ContinuationIndent;
} else if (Current.Type == TT_StartOfName ||
Previous.isOneOf(tok::coloncolon, tok::equal) ||
Previous.Type == TT_ObjCMethodExpr) {
Previous.isOneOf(tok::coloncolon, tok::equal)) {
State.Column = ContinuationIndent;
} else if (PreviousNonComment &&
PreviousNonComment->Type == TT_ObjCMethodExpr) {
State.Column = ContinuationIndent;
// FIXME: This is hacky, find a better way. The problem is that in an ObjC
// method expression, the block should be aligned to the line starting it,
// e.g.:
// [aaaaaaaaaaaaaaa aaaaaaaaa: \\ break for some reason
// ^(int *i) {
// // ...
// }];
// Thus, we set LastSpace of the next higher ParenLevel, to which we move
// when we consume all of the "}"'s FakeRParens at the "{".
State.Stack[State.Stack.size() - 2].LastSpace = ContinuationIndent;
} else if (Current.Type == TT_CtorInitializerColon) {
State.Column = State.FirstIndent + Style.ConstructorInitializerIndentWidth;
} else if (Current.Type == TT_CtorInitializerComma) {
@ -449,7 +471,7 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
!PreviousNonComment->isOneOf(tok::comma, tok::semi) &&
PreviousNonComment->Type != TT_TemplateCloser &&
PreviousNonComment->Type != TT_BinaryOperator &&
Current.Type != TT_BinaryOperator &&
Current.Type != TT_BinaryOperator &&
!PreviousNonComment->opensScope())
State.Stack.back().BreakBeforeParameter = true;
@ -494,6 +516,8 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
if (Current.isMemberAccess())
State.Stack.back().StartOfFunctionCall =
Current.LastInChainOfCalls ? 0 : State.Column + Current.ColumnWidth;
if (Current.Type == TT_ObjCSelectorName)
State.Stack.back().ObjCSelectorNameFound = true;
if (Current.Type == TT_CtorInitializerColon) {
// Indent 2 from the column, so:
// SomeClass::SomeClass()
@ -588,7 +612,16 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
// });
for (unsigned i = 0; i != Current.MatchingParen->FakeRParens; ++i)
State.Stack.pop_back();
NewIndent = State.Stack.back().LastSpace + Style.IndentWidth;
bool IsObjCBlock =
Previous &&
(Previous->is(tok::caret) ||
(Previous->is(tok::r_paren) && Previous->MatchingParen &&
Previous->MatchingParen->Previous &&
Previous->MatchingParen->Previous->is(tok::caret)));
// For some reason, ObjC blocks are indented like continuations.
NewIndent =
State.Stack.back().LastSpace +
(IsObjCBlock ? Style.ContinuationIndentWidth : Style.IndentWidth);
++NewIndentLevel;
BreakBeforeParameter = true;
} else {

View File

@ -133,7 +133,8 @@ struct ParenState {
NoLineBreak(NoLineBreak), ColonPos(0), StartOfFunctionCall(0),
StartOfArraySubscripts(0), NestedNameSpecifierContinuation(0),
CallContinuation(0), VariablePos(0), ContainsLineBreak(false),
ContainsUnwrappedBuilder(0) {}
ContainsUnwrappedBuilder(0), AlignColons(true),
ObjCSelectorNameFound(false) {}
/// \brief The position to which a specific parenthesis level needs to be
/// indented.
@ -210,6 +211,20 @@ struct ParenState {
/// builder-type call on one line.
bool ContainsUnwrappedBuilder;
/// \brief \c true if the colons of the curren ObjC method expression should
/// be aligned.
///
/// Not considered for memoization as it will always have the same value at
/// the same token.
bool AlignColons;
/// \brief \c true if at least one selector name was found in the current
/// ObjC method expression.
///
/// Not considered for memoization as it will always have the same value at
/// the same token.
bool ObjCSelectorNameFound;
bool operator<(const ParenState &Other) const {
if (Indent != Other.Indent)
return Indent < Other.Indent;

View File

@ -217,6 +217,9 @@ struct FormatToken {
/// \brief If this is the first ObjC selector name in an ObjC method
/// definition or call, this contains the length of the longest name.
///
/// This being set to 0 means that the selectors should not be colon-aligned,
/// e.g. because several of them are block-type.
unsigned LongestObjCSelectorName;
/// \brief Stores the number of required fake parentheses and the

View File

@ -84,7 +84,7 @@ private:
bool StartsObjCMethodExpr = false;
FormatToken *Left = CurrentToken->Previous;
if (CurrentToken->is(tok::caret)) {
// ^( starts a block.
// (^ can start a block type.
Left->Type = TT_ObjCBlockLParen;
} else if (FormatToken *MaybeSel = Left->Previous) {
// @selector( starts a selector.
@ -103,6 +103,10 @@ private:
Left->Previous->MatchingParen->Type == TT_LambdaLSquare) {
// This is a parameter list of a lambda expression.
Contexts.back().IsExpression = false;
} else if (Left->Previous && Left->Previous->is(tok::caret) &&
Left->Previous->Type == TT_UnaryOperator) {
// This is the parameter list of an ObjC block.
Contexts.back().IsExpression = false;
}
if (StartsObjCMethodExpr) {
@ -226,9 +230,12 @@ private:
}
Left->MatchingParen = CurrentToken;
CurrentToken->MatchingParen = Left;
if (Contexts.back().FirstObjCSelectorName != NULL)
if (Contexts.back().FirstObjCSelectorName != NULL) {
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName =
Contexts.back().LongestObjCSelectorName;
if (Contexts.back().NumBlockParameters > 1)
Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0;
}
next();
return true;
}
@ -555,15 +562,16 @@ private:
Context(tok::TokenKind ContextKind, unsigned BindingStrength,
bool IsExpression)
: ContextKind(ContextKind), BindingStrength(BindingStrength),
LongestObjCSelectorName(0), ColonIsForRangeExpr(false),
ColonIsDictLiteral(false), ColonIsObjCMethodExpr(false),
FirstObjCSelectorName(NULL), FirstStartOfName(NULL),
IsExpression(IsExpression), CanBeExpression(true),
InCtorInitializer(false) {}
LongestObjCSelectorName(0), NumBlockParameters(0),
ColonIsForRangeExpr(false), ColonIsDictLiteral(false),
ColonIsObjCMethodExpr(false), FirstObjCSelectorName(NULL),
FirstStartOfName(NULL), IsExpression(IsExpression),
CanBeExpression(true), InCtorInitializer(false) {}
tok::TokenKind ContextKind;
unsigned BindingStrength;
unsigned LongestObjCSelectorName;
unsigned NumBlockParameters;
bool ColonIsForRangeExpr;
bool ColonIsDictLiteral;
bool ColonIsObjCMethodExpr;
@ -650,6 +658,8 @@ private:
Contexts.back().IsExpression);
} else if (Current.isOneOf(tok::minus, tok::plus, tok::caret)) {
Current.Type = determinePlusMinusCaretUsage(Current);
if (Current.Type == TT_UnaryOperator)
++Contexts.back().NumBlockParameters;
} else if (Current.isOneOf(tok::minusminus, tok::plusplus)) {
Current.Type = determineIncrementUsage(Current);
} else if (Current.is(tok::exclaim)) {
@ -916,8 +926,11 @@ public:
int CurrentPrecedence = getCurrentPrecedence();
if (Current && Current->Type == TT_ObjCSelectorName &&
Precedence == CurrentPrecedence)
Precedence == CurrentPrecedence) {
if (LatestOperator)
addFakeParenthesis(Start, prec::Level(Precedence));
Start = Current;
}
// At the end of the line or when an operator with higher precedence is
// found, insert fake parenthesis and return.
@ -1077,7 +1090,7 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
else
Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
} else if (Current->SpacesRequiredBefore == 0 &&
} else if (Current->SpacesRequiredBefore == 0 &&
spaceRequiredBefore(Line, *Current)) {
Current->SpacesRequiredBefore = 1;
}

View File

@ -740,7 +740,7 @@ void UnwrappedLineParser::parseStructuralElement() {
}
break;
case tok::l_square:
tryToParseLambda();
parseSquare();
break;
default:
nextToken();
@ -749,18 +749,18 @@ void UnwrappedLineParser::parseStructuralElement() {
} while (!eof());
}
void UnwrappedLineParser::tryToParseLambda() {
bool UnwrappedLineParser::tryToParseLambda() {
// FIXME: This is a dirty way to access the previous token. Find a better
// solution.
if (!Line->Tokens.empty() &&
Line->Tokens.back().Tok->isOneOf(tok::identifier, tok::kw_operator)) {
nextToken();
return;
return false;
}
assert(FormatTok->is(tok::l_square));
FormatToken &LSquare = *FormatTok;
if (!tryToParseLambdaIntroducer())
return;
return false;
while (FormatTok->isNot(tok::l_brace)) {
switch (FormatTok->Tok.getKind()) {
@ -774,11 +774,12 @@ void UnwrappedLineParser::tryToParseLambda() {
nextToken();
break;
default:
return;
return true;
}
}
LSquare.Type = TT_LambdaLSquare;
parseChildBlock();
return true;
}
bool UnwrappedLineParser::tryToParseLambdaIntroducer() {
@ -950,6 +951,43 @@ void UnwrappedLineParser::parseParens() {
} while (!eof());
}
void UnwrappedLineParser::parseSquare() {
assert(FormatTok->Tok.is(tok::l_square) && "'[' expected.");
if (tryToParseLambda())
return;
do {
// llvm::errs() << FormatTok->Tok.getName() << "\n";
switch (FormatTok->Tok.getKind()) {
case tok::l_paren:
parseParens();
break;
case tok::r_square:
nextToken();
return;
case tok::r_brace:
// A "}" inside parenthesis is an error if there wasn't a matching "{".
return;
case tok::l_square:
parseSquare();
break;
case tok::l_brace: {
if (!tryToParseBracedList()) {
parseChildBlock();
}
break;
}
case tok::at:
nextToken();
if (FormatTok->Tok.is(tok::l_brace))
parseBracedList();
break;
default:
nextToken();
break;
}
} while (!eof());
}
void UnwrappedLineParser::parseIfThenElse() {
assert(FormatTok->Tok.is(tok::kw_if) && "'if' expected");
nextToken();

View File

@ -84,6 +84,7 @@ private:
bool parseBracedList(bool ContinueOnSemicolons = false);
void parseReturn();
void parseParens();
void parseSquare();
void parseIfThenElse();
void parseForOrWhileLoop();
void parseDoWhile();
@ -98,7 +99,7 @@ private:
void parseObjCUntilAtEnd();
void parseObjCInterfaceOrImplementation();
void parseObjCProtocol();
void tryToParseLambda();
bool tryToParseLambda();
bool tryToParseLambdaIntroducer();
void addUnwrappedLine();
bool eof() const;

View File

@ -7816,6 +7816,49 @@ TEST_F(FormatTest, FormatsBlocks) {
verifyFormat("[operation setCompletionBlock:^{ [self onOperationDone]; }];");
verifyFormat("int i = {[operation setCompletionBlock : ^{ [self "
"onOperationDone]; }] };");
verifyFormat("[operation setCompletionBlock:^(int *i) { f(); }];");
verifyFormat("[operation setCompletionBlock:^{\n"
" [self.delegate newDataAvailable];\n"
"}];",
getLLVMStyleWithColumns(60));
verifyFormat("dispatch_async(_fileIOQueue, ^{\n"
" NSString *path = [self sessionFilePath];\n"
" if (path) {\n"
" // ...\n"
" }\n"
"});");
verifyFormat("[[SessionService sharedService]\n"
" loadWindowWithCompletionBlock:^(SessionWindow *window) {\n"
" if (window) {\n"
" [self windowDidLoad:window];\n"
" } else {\n"
" [self errorLoadingWindow];\n"
" }\n"
" }];");
verifyFormat("void (^largeBlock)(void) = ^{\n"
" // ...\n"
"};\n",
getLLVMStyleWithColumns(40));
verifyFormat("[[SessionService sharedService]\n"
" loadWindowWithCompletionBlock: //\n"
" ^(SessionWindow *window) {\n"
" if (window) {\n"
" [self windowDidLoad:window];\n"
" } else {\n"
" [self errorLoadingWindow];\n"
" }\n"
" }];",
getLLVMStyleWithColumns(60));
verifyFormat("[myObject doSomethingWith:arg1\n"
" firstBlock:^(Foo *a) {\n"
" // ...\n"
" int i;\n"
" }\n"
" secondBlock:^(Bar *b) {\n"
" // ...\n"
" int i;\n"
" }];");
}
TEST_F(FormatTest, SupportsCRLF) {