r181887 - Don't put short namespace on a single line.

Daniel Jasper djasper at google.com
Wed May 15 07:09:55 PDT 2013


Author: djasper
Date: Wed May 15 09:09:55 2013
New Revision: 181887

URL: http://llvm.org/viewvc/llvm-project?rev=181887&view=rev
Log:
Don't put short namespace on a single line.

Before:
namespace abc { class SomeClass; }
namespace def { void someFunction() {} }

After:
namespace abc {
class Def;
}
namespace def {
void someFunction() {}
}

Rationale:
a) Having anything other than forward declaration on the same line
   as a namespace looks confusing.
b) Formatting namespace-forward-declaration-combinations different
   from other stuff is inconsistent.
c) Wasting vertical space close to such forward declarations really
   does not affect 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=181887&r1=181886&r2=181887&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed May 15 09:09:55 2013
@@ -1348,8 +1348,7 @@ private:
     if (I + 1 == E || (I + 1)->Type == LT_Invalid)
       return;
 
-    if (I->Last->is(tok::l_brace) &&
-        Style.BreakBeforeBraces == FormatStyle::BS_Attach) {
+    if (I->Last->is(tok::l_brace)) {
       tryMergeSimpleBlock(I, E, Limit);
     } else if (I->First.is(tok::kw_if)) {
       tryMergeSimpleIf(I, E, Limit);
@@ -1402,13 +1401,17 @@ private:
   void tryMergeSimpleBlock(std::vector<AnnotatedLine>::iterator &I,
                            std::vector<AnnotatedLine>::iterator E,
                            unsigned Limit) {
+    // No merging if the brace already is on the next line.
+    if (Style.BreakBeforeBraces != FormatStyle::BS_Attach)
+      return;
+
     // First, check that the current line allows merging. This is the case if
     // we're not in a control flow statement and the last token is an opening
     // brace.
     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_for, tok::kw_namespace,
                            // This gets rid of all ObjC @ keywords and methods.
                            tok::at, tok::minus, tok::plus))
       return;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=181887&r1=181886&r2=181887&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed May 15 09:09:55 2013
@@ -745,11 +745,11 @@ TEST_F(FormatTest, SplitsLongCxxComments
 }
 
 TEST_F(FormatTest, ParsesCommentsAdjacentToPPDirectives) {
-  EXPECT_EQ("namespace {}\n// Test\n#define A",
+  EXPECT_EQ("namespace {\n}\n// Test\n#define A",
             format("namespace {}\n   // Test\n#define A"));
-  EXPECT_EQ("namespace {}\n/* Test */\n#define A",
+  EXPECT_EQ("namespace {\n}\n/* Test */\n#define A",
             format("namespace {}\n   /* Test */\n#define A"));
-  EXPECT_EQ("namespace {}\n/* Test */ #define A",
+  EXPECT_EQ("namespace {\n}\n/* Test */ #define A",
             format("namespace {}\n   /* Test */    #define A"));
 }
 
@@ -2921,7 +2921,10 @@ TEST_F(FormatTest, IncorrectCodeMissingS
             "    return\n"
             "}",
             format("void  f  (  )  {  if  ( a )  return  }"));
-  EXPECT_EQ("namespace N { void f() }", format("namespace  N  {  void f()  }"));
+  EXPECT_EQ("namespace N {\n"
+            "void f()\n"
+            "}",
+            format("namespace  N  {  void f()  }"));
   EXPECT_EQ("namespace N {\n"
             "void f() {}\n"
             "void g()\n"





More information about the cfe-commits mailing list