r174364 - Initial support for formatting ObjC method declarations/calls.
Nico Weber
thakis at chromium.org
Tue Feb 5 07:06:29 PST 2013
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