benhamilton created this revision. benhamilton added reviewers: sammccall, MyDeveloperDay. Herald added a project: clang. Herald added a subscriber: cfe-commits. benhamilton requested review of this revision.
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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D89496 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/lib/Format/UnwrappedLineParser.h clang/unittests/Format/FormatTestObjC.cpp
Index: clang/unittests/Format/FormatTestObjC.cpp =================================================================== --- clang/unittests/Format/FormatTestObjC.cpp +++ clang/unittests/Format/FormatTestObjC.cpp @@ -328,6 +328,18 @@ "+ (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 @@ " fffffffffffff,\n" " fffffffffffff> {\n" "}"); + verifyFormat("@interface ggggggggggggg\n" + " : ggggggggggggg <ggggggggggggg>\n" + " <ggggggggggggg>\n" + "@end"); } TEST_F(FormatTestObjC, FormatObjCImplementation) { Index: clang/lib/Format/UnwrappedLineParser.h =================================================================== --- clang/lib/Format/UnwrappedLineParser.h +++ clang/lib/Format/UnwrappedLineParser.h @@ -118,6 +118,7 @@ // 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(); Index: clang/lib/Format/UnwrappedLineParser.cpp =================================================================== --- clang/lib/Format/UnwrappedLineParser.cpp +++ clang/lib/Format/UnwrappedLineParser.cpp @@ -2612,32 +2612,15 @@ // @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 @@ 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() {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits