kuzkry updated this revision to Diff 322880.
kuzkry added a comment.
Sorted code alphabetically as suggested by @HazardyKnusperkeks and applied
almost all fixes proposed by @curdeius (entry in release notes + some changes
in unit tests)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87587/new/
https://reviews.llvm.org/D87587
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/NamespaceEndCommentsFixer.cpp
clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===================================================================
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -816,7 +816,7 @@
"}\n"));
}
-TEST_F(NamespaceEndCommentsFixerTest, AddEndCommentForNamespacesAroundMacros) {
+TEST_F(NamespaceEndCommentsFixerTest, AddsEndCommentForNamespacesAroundMacros) {
// Conditional blocks around are fine
EXPECT_EQ("namespace A {\n"
"#if 1\n"
@@ -1116,6 +1116,107 @@
"}\n"
"}\n"));
}
+
+using ShortNamespaceLinesTest = NamespaceEndCommentsFixerTest;
+
+TEST_F(ShortNamespaceLinesTest,
+ ZeroUnwrappedLines_DoesNotAddEndCommentForOneLinerNamespace) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 0u;
+
+ EXPECT_EQ("namespace OneLinerNamespace {}\n",
+ fixNamespaceEndComments("namespace OneLinerNamespace {}\n", Style));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+ ZeroUnwrappedLines_DoesNotAddEndCommentForShortNamespace) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 0u;
+
+ EXPECT_EQ("namespace ShortNamespace {\n"
+ "}\n",
+ fixNamespaceEndComments("namespace ShortNamespace {\n"
+ "}\n",
+ Style));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+ ZeroUnwrappedLines_AddsEndCommentForLongNamespace) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 0u;
+
+ EXPECT_EQ("namespace LongNamespace {\n"
+ "int i;\n"
+ "}// namespace LongNamespace\n",
+ fixNamespaceEndComments("namespace LongNamespace {\n"
+ "int i;\n"
+ "}\n",
+ Style));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+ OneUnwrappedLine_DoesNotAddEndCommentForShortNamespace) {
+ constexpr auto DefaultUnwrappedLines = 1u;
+ auto const Style = getLLVMStyle();
+
+ EXPECT_EQ(DefaultUnwrappedLines, Style.ShortNamespaceLines);
+ EXPECT_EQ("namespace ShortNamespace {\n"
+ "int i;\n"
+ "}\n",
+ fixNamespaceEndComments("namespace ShortNamespace {\n"
+ "int i;\n"
+ "}\n"));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+ OneUnwrappedLine_AddsEndCommentForLongNamespace) {
+ constexpr auto DefaultUnwrappedLines = 1u;
+ auto const Style = getLLVMStyle();
+
+ EXPECT_EQ(DefaultUnwrappedLines, Style.ShortNamespaceLines);
+ EXPECT_EQ("namespace LongNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "}// namespace LongNamespace\n",
+ fixNamespaceEndComments("namespace LongNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "}\n"));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+ MultipleUnwrappedLine_DoesNotAddEndCommentForShortNamespace) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 2u;
+
+ EXPECT_EQ("namespace ShortNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "}\n",
+ fixNamespaceEndComments("namespace ShortNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "}\n",
+ Style));
+}
+
+TEST_F(ShortNamespaceLinesTest,
+ MultipleUnwrappedLine_AddsEndCommentForLongNamespace) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 2u;
+
+ EXPECT_EQ("namespace LongNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "int k;\n"
+ "}// namespace LongNamespace\n",
+ fixNamespaceEndComments("namespace LongNamespace {\n"
+ "int i;\n"
+ "int j;\n"
+ "int k;\n"
+ "}\n",
+ Style));
+}
} // end namespace
} // end namespace format
} // end namespace clang
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===================================================================
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -22,10 +22,6 @@
namespace format {
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;
-
// Computes the name of a namespace given the namespace token.
// Returns "" for anonymous namespace.
std::string computeName(const FormatToken *NamespaceTok) {
@@ -283,7 +279,7 @@
computeEndCommentText(NamespaceName, AddNewline, NamespaceTok,
Style.SpacesInLineCommentPrefix.Minimum);
if (!hasEndComment(EndCommentPrevTok)) {
- bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1;
+ bool isShort = I - StartLineIndex <= Style.ShortNamespaceLines + 1;
if (!isShort)
addEndComment(EndCommentPrevTok, EndCommentText, SourceMgr, &Fixes);
} else if (!validEndComment(EndCommentPrevTok, NamespaceName,
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -641,6 +641,7 @@
IO.mapOptional("PointerAlignment", Style.PointerAlignment);
IO.mapOptional("RawStringFormats", Style.RawStringFormats);
IO.mapOptional("ReflowComments", Style.ReflowComments);
+ IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
IO.mapOptional("SortIncludes", Style.SortIncludes);
IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
@@ -1003,6 +1004,7 @@
LLVMStyle.ObjCSpaceAfterProperty = false;
LLVMStyle.ObjCSpaceBeforeProtocolList = true;
LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
+ LLVMStyle.ShortNamespaceLines = 1;
LLVMStyle.SpacesBeforeTrailingComments = 1;
LLVMStyle.Standard = FormatStyle::LS_Latest;
LLVMStyle.UseCRLF = false;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1959,12 +1959,14 @@
/// not use this in config files, etc. Use at your own risk.
bool ExperimentalAutoDetectBinPacking;
- /// If ``true``, clang-format adds missing namespace end comments and
- /// fixes invalid existing ones.
+ /// If ``true``, clang-format adds missing namespace end comments for
+ /// short namespaces and fixes invalid existing ones. Short ones are
+ /// controlled by "ShortNamespaceLines".
/// \code
/// true: false:
/// namespace a { vs. namespace a {
/// foo(); foo();
+ /// bar(); bar();
/// } // namespace a }
/// \endcode
bool FixNamespaceComments;
@@ -2613,6 +2615,27 @@
bool ReflowComments;
// clang-format on
+ /// The maximal number of unwrapped lines that a short namespace spans.
+ /// Defaults to 1.
+ ///
+ /// This determines the maximum length of short namespaces by counting
+ /// unwrapped lines (i.e. containing neither opening nor closing
+ /// namespace brace) and makes "FixNamespaceComments" omit adding
+ /// end comments for those.
+ /// \code
+ /// ShortNamespaceLines: 1 vs. ShortNamespaceLines: 0
+ /// namespace a { namespace a {
+ /// int foo; int foo;
+ /// } } // namespace a
+ ///
+ /// ShortNamespaceLines: 1 vs. ShortNamespaceLines: 0
+ /// namespace b { namespace b {
+ /// int foo; int foo;
+ /// int bar; int bar;
+ /// } // namespace b } // namespace b
+ /// \endcode
+ unsigned ShortNamespaceLines;
+
/// Include sorting options.
enum SortIncludesOptions : unsigned char {
/// Includes are never sorted.
@@ -3192,6 +3215,7 @@
R.PenaltyBreakTemplateDeclaration &&
PointerAlignment == R.PointerAlignment &&
RawStringFormats == R.RawStringFormats &&
+ ShortNamespaceLines == R.ShortNamespaceLines &&
SortIncludes == R.SortIncludes &&
SortJavaStaticImport == R.SortJavaStaticImport &&
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -189,6 +189,9 @@
#include "B/A.h"
#include "B/a.h"
+- Option ``ShortNamespaceLines`` has been added to give better control
+ over ``FixNamespaceComments`` when determining a namespace length.
+
libclang
--------
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2201,14 +2201,16 @@
not use this in config files, etc. Use at your own risk.
**FixNamespaceComments** (``bool``)
- If ``true``, clang-format adds missing namespace end comments and
- fixes invalid existing ones.
+ If ``true``, clang-format adds missing namespace end comments for
+ short namespaces and fixes invalid existing ones. Short ones are
+ controlled by "ShortNamespaceLines".
.. code-block:: c++
true: false:
namespace a { vs. namespace a {
foo(); foo();
+ bar(); bar();
} // namespace a }
**ForEachMacros** (``std::vector<std::string>``)
@@ -3004,6 +3006,28 @@
/* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
* information */
+**ShortNamespaceLines** (``unsigned``)
+ The maximal number of unwrapped lines that a short namespace spans.
+ Defaults to 1.
+
+ This determines the maximum length of short namespaces by counting
+ unwrapped lines (i.e. containing neither opening nor closing
+ namespace brace) and makes "FixNamespaceComments" omit adding
+ end comments for those.
+
+ .. code-block:: c++
+
+ ShortNamespaceLines: 1 vs. ShortNamespaceLines: 0
+ namespace a { namespace a {
+ int foo; int foo;
+ } } // namespace a
+
+ ShortNamespaceLines: 1 vs. ShortNamespaceLines: 0
+ namespace b { namespace b {
+ int foo; int foo;
+ int bar; int bar;
+ } // namespace b } // namespace b
+
**SortIncludes** (``SortIncludesOptions``)
Controls if and how clang-format will sort ``#includes``.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
