r185327 - Don't align "} // namespace" comments.

[Daniel Jasper]

Author: djasper
Date: Mon Jul  1 06:22:57 2013
New Revision: 185327

URL: http://llvm.org/viewvc/llvm-project?rev=185327&view=rev
Log:
Don't align "} // namespace" comments.

This is not all bad, but people are often surprised by it.

Before:
namespace {
int SomeVariable = 0; // comment
}                     // namespace

After:
namespace {
int SomeVariable = 0; // comment
} // namespace

Modified:
    cfe/trunk/lib/Format/WhitespaceManager.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=185327&r1=185326&r2=185327&view=diff
==============================================================================
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Mon Jul  1 06:22:57 2013
@@ -124,6 +124,9 @@ void WhitespaceManager::alignTrailingCom
     unsigned ChangeMaxColumn = Style.ColumnLimit - Changes[i].TokenLength;
     Newlines += Changes[i].NewlinesBefore;
     if (Changes[i].IsTrailingComment) {
+      bool FollowsRBraceInColumn0 = i > 0 && Changes[i].NewlinesBefore == 0 &&
+                                    Changes[i - 1].Kind == tok::r_brace &&
+                                    Changes[i - 1].StartOfTokenColumn == 0;
       bool WasAlignedWithStartOfNextLine =
           // A comment on its own line.
           Changes[i].NewlinesBefore == 1 &&
@@ -137,13 +140,20 @@ void WhitespaceManager::alignTrailingCom
                Changes[i + 1].OriginalWhitespaceRange.getEnd())) &&
           // Which is not a comment itself.
           Changes[i + 1].Kind != tok::comment;
-      if (BreakBeforeNext || Newlines > 1 ||
-          (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
-          // Break the comment sequence if the previous line did not end
-          // in a trailing comment.
-          (Changes[i].NewlinesBefore == 1 && i > 0 &&
-           !Changes[i - 1].IsTrailingComment) ||
-          WasAlignedWithStartOfNextLine) {
+      if (FollowsRBraceInColumn0) {
+        // If this comment follows an } in column 0, it probably documents the
+        // closing of a namespace and we don't want to align it.
+        alignTrailingComments(StartOfSequence, i, MinColumn);
+        MinColumn = ChangeMinColumn;
+        MaxColumn = ChangeMinColumn;
+        StartOfSequence = i;
+      } else if (BreakBeforeNext || Newlines > 1 ||
+                 (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
+                 // Break the comment sequence if the previous line did not end
+                 // in a trailing comment.
+                 (Changes[i].NewlinesBefore == 1 && i > 0 &&
+                  !Changes[i - 1].IsTrailingComment) ||
+                 WasAlignedWithStartOfNextLine) {
         alignTrailingComments(StartOfSequence, i, MinColumn);
         MinColumn = ChangeMinColumn;
         MaxColumn = ChangeMaxColumn;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=185327&r1=185326&r2=185327&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jul  1 06:22:57 2013
@@ -648,14 +648,17 @@ TEST_F(FormatTest, UnderstandsSingleLine
             format("void f()    {     // This does something ..\n"
                    "  }\n"
                    "int   a;     // This is unrelated"));
-  EXPECT_EQ("void f() { // This does something ..\n"
-            "}          // awesome..\n"
+  EXPECT_EQ("class C {\n"
+            "  void f() { // This does something ..\n"
+            "  }          // awesome..\n"
             "\n"
-            "int a; // This is unrelated",
-            format("void f()    { // This does something ..\n"
+            "  int a; // This is unrelated\n"
+            "};",
+            format("class C{void f()    { // This does something ..\n"
                    "      } // awesome..\n"
                    " \n"
-                   "int a;    // This is unrelated"));
+                   "int a;    // This is unrelated\n"
+                   "};"));
 
   EXPECT_EQ("int i; // single line trailing comment",
             format("int i;\\\n// single line trailing comment"));
@@ -1479,6 +1482,22 @@ TEST_F(FormatTest, FormatsNamespaces) {
   verifyFormat("namespace {\n"
                "class A {};\n"
                "};");
+
+  verifyFormat("namespace {\n"
+               "int SomeVariable = 0; // comment\n"
+               "} // namespace");
+  EXPECT_EQ("#ifndef HEADER_GUARD\n"
+            "#define HEADER_GUARD\n"
+            "namespace my_namespace {\n"
+            "int i;\n"
+            "} // my_namespace\n"
+            "#endif // HEADER_GUARD",
+            format("#ifndef HEADER_GUARD\n"
+                   " #define HEADER_GUARD\n"
+                   "   namespace my_namespace {\n"
+                   "int i;\n"
+                   "}    // my_namespace\n"
+                   "#endif    // HEADER_GUARD"));
 }
 
 TEST_F(FormatTest, FormatsExternC) { verifyFormat("extern \"C\" {\nint a;"); }