On Tue, Feb 5, 2013 at 7:38 AM, Daniel Jasper <[email protected]> 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 <[email protected]> wrote: >> >> Nice! >> >> On Tue, Feb 5, 2013 at 2:07 AM, Daniel Jasper <[email protected]> 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 >> > [email protected] >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
