[clang-format] Support lightweight Objective-C generics

[benhamilton]

Summary

Previously, clang-format didn't understand lightweight
Objective-C generics, which have the form:

@interface Foo <KeyType,
                ValueTypeWithConstraint : Foo,
		AnotherValueTypeWithGenericConstraint: Bar<Baz>, ... > ...

The lightweight generic specifier list appears before the base
class, if present, but because it starts with < like the protocol
specifier list, UnwrappedLineParser was getting confused and
failed to parse interfaces with both generics and protocol lists:

@interface Foo <KeyType> : NSObject <NSCopying>

Since the parsed line would be incomplete, the format result
would be very confused (e.g., https://bugs.llvm.org/show_bug.cgi?id=24381).

This fixes the issue by explicitly parsing the ObjC lightweight
generic conformance list, so the line is fully parsed.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=24381

Test Plan: New tests added. Ran tests with:

% make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests

[djasper]

lib/Format/UnwrappedLineParser.cpp

2135

The UnwrappedLineParser is very much about error recovery. Implemented like this, it will consume the rest of the file if someone forgets to add the closing ">", which can very easily happen when formatting in an editor while coding.

Are there things (e.g. semicolons and braces) that clearly point to this having happened so that clang-format can recover?

2136

I think we generally prefer:

++NumOpenAngles;

2138

I am slightly worried that this loop might be getting more complicated and it will be harder to determine that NumOpenAngles cannot be 0 here. Might be worth adding an assert. But might be overkill.

2165

Are there more places where we might want to parse a lightweight generic list? If this is going to remain they only instance, I think I'd prefer a local lambda or just inlining the code. But up to you.

2166

nitpick: As the comment refers to this following block as well, I'd remove the empty line.

[benhamilton]

• @djasper fixes

[benhamilton]

Thanks, fixed!

lib/Format/UnwrappedLineParser.cpp

2135

Good call. I had modeled this after UnwrappedLineParser::parseObjCProtocolList(), which has the same issue.

Fixed both that method and this one to exit early on semicolon, l_brace, and @end.

2136

Fixed.

2138

Assert added.

2165

It's just the one place, so inlined the code.

Protocol lists can appear both after @interface and @protocol, but lightweight generics are only for @interface. I confirmed this with a quick test — changing @protocol to @interface in the example below allows it to compile:

% cat test.m
#import <Foundation/Foundation.h>

@protocol Foo <KeyType> : NSObject
- (id)foo:(KeyType)aKey;
@end

lang -c test.m                      
cattest.m:3:16: error: cannot find protocol declaration for 'KeyType'
@protocol Foo <KeyType> : NSObject
               ^
test.m:3:25: error: expected identifier or '('
@protocol Foo <KeyType> : NSObject
                        ^
test.m:4:12: error: expected a type
- (id)foo:(KeyType)aKey;
           ^

2166

Done.

[djasper]

Looks good, thank you!

lib/Format/UnwrappedLineParser.cpp

void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
   }
   // There is no addUnwrappedLine() here so that we fall through to parsing a
   // structural element afterwards. Thus, in "class A {} n, m;",
   // "} n, m;" will end up in one unwrapped line.
 }
 
 void UnwrappedLineParser::parseObjCProtocolList() {
   assert(FormatTok->Tok.is(tok::less) && "'<' expected.");
-  do
+  do {
     nextToken();
-  while (!eof() && FormatTok->Tok.isNot(tok::greater));
+    // Early exit in case someone forgot a close angle.
+    if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
+        FormatTok->Tok.isObjCAtKeyword(tok::objc_end))
+      return;
+  } while (!eof() && FormatTok->Tok.isNot(tok::greater));
   nextToken(); // Skip '>'.
 }
 
 void UnwrappedLineParser::parseObjCUntilAtEnd() {
   do {
     if (FormatTok->Tok.isObjCAtKeyword(tok::objc_end)) {
       nextToken();
       addUnwrappedLine();
       break;
     }
     if (FormatTok->is(tok::l_brace)) {
       parseBlock(/*MustBeDeclaration=*/false);
       // In ObjC interfaces, nothing should be following the "}".
       addUnwrappedLine();
     } else if (FormatTok->is(tok::r_brace)) {
       // Ignore stray "}". parseStructuralElement doesn't consume them.
       nextToken();
       addUnwrappedLine();
     } else {
       parseStructuralElement();
     }
   } while (!eof());
 }
 
 void UnwrappedLineParser::parseObjCInterfaceOrImplementation() {
   assert(FormatTok->Tok.getObjCKeywordID() == tok::objc_interface ||
          FormatTok->Tok.getObjCKeywordID() == tok::objc_implementation);
   nextToken();
   nextToken(); // interface name
 
-  // @interface can be followed by either a base class, or a category.
+  // @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 '>'.
+  }
   if (FormatTok->Tok.is(tok::colon)) {
     nextToken();
     nextToken(); // base class name
   } else if (FormatTok->Tok.is(tok::l_paren))
     // Skip category, if present.
     parseParens();
 
   if (FormatTok->Tok.is(tok::less))

unittests/Format/FormatTestObjC.cpp

verifyFormat("@interface Foo : Bar {\n"
                "@end");
 
   verifyFormat("@interface Foo : Bar <Baz, Quux> {\n"
                "  int _i;\n"
                "}\n"
                "+ (id)init;\n"
                "@end");
 
+  verifyFormat("@interface Foo <Baz : Blech> : Bar <Baz, Quux> {\n"
+               "  int _i;\n"
+               "}\n"
+               "+ (id)init;\n"
+               "@end");
+
+  verifyFormat("@interface Foo <Bar : Baz <Blech>> : Xyzzy <Corge> {\n"
+               "  int _i;\n"
+               "}\n"
+               "+ (id)init;\n"
+               "@end");
+
   verifyFormat("@interface Foo (HackStuff) {\n"
                "  int _i;\n"
                "}\n"
                "+ (id)init;\n"
                "@end");
 
   verifyFormat("@interface Foo () {\n"
                "  int _i;\n"