r197900 - clang-format: Fix various problems in formatting ObjC blocks.
Daniel Jasper
djasper at google.com
Sun Dec 22 23:29:07 PST 2013
Author: djasper
Date: Mon Dec 23 01:29:06 2013
New Revision: 197900
URL: http://llvm.org/viewvc/llvm-project?rev=197900&view=rev
Log:
clang-format: Fix various problems in formatting ObjC blocks.
Among other things, this fixes llvm.org/PR15269.
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/ContinuationIndenter.h
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.h
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=197900&r1=197899&r2=197900&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Dec 23 01:29:06 2013
@@ -177,10 +177,8 @@ bool ContinuationIndenter::mustBreak(con
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::addTokenOnCur
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::addTokenOnCur
// 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::addTokenO
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::addTokenO
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::addTokenO
!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::moveState
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::moveState
// });
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 {
Modified: cfe/trunk/lib/Format/ContinuationIndenter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.h?rev=197900&r1=197899&r2=197900&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.h (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.h Mon Dec 23 01:29:06 2013
@@ -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;
Modified: cfe/trunk/lib/Format/FormatToken.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=197900&r1=197899&r2=197900&view=diff
==============================================================================
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Mon Dec 23 01:29:06 2013
@@ -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
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=197900&r1=197899&r2=197900&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon Dec 23 01:29:06 2013
@@ -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::calculateFormatting
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;
}
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=197900&r1=197899&r2=197900&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon Dec 23 01:29:06 2013
@@ -740,7 +740,7 @@ void UnwrappedLineParser::parseStructura
}
break;
case tok::l_square:
- tryToParseLambda();
+ parseSquare();
break;
default:
nextToken();
@@ -749,18 +749,18 @@ void UnwrappedLineParser::parseStructura
} 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::tryToParseLamb
nextToken();
break;
default:
- return;
+ return true;
}
}
LSquare.Type = TT_LambdaLSquare;
parseChildBlock();
+ return true;
}
bool UnwrappedLineParser::tryToParseLambdaIntroducer() {
@@ -934,6 +935,43 @@ void UnwrappedLineParser::parseParens()
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::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;
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=197900&r1=197899&r2=197900&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h Mon Dec 23 01:29:06 2013
@@ -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;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=197900&r1=197899&r2=197900&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Dec 23 01:29:06 2013
@@ -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) {
More information about the cfe-commits
mailing list