benhamilton added a comment. Thanks, fixed!
================ Comment at: lib/Format/UnwrappedLineParser.cpp:2135 + nextToken(); + if (FormatTok->Tok.is(tok::less)) + NumOpenAngles++; ---------------- djasper wrote: > 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? 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`. ================ Comment at: lib/Format/UnwrappedLineParser.cpp:2136 + if (FormatTok->Tok.is(tok::less)) + NumOpenAngles++; + else if (FormatTok->Tok.is(tok::greater)) ---------------- djasper wrote: > I think we generally prefer: > > ++NumOpenAngles; Fixed. ================ Comment at: lib/Format/UnwrappedLineParser.cpp:2138 + else if (FormatTok->Tok.is(tok::greater)) + NumOpenAngles--; + } while (!eof() && NumOpenAngles != 0); ---------------- djasper wrote: > 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. Assert added. ================ Comment at: lib/Format/UnwrappedLineParser.cpp:2181 + if (FormatTok->Tok.is(tok::less)) + parseObjCLightweightGenericList(); + ---------------- djasper wrote: > 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. 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; ^ ``` ================ Comment at: lib/Format/UnwrappedLineParser.cpp:2182 + parseObjCLightweightGenericList(); + if (FormatTok->Tok.is(tok::colon)) { ---------------- djasper wrote: > nitpick: As the comment refers to this following block as well, I'd remove > the empty line. Done. Repository: rC Clang https://reviews.llvm.org/D45185 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits