r183008 - Make formatting of empty blocks more consistent.
Daniel Jasper
djasper at google.com
Fri May 31 07:56:21 PDT 2013
Author: djasper
Date: Fri May 31 09:56:20 2013
New Revision: 183008
URL: http://llvm.org/viewvc/llvm-project?rev=183008&view=rev
Log:
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.
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=183008&r1=183007&r2=183008&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Fri May 31 09:56:20 2013
@@ -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))
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=183008&r1=183007&r2=183008&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri May 31 09:56:20 2013
@@ -877,11 +877,11 @@ TEST_F(FormatTest, MultiLineCommentsInDe
}
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, SeparatesLogicalBlock
}
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, MacroCallsWithoutTrai
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, WrapsTemplateDeclarat
verifyFormat("a<aaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaa>(\n"
" a(aaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaa));");
- verifyFormat("template <typename T> class C {\n};");
+ verifyFormat("template <typename T> class C {};");
verifyFormat("template <typename T> void f();");
verifyFormat("template <typename T> void f() {}");
FormatStyle AlwaysBreak = getLLVMStyle();
AlwaysBreak.AlwaysBreakTemplateDeclarations = true;
- verifyFormat("template <typename T>\nclass C {\n};", AlwaysBreak);
+ verifyFormat("template <typename T>\nclass C {};", AlwaysBreak);
verifyFormat("template <typename T>\nvoid f();", AlwaysBreak);
verifyFormat("template <typename T>\nvoid f() {}", AlwaysBreak);
}
@@ -3547,7 +3535,7 @@ TEST_F(FormatTest, UnderstandContextOfRe
" : Implementation(new ImplicitCastMatcher<F>(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.
More information about the cfe-commits
mailing list