kuzkry updated this revision to Diff 312940.
kuzkry retitled this revision from "[clang-format][PR47290] Make one-line
namespaces resistant to FixNamespaceComments, update documentation" to
"[clang-format][PR47290] Add MaxUnwrappedLinesForShortNamespace format option".
kuzkry edited the summary of this revision.
kuzkry added a comment.
As proposed by @MyDeveloperDay, I added MaxUnwrappedLinesForShortNamespace
option to control behaviour of FixNamespaceComments. This time I'm providing
you with a full diff.
Btw. sorry for taking so long.
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/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,92 @@
"}\n"
"}\n"));
}
+
+using MaxUnwrappedLinesForShortNamespaceTest = NamespaceEndCommentsFixerTest;
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest, DefaultsToOneUnwrappedLine) {
+ constexpr auto UnwrappedLines = 1u;
+ auto const Style = getLLVMStyle();
+
+ EXPECT_EQ(UnwrappedLines, Style.MaxUnwrappedLinesForShortNamespace);
+}
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest,
+ ZeroUnwrappedLines_DoesNotAddEndCommentForOneLinerNamespace) {
+ auto Style = getLLVMStyle();
+ Style.MaxUnwrappedLinesForShortNamespace = 0;
+
+ EXPECT_EQ("namespace OneLinerNamespace {}\n",
+ fixNamespaceEndComments("namespace OneLinerNamespace {}\n", Style));
+}
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest,
+ ZeroUnwrappedLines_DoesNotAddEndCommentForNonOneLinerNamespace) {
+ auto Style = getLLVMStyle();
+ Style.MaxUnwrappedLinesForShortNamespace = 0;
+
+ EXPECT_EQ("namespace NonOneLinerNamespace {\n"
+ "}\n",
+ fixNamespaceEndComments("namespace NonOneLinerNamespace {\n"
+ "}\n",
+ Style));
+}
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest,
+ OneUnwrappedLine_DoesNotAddEndCommentForShortNamespace) {
+ EXPECT_EQ("namespace ShortNamespace {\n"
+ "int i;\n"
+ "}\n",
+ fixNamespaceEndComments("namespace ShortNamespace {\n"
+ "int i;\n"
+ "}\n"));
+}
+
+TEST_F(MaxUnwrappedLinesForShortNamespaceTest,
+ OneUnwrappedLine_AddsEndCommentForLongNamespace) {
+ 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(MaxUnwrappedLinesForShortNamespaceTest,
+ MultipleUnwrappedLine_DoesNotAddEndCommentForShortNamespace) {
+ auto Style = getLLVMStyle();
+ Style.MaxUnwrappedLinesForShortNamespace = 2;
+
+ 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(MaxUnwrappedLinesForShortNamespaceTest,
+ MultipleUnwrappedLine_AddsEndCommentForLongNamespace) {
+ auto Style = getLLVMStyle();
+ Style.MaxUnwrappedLinesForShortNamespace = 2;
+
+ 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) {
@@ -280,7 +276,8 @@
const std::string EndCommentText =
computeEndCommentText(NamespaceName, AddNewline, NamespaceTok);
if (!hasEndComment(EndCommentPrevTok)) {
- bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1;
+ bool isShort =
+ I - StartLineIndex <= Style.MaxUnwrappedLinesForShortNamespace + 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
@@ -573,6 +573,8 @@
IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
+ IO.mapOptional("MaxUnwrappedLinesForShortNamespace",
+ Style.MaxUnwrappedLinesForShortNamespace);
IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation);
IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
@@ -935,6 +937,7 @@
LLVMStyle.JavaScriptWrapImports = true;
LLVMStyle.TabWidth = 8;
LLVMStyle.MaxEmptyLinesToKeep = 1;
+ LLVMStyle.MaxUnwrappedLinesForShortNamespace = 1;
LLVMStyle.KeepEmptyLinesAtTheStartOfBlocks = true;
LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1653,12 +1653,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 "MaxUnwrappedLinesForShortNamespace".
/// \code
/// true: false:
/// namespace a { vs. namespace a {
/// foo(); foo();
+ /// bar(); bar();
/// } // namespace a }
/// \endcode
bool FixNamespaceComments;
@@ -2094,6 +2096,27 @@
/// \endcode
unsigned MaxEmptyLinesToKeep;
+ /// 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
+ /// MaxUnwrappedLinesForShortNamespace: 1 vs. MaxUnwrappedLinesForShortNamespace: 0
+ /// namespace a { namespace a {
+ /// int foo; int foo;
+ /// } } // namespace a
+ ///
+ /// MaxUnwrappedLinesForShortNamespace: 1 vs. MaxUnwrappedLinesForShortNamespace: 0
+ /// namespace b { namespace b {
+ /// int foo; int foo;
+ /// int bar; int bar;
+ /// } // namespace b } // namespace b
+ /// \endcode
+ unsigned MaxUnwrappedLinesForShortNamespace;
+
/// Different ways to indent namespace contents.
enum NamespaceIndentationKind {
/// Don't indent in namespaces.
@@ -2795,6 +2818,8 @@
MacroBlockBegin == R.MacroBlockBegin &&
MacroBlockEnd == R.MacroBlockEnd &&
MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&
+ MaxUnwrappedLinesForShortNamespace ==
+ R.MaxUnwrappedLinesForShortNamespace &&
NamespaceIndentation == R.NamespaceIndentation &&
NamespaceMacros == R.NamespaceMacros &&
ObjCBinPackProtocolList == R.ObjCBinPackProtocolList &&
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1886,14 +1886,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 "MaxUnwrappedLinesForShortNamespace".
.. code-block:: c++
true: false:
namespace a { vs. namespace a {
foo(); foo();
+ bar(); bar();
} // namespace a }
**ForEachMacros** (``std::vector<std::string>``)
@@ -2458,6 +2460,28 @@
return i;
}
+**MaxUnwrappedLinesForShortNamespace** (``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++
+
+ MaxUnwrappedLinesForShortNamespace: 1 vs. MaxUnwrappedLinesForShortNamespace: 0
+ namespace a { namespace a {
+ int foo; int foo;
+ } } // namespace a
+
+ MaxUnwrappedLinesForShortNamespace: 1 vs. MaxUnwrappedLinesForShortNamespace: 0
+ namespace b { namespace b {
+ int foo; int foo;
+ int bar; int bar;
+ } // namespace b } // namespace b
+
**NamespaceIndentation** (``NamespaceIndentationKind``)
The indentation used for namespaces.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
