benhamilton created this revision.
benhamilton added reviewers: djasper, jolesiak.
Herald added subscribers: cfe-commits, klimek.
Please take a close look at this CL. I haven't touched much of
`UnwrappedLineParser` before, so I may have gotten things wrong.
Previously, clang-format would incorrectly format the following:
@implementation Foo
- (Class)class {
}
- (void)foo {
}
@end
as:
@implementation Foo
- (Class)class {
}
- (void)foo {
}
@end
The problem is whenever `UnwrappedLineParser::parseStructuralElement()`
sees any of the keywords `class`, `struct`, or `enum`, it calls
`parseRecord()` to parse them as a C/C++ record.
This causes subsequent lines to be parsed incorrectly, which
causes them to be indented incorrectly.
In Objective-C/Objective-C++, these keywords are valid selector
components.
This diff fixes the issue by explicitly handling `+` and `-` lines
inside `@implementation` / `@interface` / `@protocol` blocks
and parsing them as Objective-C methods.
Test Plan: New tests added. Ran tests with:
make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests
Repository:
rC Clang
https://reviews.llvm.org/D47095
Files:
lib/Format/UnwrappedLineParser.cpp
lib/Format/UnwrappedLineParser.h
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===================================================================
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -998,6 +998,59 @@
verifyFormat("MACRO(new:)\n");
verifyFormat("MACRO(delete:)\n");
verifyFormat("foo = @{MACRO(new:) : MACRO(delete:)}\n");
+ verifyFormat("@implementation Foo\n"
+ "// Testing\n"
+ "- (Class)class {\n"
+ "}\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end\n");
+ verifyFormat("@implementation Foo\n"
+ "- (Class)class {\n"
+ "}\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end");
+ verifyFormat("@implementation Foo\n"
+ "+ (Class)class {\n"
+ "}\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end");
+ verifyFormat("@implementation Foo\n"
+ "- (Class)class:(Class)klass {\n"
+ "}\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end");
+ verifyFormat("@implementation Foo\n"
+ "+ (Class)class:(Class)klass {\n"
+ "}\n"
+ "- (void)foo {\n"
+ "}\n"
+ "@end");
+
+ verifyFormat("@interface Foo\n"
+ "// Testing\n"
+ "- (Class)class;\n"
+ "- (void)foo;\n"
+ "@end\n");
+ verifyFormat("@interface Foo\n"
+ "- (Class)class;\n"
+ "- (void)foo;\n"
+ "@end");
+ verifyFormat("@interface Foo\n"
+ "+ (Class)class;\n"
+ "- (void)foo;\n"
+ "@end");
+ verifyFormat("@interface Foo\n"
+ "- (Class)class:(Class)klass;\n"
+ "- (void)foo;\n"
+ "@end");
+ verifyFormat("@interface Foo\n"
+ "+ (Class)class:(Class)klass;\n"
+ "- (void)foo;\n"
+ "@end");
}
TEST_F(FormatTestObjC, ObjCLiterals) {
Index: lib/Format/UnwrappedLineParser.h
===================================================================
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -120,6 +120,7 @@
// parses the record as a child block, i.e. if the class declaration is an
// expression.
void parseRecord(bool ParseAsExpr = false);
+ void parseObjCMethod();
void parseObjCProtocolList();
void parseObjCUntilAtEnd();
void parseObjCInterfaceOrImplementation();
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2130,6 +2130,22 @@
// "} n, m;" will end up in one unwrapped line.
}
+void UnwrappedLineParser::parseObjCMethod() {
+ assert(FormatTok->Tok.isOneOf(tok::l_paren, tok::identifier) && "'(' or identifier expected.");
+ do {
+ if (FormatTok->Tok.isOneOf(tok::semi, tok::r_brace)) {
+ nextToken();
+ addUnwrappedLine();
+ return;
+ } else if (FormatTok->Tok.is(tok::l_brace)) {
+ parseBlock(/*MustBeDeclaration=*/false);
+ addUnwrappedLine();
+ } else {
+ nextToken();
+ }
+ } while (!eof());
+}
+
void UnwrappedLineParser::parseObjCProtocolList() {
assert(FormatTok->Tok.is(tok::less) && "'<' expected.");
do {
@@ -2157,6 +2173,9 @@
// Ignore stray "}". parseStructuralElement doesn't consume them.
nextToken();
addUnwrappedLine();
+ } else if (FormatTok->isOneOf(tok::minus, tok::plus)) {
+ nextToken();
+ parseObjCMethod();
} else {
parseStructuralElement();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
