[clang] 24b5266 - [Format/ObjC] Correctly handle base class with lightweight generics and protocol

Ben Hamilton via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 16 14:12:37 PDT 2020


Author: Ben Hamilton
Date: 2020-10-16T15:12:25-06:00
New Revision: 24b5266892c30e2c9cb6ea28c2631e988a5754b6

URL: https://github.com/llvm/llvm-project/commit/24b5266892c30e2c9cb6ea28c2631e988a5754b6
DIFF: https://github.com/llvm/llvm-project/commit/24b5266892c30e2c9cb6ea28c2631e988a5754b6.diff

LOG: [Format/ObjC] Correctly handle base class with lightweight generics and protocol

ClangFormat does not correctly handle an Objective-C interface declaration
with both lightweight generics and a protocol conformance.

This simple example:

```
@interface Foo : Bar <Baz> <Blech>

@end
```

means `Foo` extends `Bar` (a lightweight generic class whose type
parameter is `Baz`) and also conforms to the protocol `Blech`.

ClangFormat should not apply any changes to the above example, but
instead it currently formats it quite poorly:

```
@interface Foo : Bar <Baz>
<Blech>

    @end
    ```

The bug is that `UnwrappedLineParser` assumes an open-angle bracket
after a base class name is a protocol list, but it can also be a
lightweight generic specification.

This diff fixes the bug by factoring out the logic to parse
lightweight generics so it can apply both to the declared class
as well as the base class.

Test Plan: New tests added. Ran tests with:
  % ninja FormatTests && ./tools/clang/unittests/Format/FormatTests
  Confirmed tests failed before diff and passed after diff.

Reviewed By: sammccall, MyDeveloperDay

Differential Revision: https://reviews.llvm.org/D89496

Added: 
    

Modified: 
    clang/lib/Format/UnwrappedLineParser.cpp
    clang/lib/Format/UnwrappedLineParser.h
    clang/unittests/Format/FormatTestObjC.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 7075a6fe33f7..ab1f647481d0 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2612,32 +2612,15 @@ void UnwrappedLineParser::parseObjCInterfaceOrImplementation() {
   // @interface can be followed by a lightweight generic
   // specialization list, then either a base class or a category.
   if (FormatTok->Tok.is(tok::less)) {
-    // Unlike protocol lists, generic parameterizations support
-    // nested angles:
-    //
-    // @interface Foo<ValueType : id <NSCopying, NSSecureCoding>> :
-    //     NSObject <NSCopying, NSSecureCoding>
-    //
-    // so we need to count how many open angles we have left.
-    unsigned NumOpenAngles = 1;
-    do {
-      nextToken();
-      // Early exit in case someone forgot a close angle.
-      if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
-          FormatTok->Tok.isObjCAtKeyword(tok::objc_end))
-        break;
-      if (FormatTok->Tok.is(tok::less))
-        ++NumOpenAngles;
-      else if (FormatTok->Tok.is(tok::greater)) {
-        assert(NumOpenAngles > 0 && "'>' makes NumOpenAngles negative");
-        --NumOpenAngles;
-      }
-    } while (!eof() && NumOpenAngles != 0);
-    nextToken(); // Skip '>'.
+    parseObjCLightweightGenerics();
   }
   if (FormatTok->Tok.is(tok::colon)) {
     nextToken();
     nextToken(); // base class name
+    // The base class can also have lightweight generics applied to it.
+    if (FormatTok->Tok.is(tok::less)) {
+      parseObjCLightweightGenerics();
+    }
   } else if (FormatTok->Tok.is(tok::l_paren))
     // Skip category, if present.
     parseParens();
@@ -2658,6 +2641,32 @@ void UnwrappedLineParser::parseObjCInterfaceOrImplementation() {
   parseObjCUntilAtEnd();
 }
 
+void UnwrappedLineParser::parseObjCLightweightGenerics() {
+  assert(FormatTok->Tok.is(tok::less));
+  // Unlike protocol lists, generic parameterizations support
+  // nested angles:
+  //
+  // @interface Foo<ValueType : id <NSCopying, NSSecureCoding>> :
+  //     NSObject <NSCopying, NSSecureCoding>
+  //
+  // so we need to count how many open angles we have left.
+  unsigned NumOpenAngles = 1;
+  do {
+    nextToken();
+    // Early exit in case someone forgot a close angle.
+    if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
+        FormatTok->Tok.isObjCAtKeyword(tok::objc_end))
+      break;
+    if (FormatTok->Tok.is(tok::less))
+      ++NumOpenAngles;
+    else if (FormatTok->Tok.is(tok::greater)) {
+      assert(NumOpenAngles > 0 && "'>' makes NumOpenAngles negative");
+      --NumOpenAngles;
+    }
+  } while (!eof() && NumOpenAngles != 0);
+  nextToken(); // Skip '>'.
+}
+
 // Returns true for the declaration/definition form of @protocol,
 // false for the expression form.
 bool UnwrappedLineParser::parseObjCProtocol() {

diff  --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 8b3aa4c84edb..d682b4d6acd8 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -118,6 +118,7 @@ class UnwrappedLineParser {
   // parses the record as a child block, i.e. if the class declaration is an
   // expression.
   void parseRecord(bool ParseAsExpr = false);
+  void parseObjCLightweightGenerics();
   void parseObjCMethod();
   void parseObjCProtocolList();
   void parseObjCUntilAtEnd();

diff  --git a/clang/unittests/Format/FormatTestObjC.cpp b/clang/unittests/Format/FormatTestObjC.cpp
index f3be942ab913..21b8d4701271 100644
--- a/clang/unittests/Format/FormatTestObjC.cpp
+++ b/clang/unittests/Format/FormatTestObjC.cpp
@@ -328,6 +328,18 @@ TEST_F(FormatTestObjC, FormatObjCInterface) {
                "+ (id)init;\n"
                "@end");
 
+  verifyFormat("@interface Foo<Bar : Baz <Blech>> : Xyzzy <Corge> <Quux> {\n"
+               "  int _i;\n"
+               "}\n"
+               "+ (id)init;\n"
+               "@end");
+
+  verifyFormat("@interface Foo : Bar <Baz> <Blech>\n"
+               "@end");
+
+  verifyFormat("@interface Foo : Bar <Baz> <Blech, Xyzzy, Corge>\n"
+               "@end");
+
   verifyFormat("@interface Foo (HackStuff) {\n"
                "  int _i;\n"
                "}\n"
@@ -413,6 +425,10 @@ TEST_F(FormatTestObjC, FormatObjCInterface) {
                "    fffffffffffff,\n"
                "    fffffffffffff> {\n"
                "}");
+  verifyFormat("@interface ggggggggggggg\n"
+               "    : ggggggggggggg <ggggggggggggg>\n"
+               "      <ggggggggggggg>\n"
+               "@end");
 }
 
 TEST_F(FormatTestObjC, FormatObjCImplementation) {


        


More information about the cfe-commits mailing list