From 251b3c9e7efe38e9f86d1a17279e03f57e963376 Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Mon, 1 Jul 2013 11:22:57 +0000 Subject: [PATCH] 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 llvm-svn: 185327 --- clang/lib/Format/WhitespaceManager.cpp | 24 ++++++++++++++------- clang/unittests/Format/FormatTest.cpp | 29 +++++++++++++++++++++----- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index e69e15cf8090..90f856bd117a 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -124,6 +124,9 @@ void WhitespaceManager::alignTrailingComments() { 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::alignTrailingComments() { 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; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 5bdc1b949d91..5227554d65c7 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -648,14 +648,17 @@ TEST_F(FormatTest, UnderstandsSingleLineComments) { 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;"); }