Author: benhamilton Date: Thu Apr 5 08:26:23 2018 New Revision: 329297 URL: http://llvm.org/viewvc/llvm-project?rev=329297&view=rev Log: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly
Summary: Previously, clang-format would incorrectly annotate 0-argument Objective-C selector names as TT_TrailingAnnotation: ``` % echo "-(void)foo;" > /tmp/test.m % ./bin/clang-format -debug /tmp/test.m Language: Objective-C ---- Line(0, FSC=0): minus[T=68, OC=0] l_paren[T=68, OC=1] void[T=68, OC=2] r_paren[T=68, OC=6] identifier[T=68, OC=7] semi[T=68, OC=10] Line(0, FSC=0): eof[T=68, OC=0] Run 0... AnnotatedTokens(L=0): M=0 C=0 T=ObjCMethodSpecifier S=1 B=0 BK=0 P=0 Name=minus L=1 PPK=2 FakeLParens= FakeRParens=0 Text='-' M=0 C=1 T=Unknown S=1 B=0 BK=0 P=33 Name=l_paren L=3 PPK=2 FakeLParens= FakeRParens=0 Text='(' M=0 C=1 T=Unknown S=0 B=0 BK=0 P=140 Name=void L=7 PPK=2 FakeLParens= FakeRParens=0 Text='void' M=0 C=0 T=CastRParen S=0 B=0 BK=0 P=43 Name=r_paren L=8 PPK=2 FakeLParens= FakeRParens=0 Text=')' M=0 C=1 T=TrailingAnnotation S=0 B=0 BK=0 P=120 Name=identifier L=11 PPK=2 FakeLParens= FakeRParens=0 Text='foo' M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=semi L=12 PPK=2 FakeLParens= FakeRParens=0 Text=';' ``` This caused us to incorrectly indent 0-argument wrapped selectors when Style.IndentWrappedFunctionNames was false, as we thought the 0-argument ObjC selector name was actually a trailing annotation (which is always indented). This diff fixes the issue and adds tests. Test Plan: New tests added. Confirmed tests failed before diff. After diff, tests passed. Ran tests with: % make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests Reviewers: djasper, jolesiak Reviewed By: djasper, jolesiak Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D44996 Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTestObjC.cpp Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=329297&r1=329296&r2=329297&view=diff ============================================================================== --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original) +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Apr 5 08:26:23 2018 @@ -391,6 +391,7 @@ private: Parent->isOneOf(tok::colon, tok::l_square, tok::l_paren, tok::kw_return, tok::kw_throw) || Parent->isUnaryOperator() || + // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen. Parent->isOneOf(TT_ObjCForIn, TT_CastRParen) || getBinOpPrecedence(Parent->Tok.getKind(), true, true) > prec::Unknown); bool ColonFound = false; @@ -524,6 +525,7 @@ private: Left->ParameterCount = 0; Contexts.back().ColonIsObjCMethodExpr = true; if (Parent && Parent->is(tok::r_paren)) + // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen. Parent->Type = TT_CastRParen; } ColonFound = true; @@ -676,6 +678,7 @@ private: Tok->Type = TT_ObjCMethodExpr; const FormatToken *BeforePrevious = Tok->Previous->Previous; if (!BeforePrevious || + // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen. !(BeforePrevious->is(TT_CastRParen) || (BeforePrevious->is(TT_ObjCMethodExpr) && BeforePrevious->is(tok::colon))) || @@ -1343,6 +1346,17 @@ private: TT_LeadingJavaAnnotation)) { Current.Type = Current.Previous->Type; } + } else if (Current.isOneOf(tok::identifier, tok::kw_new) && + // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen. + Current.Previous && Current.Previous->is(TT_CastRParen) && + Current.Previous->MatchingParen && + Current.Previous->MatchingParen->Previous && + Current.Previous->MatchingParen->Previous->is( + TT_ObjCMethodSpecifier)) { + // This is the first part of an Objective-C selector name. (If there's no + // colon after this, this is the only place which annotates the identifier + // as a selector.) + Current.Type = TT_SelectorName; } else if (Current.isOneOf(tok::identifier, tok::kw_const) && Current.Previous && !Current.Previous->isOneOf(tok::equal, tok::at) && Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=329297&r1=329296&r2=329297&view=diff ============================================================================== --- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original) +++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Thu Apr 5 08:26:23 2018 @@ -523,6 +523,23 @@ TEST_F(FormatTestObjC, FormatObjCMethodD verifyFormat("- (void)drawRectOn:(id)surface\n" " ofSize:(size_t)height\n" " :(size_t)width;"); + Style.ColumnLimit = 40; + // Make sure selectors with 0, 1, or more arguments are not indented + // when IndentWrappedFunctionNames is false. + Style.IndentWrappedFunctionNames = false; + verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaa;\n"); + verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n"); + verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n"); + verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n"); + verifyFormat("- (aaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaa:(int)a;\n"); // Continuation indent width should win over aligning colons if the function // name is long. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits