[PATCH] D30405: [clang-format] Add a new flag FixNamespaceEndComments to FormatStyle

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 90172.
krasimir marked 2 inline comments as done.
krasimir added a comment.

- Address review comments


https://reviews.llvm.org/D30405

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Index/CommentToXML.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -185,7 +185,7 @@
   EXPECT_EQ("namespace N {\n"
 "\n"
 "int i;\n"
-"}",
+"}  // namespace N",
 format("namespace N {\n"
"\n"
"inti;\n"
@@ -270,12 +270,22 @@
"}"));
 
   // FIXME: This is slightly inconsistent.
+  FormatStyle LLVMWithNoNamespaceFix = getLLVMStyle();
+  LLVMWithNoNamespaceFix.FixNamespaceComments = false;
   EXPECT_EQ("namespace {\n"
 "int i;\n"
 "}",
 format("namespace {\n"
"int i;\n"
"\n"
+   "}", LLVMWithNoNamespaceFix));
+  EXPECT_EQ("namespace {\n"
+"int i;\n"
+"\n"
+"} // namespace",
+format("namespace {\n"
+   "int i;\n"
+   "\n"
"}"));
   EXPECT_EQ("namespace {\n"
 "int i;\n"
@@ -1197,33 +1207,43 @@
 }
 
 TEST_F(FormatTest, FormatsNamespaces) {
+  FormatStyle LLVMWithNoNamespaceFix = getLLVMStyle();
+  LLVMWithNoNamespaceFix.FixNamespaceComments = false;
+
   verifyFormat("namespace some_namespace {\n"
"class A {};\n"
"void f() { f(); }\n"
-   "}");
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("namespace {\n"
"class A {};\n"
"void f() { f(); }\n"
-   "}");
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("inline namespace X {\n"
"class A {};\n"
"void f() { f(); }\n"
-   "}");
+   "}",
+   LLVMWithNoNamespaceFix);
   verifyFormat("using namespace some_namespace;\n"
"class A {};\n"
-   "void f() { f(); }");
+   "void f() { f(); }",
+   LLVMWithNoNamespaceFix);
 
   // This code is more common than we thought; if we
   // layout this correctly the semicolon will go into
   // its own line, which is undesirable.
-  verifyFormat("namespace {};");
+  verifyFormat("namespace {};",
+   LLVMWithNoNamespaceFix);
   verifyFormat("namespace {\n"
"class A {};\n"
-   "};");
+   "};",
+   LLVMWithNoNamespaceFix);
 
   verifyFormat("namespace {\n"
"int SomeVariable = 0; // comment\n"
-   "} // namespace");
+   "} // namespace",
+   LLVMWithNoNamespaceFix);
   EXPECT_EQ("#ifndef HEADER_GUARD\n"
 "#define HEADER_GUARD\n"
 "namespace my_namespace {\n"
@@ -1235,44 +1255,46 @@
"   namespace my_namespace {\n"
"int i;\n"
"}// my_namespace\n"
-   "#endif// HEADER_GUARD"));
+   "#endif// HEADER_GUARD",
+   LLVMWithNoNamespaceFix));
 
   EXPECT_EQ("namespace A::B {\n"
 "class C {};\n"
 "}",
 format("namespace A::B {\n"
"class C {};\n"
-   "}"));
+   "}",
+   LLVMWithNoNamespaceFix));
 
   FormatStyle Style = getLLVMStyle();
   Style.NamespaceIndentation = FormatStyle::NI_All;
   EXPECT_EQ("namespace out {\n"
 "  int i;\n"
 "  namespace in {\n"
 "int i;\n"
-"  } // namespace\n"
-"} // namespace",
+"  } // namespace in\n"
+"} // namespace out",
 format("namespace out {\n"
"int i;\n"
"namespace in {\n"
"int i;\n"
-   "} // namespace\n"
-   "} // namespace",
+   "} // namespace in\n"
+   "} // namespace out",
Style));
 
   Style.NamespaceIndentation = FormatStyle::NI_Inner;
   EXPECT_EQ("namespace out {\n"
 "int i;\n"
 "namespace in {\n"
 "  int i;\n"
-"} // namespace\n"
-"} // namespace",
+"} // namespace in\n"
+"} // namespace out",
 format("namespace out {\n"
"int i;\n"
"namespace in {\n"
"int i;\n"
-   "} // namespace\n"
-   "} // namespace",
+   "} // namespace in\n"
+   "} // namespace out",
  

[PATCH] D30405: [clang-format] Add a new flag FixNamespaceEndComments to FormatStyle

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.



Comment at: include/clang/Format/Format.h:354
+  /// fixes invalid existing ones.
+  bool FixNamespaceEndComments;
+

To be consistent with the clang-tidy check, just call this 
"FixNamespaceComments".

After a change like this, you need to call docs/tools/dump_format_style.py in 
order to update the documentation.



Comment at: lib/Format/Format.cpp:1837
 
+  if (Style.Language == FormatStyle::LK_Cpp &&
+  Style.FixNamespaceEndComments) {

So much code copied and pasted. Could you pull out a method or lambda and call 
this with either the JavaScriptRequoter or the NamespaceEndCommentsFixer?

Also, at some point, we'll probably want to call several of these fixers in 
series. But we can leave that problem for another day.


https://reviews.llvm.org/D30405



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits