From: Daniel Jasper Date: Fri, 31 May 2013 14:56:20 +0000 (+0000) Subject: Make formatting of empty blocks more consistent. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8893b8a28d32359a5c778d9308780200a09402ba;p=clang Make formatting of empty blocks more consistent. With this patch, the simplified rule is: If the block is part of a declaration (class, namespace, function, enum, ..), merge an empty block onto a single line. Otherwise (specifically for the compound statements of if, for, while, ...), keep the braces on two separate lines. The reasons are: - Mostly the formatting of empty blocks does not matter much. - Empty compound statements are really rare and are usually just inserted while still working on the code. If they are on two lines, inserting code is easier. Also, overlooking the "{}" of an "if (...) {}" can be really bad. - Empty declarations are not uncommon, e.g. empty constructors. Putting them on one line saves vertical space at no loss of readability. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@183008 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index b6e8079b31..3ae279216f 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -1479,20 +1479,21 @@ private: AnnotatedLine &Line = *I; if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::r_brace, tok::kw_else, tok::kw_try, tok::kw_catch, - tok::kw_for, tok::kw_namespace, + tok::kw_for, // This gets rid of all ObjC @ keywords and methods. tok::at, tok::minus, tok::plus)) return; FormatToken *Tok = (I + 1)->First; - if (Tok->getNextNoneComment() == NULL && Tok->is(tok::r_brace) && - !Tok->MustBreakBefore) { + if (Tok->is(tok::r_brace) && !Tok->MustBreakBefore && + (Tok->getNextNoneComment() == NULL || + Tok->getNextNoneComment()->is(tok::semi))) { // We merge empty blocks even if the line exceeds the column limit. Tok->SpacesRequiredBefore = 0; Tok->CanBreakBefore = true; join(Line, *(I + 1)); I += 1; - } else if (Limit != 0) { + } else if (Limit != 0 && Line.First->isNot(tok::kw_namespace)) { // Check that we still have three lines and they fit into the limit. if (I + 2 == E || (I + 2)->Type == LT_Invalid || !nextTwoLinesFitInto(I, Limit)) diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index b09daa867b..8f6e9d6ad6 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -877,11 +877,11 @@ TEST_F(FormatTest, MultiLineCommentsInDefines) { } TEST_F(FormatTest, ParsesCommentsAdjacentToPPDirectives) { - EXPECT_EQ("namespace {\n}\n// Test\n#define A", + EXPECT_EQ("namespace {}\n// Test\n#define A", format("namespace {}\n // Test\n#define A")); - EXPECT_EQ("namespace {\n}\n/* Test */\n#define A", + EXPECT_EQ("namespace {}\n/* Test */\n#define A", format("namespace {}\n /* Test */\n#define A")); - EXPECT_EQ("namespace {\n}\n/* Test */ #define A", + EXPECT_EQ("namespace {}\n/* Test */ #define A", format("namespace {}\n /* Test */ #define A")); } @@ -1245,7 +1245,7 @@ TEST_F(FormatTest, IgnoresIf0Contents) { //===----------------------------------------------------------------------===// TEST_F(FormatTest, DoesNotBreakSemiAfterClassDecl) { - verifyFormat("class A {\n};"); + verifyFormat("class A {};"); } TEST_F(FormatTest, UnderstandsAccessSpecifiers) { @@ -1287,27 +1287,23 @@ TEST_F(FormatTest, SeparatesLogicalBlocks) { } TEST_F(FormatTest, FormatsClasses) { - verifyFormat("class A : public B {\n};"); - verifyFormat("class A : public ::B {\n};"); + verifyFormat("class A : public B {};"); + verifyFormat("class A : public ::B {};"); verifyFormat( "class AAAAAAAAAAAAAAAAAAAA : public BBBBBBBBBBBBBBBBBBBBBBBBBBBBBB,\n" - " public CCCCCCCCCCCCCCCCCCCCCCCCCCCCCC {\n" - "};\n"); + " public CCCCCCCCCCCCCCCCCCCCCCCCCCCCCC {};"); verifyFormat("class AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n" " : public BBBBBBBBBBBBBBBBBBBBBBBBBBBBBB,\n" - " public CCCCCCCCCCCCCCCCCCCCCCCCCCCCCC {\n" - "};\n"); + " public CCCCCCCCCCCCCCCCCCCCCCCCCCCCCC {};"); verifyFormat( - "class A : public B, public C, public D, public E, public F, public G {\n" - "};"); + "class A : public B, public C, public D, public E, public F {};"); verifyFormat("class AAAAAAAAAAAA : public B,\n" " public C,\n" " public D,\n" " public E,\n" " public F,\n" - " public G {\n" - "};"); + " public G {};"); verifyFormat("class\n" " ReallyReallyLongClassName {\n};", @@ -1329,10 +1325,8 @@ TEST_F(FormatTest, FormatsEnum) { " Four = (Zero && (One ^ Two)) | (One << Two),\n" " Five = (One, Two, Three, Four, 5)\n" "};"); - verifyFormat("enum Enum {\n" - "};"); - verifyFormat("enum {\n" - "};"); + verifyFormat("enum Enum {};"); + verifyFormat("enum {};"); verifyFormat("enum X E {\n} d;"); verifyFormat("enum __attribute__((...)) E {\n} d;"); verifyFormat("enum __declspec__((...)) E {\n} d;"); @@ -1348,28 +1342,27 @@ TEST_F(FormatTest, FormatsBitfields) { TEST_F(FormatTest, FormatsNamespaces) { verifyFormat("namespace some_namespace {\n" - "class A {\n};\n" + "class A {};\n" "void f() { f(); }\n" "}"); verifyFormat("namespace {\n" - "class A {\n};\n" + "class A {};\n" "void f() { f(); }\n" "}"); verifyFormat("inline namespace X {\n" - "class A {\n};\n" + "class A {};\n" "void f() { f(); }\n" "}"); verifyFormat("using namespace some_namespace;\n" - "class A {\n};\n" + "class A {};\n" "void f() { f(); }"); // This code is more common than we thought; if we // layout this correctly the semicolon will go into // its own line, which is undesireable. - verifyFormat("namespace {\n};"); + verifyFormat("namespace {};"); verifyFormat("namespace {\n" - "class A {\n" - "};\n" + "class A {};\n" "};"); } @@ -1730,8 +1723,7 @@ TEST_F(FormatTest, MacroCallsWithoutTrailingSemicolon) { EXPECT_EQ("INITIALIZE_PASS_BEGIN(ScopDetection, \"polly-detect\")\n" "INITIALIZE_AG_DEPENDENCY(AliasAnalysis)\n" "INITIALIZE_PASS_DEPENDENCY(DominatorTree)\n" - "class X {\n" - "};\n" + "class X {};\n" "INITIALIZE_PASS_END(ScopDetection, \"polly-detect\")\n" "int *createScopDetectionPass() { return 0; }", format(" INITIALIZE_PASS_BEGIN(ScopDetection, \"polly-detect\")\n" @@ -1930,11 +1922,7 @@ TEST_F(FormatTest, LayoutNestedBlocks) { TEST_F(FormatTest, PutEmptyBlocksIntoOneLine) { EXPECT_EQ("{}", format("{}")); - - // Negative test for enum. - verifyFormat("enum E {\n};"); - - // Note that when there's a missing ';', we still join... + verifyFormat("enum E {};"); verifyFormat("enum E {}"); } @@ -2821,13 +2809,13 @@ TEST_F(FormatTest, WrapsTemplateDeclarations) { verifyFormat("a(\n" " a(aaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa));"); - verifyFormat("template class C {\n};"); + verifyFormat("template class C {};"); verifyFormat("template void f();"); verifyFormat("template void f() {}"); FormatStyle AlwaysBreak = getLLVMStyle(); AlwaysBreak.AlwaysBreakTemplateDeclarations = true; - verifyFormat("template \nclass C {\n};", AlwaysBreak); + verifyFormat("template \nclass C {};", AlwaysBreak); verifyFormat("template \nvoid f();", AlwaysBreak); verifyFormat("template \nvoid f() {}", AlwaysBreak); } @@ -3547,7 +3535,7 @@ TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) { " : Implementation(new ImplicitCastMatcher(Other)) {}"); // FIXME: This is still incorrectly handled at the formatter side. - verifyFormat("template <> struct X < 15, i < 3 && 42 < 50 && 33<28> {\n};"); + verifyFormat("template <> struct X < 15, i < 3 && 42 < 50 && 33<28> {};"); // FIXME: // This now gets parsed incorrectly as class definition.