Author: jolesiak Date: Wed Feb 7 02:35:08 2018 New Revision: 324469 URL: http://llvm.org/viewvc/llvm-project?rev=324469&view=rev Log: [clang-format] Fix ObjC message arguments formatting.
Summary: Fixes formatting of ObjC message arguments when inline block is a first argument. Having inline block as a first argument when method has multiple parameters is discouraged by Apple: "It’s best practice to use only one block argument to a method. If the method also needs other non-block arguments, the block should come last" (https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/WorkingwithBlocks/WorkingwithBlocks.html#//apple_ref/doc/uid/TP40011210-CH8-SW7), it should be correctly formatted nevertheless. Current formatting: ``` [object blockArgument:^{ a = 42; } anotherArg:42]; ``` Fixed (colon alignment): ``` [object blockArgument:^{ a = 42; } anotherArg:42]; ``` Test Plan: make -j12 FormatTests && tools/clang/unittests/Format/FormatTests Reviewers: krasimir, benhamilton Reviewed By: krasimir, benhamilton Subscribers: benhamilton, klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D42493 Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp cfe/trunk/lib/Format/FormatToken.h cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTestObjC.cpp Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=324469&r1=324468&r2=324469&view=diff ============================================================================== --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original) +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Wed Feb 7 02:35:08 2018 @@ -266,6 +266,11 @@ bool ContinuationIndenter::mustBreak(con return true; if (Previous.is(tok::semi) && State.LineContainsContinuedForLoopSection) return true; + if (Style.Language == FormatStyle::LK_ObjC && + Current.ObjCSelectorNameParts > 1 && + Current.startsSequence(TT_SelectorName, tok::colon, tok::caret)) { + return true; + } if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) || (Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) && Style.isCpp() && Modified: cfe/trunk/lib/Format/FormatToken.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=324469&r1=324468&r2=324469&view=diff ============================================================================== --- cfe/trunk/lib/Format/FormatToken.h (original) +++ cfe/trunk/lib/Format/FormatToken.h Wed Feb 7 02:35:08 2018 @@ -240,6 +240,10 @@ struct FormatToken { /// e.g. because several of them are block-type. unsigned LongestObjCSelectorName = 0; + /// \brief How many parts ObjC selector have (i.e. how many parameters method + /// has). + unsigned ObjCSelectorNameParts = 0; + /// \brief Stores the number of required fake parentheses and the /// corresponding operator precedence. /// Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=324469&r1=324468&r2=324469&view=diff ============================================================================== --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original) +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Feb 7 02:35:08 2018 @@ -411,6 +411,8 @@ private: if (Contexts.back().FirstObjCSelectorName) { Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = Contexts.back().LongestObjCSelectorName; + Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts = + Left->ParameterCount; if (Left->BlockParameterCount > 1) Contexts.back().FirstObjCSelectorName->LongestObjCSelectorName = 0; } @@ -424,6 +426,11 @@ private: TT_DesignatedInitializerLSquare)) { Left->Type = TT_ObjCMethodExpr; StartsObjCMethodExpr = true; + // ParameterCount might have been set to 1 before expression was + // recognized as ObjCMethodExpr (as '1 + number of commas' formula is + // used for other expression types). Parameter counter has to be, + // therefore, reset to 0. + Left->ParameterCount = 0; Contexts.back().ColonIsObjCMethodExpr = true; if (Parent && Parent->is(tok::r_paren)) Parent->Type = TT_CastRParen; @@ -498,7 +505,10 @@ private: void updateParameterCount(FormatToken *Left, FormatToken *Current) { if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block) ++Left->BlockParameterCount; - if (Current->is(tok::comma)) { + if (Left->Type == TT_ObjCMethodExpr) { + if (Current->is(tok::colon)) + ++Left->ParameterCount; + } else if (Current->is(tok::comma)) { ++Left->ParameterCount; if (!Left->Role) Left->Role.reset(new CommaSeparatedList(Style)); Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=324469&r1=324468&r2=324469&view=diff ============================================================================== --- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original) +++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Wed Feb 7 02:35:08 2018 @@ -693,6 +693,39 @@ TEST_F(FormatTestObjC, FormatObjCMethodE " ofSize:aa:bbb\n" " atOrigin:cc:dd];"); + // Inline block as a first argument. + verifyFormat("[object justBlock:^{\n" + " a = 42;\n" + "}];"); + verifyFormat("[object\n" + " justBlock:^{\n" + " a = 42;\n" + " }\n" + " notBlock:42\n" + " a:42];"); + verifyFormat("[object\n" + " firstBlock:^{\n" + " a = 42;\n" + " }\n" + " blockWithLongerName:^{\n" + " a = 42;\n" + " }];"); + verifyFormat("[object\n" + " blockWithLongerName:^{\n" + " a = 42;\n" + " }\n" + " secondBlock:^{\n" + " a = 42;\n" + " }];"); + verifyFormat("[object\n" + " firstBlock:^{\n" + " a = 42;\n" + " }\n" + " notBlock:42\n" + " secondBlock:^{\n" + " a = 42;\n" + " }];"); + Style.ColumnLimit = 70; verifyFormat( "void f() {\n" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
