r329297 - [clang-format] Ensure ObjC selectors with 0 args are annotated correctly

Ben Hamilton via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 5 08:26:23 PDT 2018


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.




More information about the cfe-commits mailing list