r174364 - Initial support for formatting ObjC method declarations/calls.
Nico Weber
thakis at chromium.org
Tue Feb 5 09:40:11 PST 2013
On Tue, Feb 5, 2013 at 7:38 AM, Daniel Jasper <djasper at google.com> wrote:
> Not yet. Feel free to file bugs with examples (I don't really have ObjC code
> at my disposal)...
I filed a few bugs.
Thank you for implementing this! This is already very very useful.
>
>
> On Tue, Feb 5, 2013 at 4:06 PM, Nico Weber <thakis at chromium.org> wrote:
>>
>> Nice!
>>
>> On Tue, Feb 5, 2013 at 2:07 AM, Daniel Jasper <djasper at google.com> wrote:
>> > Author: djasper
>> > Date: Tue Feb 5 04:07:47 2013
>> > New Revision: 174364
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=174364&view=rev
>> > Log:
>> > Initial support for formatting ObjC method declarations/calls.
>> >
>> > We can now format stuff like:
>> > - (void)doSomethingWith:(GTMFoo *)theFoo
>> > rect:(NSRect)theRect
>> > interval:(float)theInterval {
>> > [myObject doFooWith:arg1 //
>> > name:arg2
>> > error:arg3];
>> >
>> > }
>> >
>> > This seems to fix everything mentioned in llvm.org/PR14939.
>> >
>> > Modified:
>> > cfe/trunk/lib/Format/Format.cpp
>> > cfe/trunk/lib/Format/TokenAnnotator.cpp
>> > cfe/trunk/lib/Format/TokenAnnotator.h
>> > 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=174364&r1=174363&r2=174364&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Format/Format.cpp (original)
>> > +++ cfe/trunk/lib/Format/Format.cpp Tue Feb 5 04:07:47 2013
>> > @@ -268,7 +268,7 @@ private:
>> > : Indent(Indent), LastSpace(LastSpace), AssignmentColumn(0),
>> > FirstLessLess(0), BreakBeforeClosingBrace(false),
>> > QuestionColumn(0),
>> > AvoidBinPacking(AvoidBinPacking), BreakAfterComma(false),
>> > - HasMultiParameterLine(HasMultiParameterLine) {
>> > + HasMultiParameterLine(HasMultiParameterLine), ColonPos(0) {
>> > }
>> >
>> > /// \brief The position to which a specific parenthesis level needs
>> > to be
>> > @@ -312,6 +312,9 @@ private:
>> > /// \brief This context already has a line with more than one
>> > parameter.
>> > bool HasMultiParameterLine;
>> >
>> > + /// \brief The position of the colon in an ObjC method
>> > declaration/call.
>> > + unsigned ColonPos;
>> > +
>> > bool operator<(const ParenState &Other) const {
>> > if (Indent != Other.Indent)
>> > return Indent < Other.Indent;
>> > @@ -331,6 +334,8 @@ private:
>> > return BreakAfterComma;
>> > if (HasMultiParameterLine != Other.HasMultiParameterLine)
>> > return HasMultiParameterLine;
>> > + if (ColonPos != Other.ColonPos)
>> > + return ColonPos < Other.ColonPos;
>> > return false;
>> > }
>> > };
>> > @@ -427,6 +432,17 @@ private:
>> > } else if (Previous.Type == TT_BinaryOperator &&
>> > State.Stack.back().AssignmentColumn != 0) {
>> > State.Column = State.Stack.back().AssignmentColumn;
>> > + } else if (Current.Type == TT_ObjCSelectorName) {
>> > + if (State.Stack.back().ColonPos >
>> > Current.FormatTok.TokenLength) {
>> > + State.Column =
>> > + State.Stack.back().ColonPos -
>> > Current.FormatTok.TokenLength;
>> > + } else {
>> > + State.Column = State.Stack.back().Indent;
>> > + State.Stack.back().ColonPos =
>> > + State.Column + Current.FormatTok.TokenLength;
>> > + }
>> > + } else if (Previous.Type == TT_ObjCMethodExpr) {
>> > + State.Column = State.Stack.back().Indent + 4;
>> > } else {
>> > State.Column = State.Stack[ParenLevel].Indent;
>> > }
>> > @@ -461,6 +477,17 @@ private:
>> > if (!DryRun)
>> > Whitespaces.replaceWhitespace(Current, 0, Spaces, State.Column,
>> > Style);
>> >
>> > + if (Current.Type == TT_ObjCSelectorName &&
>> > + State.Stack.back().ColonPos == 0) {
>> > + if (State.Stack.back().Indent + Current.LongestObjCSelectorName
>> > >
>> > + State.Column + Spaces + Current.FormatTok.TokenLength)
>> > + State.Stack.back().ColonPos =
>> > + State.Stack.back().Indent +
>> > Current.LongestObjCSelectorName;
>> > + else
>> > + State.Stack.back().ColonPos =
>> > + State.Column + Spaces + Current.LongestObjCSelectorName;
>> > + }
>> > +
>> > // FIXME: Do we need to do this for assignments nested in other
>> > // expressions?
>> > if (RootToken.isNot(tok::kw_for) && ParenLevel == 0 &&
>> > @@ -699,6 +726,9 @@ private:
>> > State.Stack.back().BreakAfterComma &&
>> > !isTrailingComment(*State.NextToken))
>> > return true;
>> > + if (State.NextToken->Type == TT_ObjCSelectorName &&
>> > + State.Stack.back().ColonPos != 0)
>> > + return true;
>> > if ((State.NextToken->Type == TT_CtorInitializerColon ||
>> > (State.NextToken->Parent->ClosesTemplateDeclaration &&
>> > State.Stack.size() == 1)))
>> >
>> > Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=174364&r1=174363&r2=174364&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
>> > +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Feb 5 04:07:47 2013
>> > @@ -20,15 +20,6 @@
>> > namespace clang {
>> > namespace format {
>> >
>> > -/// \brief Returns if a token is an Objective-C selector name.
>> > -///
>> > -/// For example, "bar" is a selector name in [foo bar:(4 + 5)].
>> > -static bool isObjCSelectorName(const AnnotatedToken &Tok) {
>> > - return Tok.is(tok::identifier) && !Tok.Children.empty() &&
>> > - Tok.Children[0].is(tok::colon) &&
>> > - Tok.Children[0].Type == TT_ObjCMethodExpr;
>> > -}
>> > -
>> > static bool isBinaryOperator(const AnnotatedToken &Tok) {
>> > // Comma is a binary operator, but does not behave as such wrt.
>> > formatting.
>> > return getPrecedence(Tok) > prec::Comma;
>> > @@ -65,6 +56,7 @@ public:
>> > AnnotatingParser(SourceManager &SourceMgr, Lexer &Lex, AnnotatedLine
>> > &Line)
>> > : SourceMgr(SourceMgr), Lex(Lex), Line(Line),
>> > CurrentToken(&Line.First),
>> > KeywordVirtualFound(false), ColonIsObjCMethodExpr(false),
>> > + LongestObjCSelectorName(0), FirstObjCSelectorName(NULL),
>>
>> Does this handle nested selectors? E.g.
>>
>> [contentsContainer replaceSubview:[subviews objectAtIndex:0]
>> with:contentsNativeView];
>>
>>
>> > ColonIsForRangeExpr(false), IsExpression(false),
>> > LookForFunctionName(Line.MustBeDeclaration), BindingStrength(1)
>> > {
>> > }
>> > @@ -82,6 +74,8 @@ public:
>> >
>> > void markStart(AnnotatedToken &Left) {
>> > P.ColonIsObjCMethodExpr = true;
>> > + P.LongestObjCSelectorName = 0;
>> > + P.FirstObjCSelectorName = NULL;
>> > Left.Type = TT_ObjCMethodExpr;
>> > }
>> >
>> > @@ -168,8 +162,13 @@ public:
>> > Left->MatchingParen = CurrentToken;
>> > CurrentToken->MatchingParen = Left;
>> >
>> > - if (StartsObjCMethodExpr)
>> > + if (StartsObjCMethodExpr) {
>> > objCSelector.markEnd(*CurrentToken);
>> > + if (FirstObjCSelectorName != NULL) {
>> > + FirstObjCSelectorName->LongestObjCSelectorName =
>> > + LongestObjCSelectorName;
>> > + }
>> > + }
>> >
>> > next();
>> > return true;
>> > @@ -221,6 +220,9 @@ public:
>> > }
>> > Left->MatchingParen = CurrentToken;
>> > CurrentToken->MatchingParen = Left;
>> > + if (FirstObjCSelectorName != NULL)
>> > + FirstObjCSelectorName->LongestObjCSelectorName =
>> > + LongestObjCSelectorName;
>> > next();
>> > return true;
>> > }
>> > @@ -295,12 +297,19 @@ public:
>> > break;
>> > case tok::colon:
>> > // Colons from ?: are handled in parseConditional().
>> > - if (Tok->Parent->is(tok::r_paren))
>> > + if (Tok->Parent->is(tok::r_paren)) {
>> > Tok->Type = TT_CtorInitializerColon;
>> > - else if (ColonIsObjCMethodExpr)
>> > + } else if (ColonIsObjCMethodExpr ||
>> > + Line.First.Type == TT_ObjCMethodSpecifier) {
>> > Tok->Type = TT_ObjCMethodExpr;
>> > - else if (ColonIsForRangeExpr)
>> > + Tok->Parent->Type = TT_ObjCSelectorName;
>> > + if (Tok->Parent->FormatTok.TokenLength >
>> > LongestObjCSelectorName)
>> > + LongestObjCSelectorName = Tok->Parent->FormatTok.TokenLength;
>> > + if (FirstObjCSelectorName == NULL)
>> > + FirstObjCSelectorName = Tok->Parent;
>> > + } else if (ColonIsForRangeExpr) {
>> > Tok->Type = TT_RangeBasedForLoopColon;
>> > + }
>> > break;
>> > case tok::kw_if:
>> > case tok::kw_while:
>> > @@ -452,6 +461,13 @@ public:
>> > if (PeriodsAndArrows >= 2 && CanBeBuilderTypeStmt)
>> > return LT_BuilderTypeCall;
>> >
>> > + if (Line.First.Type == TT_ObjCMethodSpecifier) {
>> > + if (FirstObjCSelectorName != NULL)
>> > + FirstObjCSelectorName->LongestObjCSelectorName =
>> > + LongestObjCSelectorName;
>> > + return LT_ObjCMethodDecl;
>> > + }
>> > +
>> > return LT_Other;
>> > }
>> >
>> > @@ -474,6 +490,8 @@ private:
>> > AnnotatedToken *CurrentToken;
>> > bool KeywordVirtualFound;
>> > bool ColonIsObjCMethodExpr;
>> > + unsigned LongestObjCSelectorName;
>> > + AnnotatedToken *FirstObjCSelectorName;
>> > bool ColonIsForRangeExpr;
>> > bool IsExpression;
>> > bool LookForFunctionName;
>> > @@ -725,9 +743,9 @@ unsigned TokenAnnotator::splitPenalty(co
>> >
>> > // In Objective-C method expressions, prefer breaking before "param:"
>> > over
>> > // breaking after it.
>> > - if (isObjCSelectorName(Right))
>> > + if (Right.Type == TT_ObjCSelectorName)
>> > return 0;
>> > - if (Right.is(tok::colon) && Right.Type == TT_ObjCMethodExpr)
>> > + if (Left.is(tok::colon) && Left.Type == TT_ObjCMethodExpr)
>> > return 20;
>> >
>> > if (Left.is(tok::l_paren) || Left.is(tok::l_square) ||
>> > @@ -885,7 +903,7 @@ bool TokenAnnotator::canBreakBefore(cons
>> > return false;
>> > if (Left.is(tok::colon) && Left.Type == TT_ObjCMethodExpr)
>> > return true;
>> > - if (isObjCSelectorName(Right))
>> > + if (Right.Type == TT_ObjCSelectorName)
>> > return true;
>> > if (Left.ClosesTemplateDeclaration)
>> > return true;
>> >
>> > Modified: cfe/trunk/lib/Format/TokenAnnotator.h
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=174364&r1=174363&r2=174364&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Format/TokenAnnotator.h (original)
>> > +++ cfe/trunk/lib/Format/TokenAnnotator.h Tue Feb 5 04:07:47 2013
>> > @@ -40,6 +40,7 @@ enum TokenType {
>> > TT_ObjCMethodSpecifier,
>> > TT_ObjCMethodExpr,
>> > TT_ObjCProperty,
>> > + TT_ObjCSelectorName,
>> > TT_OverloadedOperator,
>> > TT_PointerOrReference,
>> > TT_PureVirtualSpecifier,
>> > @@ -69,7 +70,8 @@ public:
>> > : FormatTok(FormatTok), Type(TT_Unknown),
>> > SpaceRequiredBefore(false),
>> > CanBreakBefore(false), MustBreakBefore(false),
>> > ClosesTemplateDeclaration(false), MatchingParen(NULL),
>> > - ParameterCount(1), BindingStrength(0), SplitPenalty(0),
>> > Parent(NULL) {
>> > + ParameterCount(1), BindingStrength(0), SplitPenalty(0),
>> > + LongestObjCSelectorName(0), Parent(NULL) {
>> > }
>> >
>> > bool is(tok::TokenKind Kind) const { return FormatTok.Tok.is(Kind); }
>> > @@ -109,6 +111,10 @@ public:
>> > /// \brief Penalty for inserting a line break before this token.
>> > unsigned SplitPenalty;
>> >
>> > + /// \brief If this is the first ObjC selector name in an ObjC method
>> > + /// definition or call, this contains the length of the longest name.
>> > + unsigned LongestObjCSelectorName;
>> > +
>> > std::vector<AnnotatedToken> Children;
>> > AnnotatedToken *Parent;
>> >
>> >
>> > Modified: cfe/trunk/unittests/Format/FormatTest.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=174364&r1=174363&r2=174364&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
>> > +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Feb 5 04:07:47 2013
>> > @@ -1979,20 +1979,18 @@ TEST_F(FormatTest, FormatForObjectiveCMe
>> > format("- (void)sendAction:(SEL)aSelector to:(id)anObject
>> > forAllCells:(BOOL)flag;"));
>> >
>> > // Very long objectiveC method declaration.
>> > - EXPECT_EQ(
>> > - "- (NSUInteger)indexOfObject:(id)anObject
>> > inRange:(NSRange)range\n "
>> > - "outRange:(NSRange)out_range outRange1:(NSRange)out_range1\n "
>> > - "outRange2:(NSRange)out_range2 outRange3:(NSRange)out_range3\n
>> > "
>> > - "outRange4:(NSRange)out_range4 outRange5:(NSRange)out_range5\n
>> > "
>> > - "outRange6:(NSRange)out_range6 outRange7:(NSRange)out_range7\n
>> > "
>> > - "outRange8:(NSRange)out_range8 outRange9:(NSRange)out_range9;",
>> > - format(
>> > - "- (NSUInteger)indexOfObject:(id)anObject
>> > inRange:(NSRange)range "
>> > - "outRange:(NSRange) out_range outRange1:(NSRange) out_range1
>> > "
>> > - "outRange2:(NSRange) out_range2 outRange3:(NSRange)
>> > out_range3 "
>> > - "outRange4:(NSRange) out_range4 outRange5:(NSRange)
>> > out_range5 "
>> > - "outRange6:(NSRange) out_range6 outRange7:(NSRange)
>> > out_range7 "
>> > - "outRange8:(NSRange) out_range8 outRange9:(NSRange)
>> > out_range9;"));
>> > + verifyFormat("- (NSUInteger)indexOfObject:(id)anObject\n"
>> > + " inRange:(NSRange)range\n"
>> > + " outRange:(NSRange)out_range\n"
>> > + " outRange1:(NSRange)out_range1\n"
>> > + " outRange2:(NSRange)out_range2\n"
>> > + " outRange3:(NSRange)out_range3\n"
>> > + " outRange4:(NSRange)out_range4\n"
>> > + " outRange5:(NSRange)out_range5\n"
>> > + " outRange6:(NSRange)out_range6\n"
>> > + " outRange7:(NSRange)out_range7\n"
>> > + " outRange8:(NSRange)out_range8\n"
>> > + " outRange9:(NSRange)out_range9;");
>> >
>> > verifyFormat("- (int)sum:(vector<int>)numbers;");
>> > verifyGoogleFormat("- (void)setDelegate:(id<Protocol>)delegate;");
>> > @@ -2218,6 +2216,18 @@ TEST_F(FormatTest, FormatObjCProtocol) {
>> > "@end\n");
>> > }
>> >
>> > +TEST_F(FormatTest, FormatObjCMethodDeclarations) {
>> > + verifyFormat("- (void)doSomethingWith:(GTMFoo *)theFoo\n"
>> > + " rect:(NSRect)theRect\n"
>> > + " interval:(float)theInterval {\n"
>> > + "}");
>> > + verifyFormat("- (void)shortf:(GTMFoo *)theFoo\n"
>> > + " longKeyword:(NSRect)theRect\n"
>> > + " evenLongerKeyword:(float)theInterval\n"
>> > + " error:(NSError **)theError {\n"
>> > + "}");
>> > +}
>> > +
>> > TEST_F(FormatTest, FormatObjCMethodExpr) {
>> > verifyFormat("[foo bar:baz];");
>> > verifyFormat("return [foo bar:baz];");
>> > @@ -2266,7 +2276,7 @@ TEST_F(FormatTest, FormatObjCMethodExpr)
>> > verifyFormat("[cond ? obj1 : obj2 methodWithParam:param]");
>> > verifyFormat("[button setAction:@selector(zoomOut:)];");
>> > verifyFormat("[color getRed:&r green:&g blue:&b alpha:&a];");
>> > -
>> > +
>> > verifyFormat("arr[[self indexForFoo:a]];");
>> > verifyFormat("throw [self errorFor:a];");
>> > verifyFormat("@throw [self errorFor:a];");
>> > @@ -2275,12 +2285,27 @@ TEST_F(FormatTest, FormatObjCMethodExpr)
>> > // which would be at 80 columns.
>> > verifyFormat(
>> > "void f() {\n"
>> > - " if ((self = [super initWithContentRect:contentRect
>> > styleMask:styleMask\n"
>> > - " backing:NSBackingStoreBuffered defer:YES]))");
>> > -
>> > + " if ((self = [super initWithContentRect:contentRect\n"
>> > + " styleMask:styleMask\n"
>> > + "
>> > backing:NSBackingStoreBuffered\n"
>> > + " defer:YES]))");
>> > +
>> > verifyFormat("[foo checkThatBreakingAfterColonWorksOk:\n"
>> > - " [bar
>> > ifItDoes:reduceOverallLineLengthLikeInThisCase]];");
>> > -
>> > + " [bar
>> > ifItDoes:reduceOverallLineLengthLikeInThisCase]];");
>> > +
>> > + verifyFormat("[myObj short:arg1 // Force line break\n"
>> > + " longKeyword:arg2\n"
>> > + " evenLongerKeyword:arg3\n"
>> > + " error:arg4];");
>> > + verifyFormat(
>> > + "void f() {\n"
>> > + " popup_window_.reset([[RenderWidgetPopupWindow alloc]\n"
>> > + " initWithContentRect:NSMakeRect(origin_global.x,
>> > origin_global.y,\n"
>> > + " pos.width(),
>> > pos.height())\n"
>> > + " styleMask:NSBorderlessWindowMask\n"
>> > + " backing:NSBackingStoreBuffered\n"
>> > + " defer:NO]);\n"
>> > + "}");
>> > }
>> >
>> > TEST_F(FormatTest, ObjCAt) {
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
More information about the cfe-commits
mailing list