kuzkry created this revision.
kuzkry added reviewers: krasimir, JakeMerdichAMD, jmerdich, rsmith.
kuzkry added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
kuzkry requested review of this revision.
clang-format documentation states that having enabled
FixNamespaceComments one may expect below code:
c++
namespace a {
foo();
}
to be turned into:
c++
namespace a {
foo();
} // namespace a
In reality, no "// namespace a" was added. The problem was too high
value of kShortNamespaceMaxLines, which is used while deciding whether
a namespace is long enough to be formatted.
As with 9163fe2, it remains to ensure clang-format is idempotent.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D87587
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/NamespaceEndCommentsFixer.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===================================================================
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -142,7 +142,7 @@
EXPECT_EQ("namespace A {\n"
"namespace B {\n"
"int i;\n"
- "}\n"
+ "}// namespace B\n"
"}// namespace A",
fixNamespaceEndComments("namespace A {\n"
"namespace B {\n"
@@ -423,8 +423,9 @@
TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddEndCommentForShortNamespace) {
EXPECT_EQ("namespace {}", fixNamespaceEndComments("namespace {}"));
EXPECT_EQ("namespace A {}", fixNamespaceEndComments("namespace A {}"));
- EXPECT_EQ("namespace A { a }", fixNamespaceEndComments("namespace A { a }"));
- EXPECT_EQ("namespace A { a };",
+ EXPECT_EQ("namespace A { a }// namespace A",
+ fixNamespaceEndComments("namespace A { a }"));
+ EXPECT_EQ("namespace A { a };// namespace A",
fixNamespaceEndComments("namespace A { a };"));
}
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -211,7 +211,7 @@
EXPECT_EQ("namespace N {\n"
"\n"
"int i;\n"
- "}",
+ "} // namespace N",
format("namespace N {\n"
"\n"
"int i;\n"
@@ -220,7 +220,7 @@
EXPECT_EQ("/* something */ namespace N {\n"
"\n"
"int i;\n"
- "}",
+ "} // namespace N",
format("/* something */ namespace N {\n"
"\n"
"int i;\n"
@@ -229,7 +229,7 @@
EXPECT_EQ("inline namespace N {\n"
"\n"
"int i;\n"
- "}",
+ "} // namespace N",
format("inline namespace N {\n"
"\n"
"int i;\n"
@@ -238,7 +238,7 @@
EXPECT_EQ("/* something */ inline namespace N {\n"
"\n"
"int i;\n"
- "}",
+ "} // namespace N",
format("/* something */ inline namespace N {\n"
"\n"
"int i;\n"
@@ -247,7 +247,7 @@
EXPECT_EQ("export namespace N {\n"
"\n"
"int i;\n"
- "}",
+ "} // namespace N",
format("export namespace N {\n"
"\n"
"int i;\n"
@@ -369,7 +369,7 @@
EXPECT_EQ("namespace {\n"
"int i;\n"
"\n"
- "}",
+ "} // namespace",
format("namespace {\n"
"int i;\n"
"\n"
@@ -8887,7 +8887,7 @@
format("void f ( ) { if ( a ) return }"));
EXPECT_EQ("namespace N {\n"
"void f()\n"
- "}",
+ "} // namespace N",
format("namespace N { void f() }"));
EXPECT_EQ("namespace N {\n"
"void f() {}\n"
@@ -9839,7 +9839,7 @@
verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
- "};",
+ "}; // namespace Foo",
Style);
}
@@ -9872,7 +9872,7 @@
Style);
verifyFormat("namespace Foo {\n"
"void Bar();\n"
- "};",
+ "}; // namespace Foo",
Style);
Style.BreakBeforeBraces = FormatStyle::BS_Custom;
@@ -9913,7 +9913,7 @@
verifyFormat("namespace Foo\n"
"{\n"
"void Bar();\n"
- "};",
+ "}; // namespace Foo",
Style);
}
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===================================================================
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -24,7 +24,7 @@
namespace {
// The maximal number of unwrapped lines that a short namespace spans.
// Short namespaces don't need an end comment.
-static const int kShortNamespaceMaxLines = 1;
+static const int kShortNamespaceMaxLines = 0;
// Computes the name of a namespace given the namespace token.
// Returns "" for anonymous namespace.
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1380,6 +1380,9 @@
/// If ``true``, clang-format adds missing namespace end comments and
/// fixes invalid existing ones.
+ ///
+ /// Short namespaces will not be given an end comment, unless the comment
+ /// is invalid in which case it is always fixed.
/// \code
/// true: false:
/// namespace a { vs. namespace a {
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1611,6 +1611,9 @@
If ``true``, clang-format adds missing namespace end comments and
fixes invalid existing ones.
+ Short namespaces will not be given an end comment, unless the comment
+ is invalid in which case it is always fixed.
+
.. code-block:: c++
true: false:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
