r174364 - Initial support for formatting ObjC method declarations/calls.
Daniel Jasper
djasper at google.com
Tue Feb 5 07:38:39 PST 2013
Not yet. Feel free to file bugs with examples (I don't really have ObjC
code at my disposal)...
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130205/8e400f1e/attachment.html>
More information about the cfe-commits
mailing list