[clang] clang-format: Add "AllowShortNamespacesOnASingleLine" option (PR #105597)
Galen Elias via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 8 15:20:52 PST 2024
https://github.com/galenelias updated https://github.com/llvm/llvm-project/pull/105597
>From 4118b7dde9adbee7b6aaf5d094d34cb6b64f6c77 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Wed, 21 Aug 2024 16:33:42 -0700
Subject: [PATCH 01/12] clang-format: Add "AllowShortNamespacesOnASingleLine"
option
This addresses: https://github.com/llvm/llvm-project/issues/101363
which is a resurrection of a previously opened but never completed
review: https://reviews.llvm.org/D11851
The feature is to allow code like the following not to be broken across
multiple lines:
```
namespace foo { class bar; }
namespace foo { namespace bar { class baz; } }
```
Code like this is commonly used for forward declarations, which are
ideally kept compact. This is also apparently the format that
include-what-you-use will insert for forward declarations.
---
clang/include/clang/Format/Format.h | 5 +
clang/lib/Format/Format.cpp | 3 +
clang/lib/Format/UnwrappedLineFormatter.cpp | 82 ++++++++++++++
clang/unittests/Format/FormatTest.cpp | 112 ++++++++++++++++++++
4 files changed, 202 insertions(+)
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 2af1d4065c3cc1..c96fc4d1d9557b 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -972,6 +972,11 @@ struct FormatStyle {
/// \version 3.7
bool AllowShortLoopsOnASingleLine;
+ /// If ``true``, ``namespace a { class b; }`` can be put on a single a single
+ /// line.
+ /// \version 19
+ bool AllowShortNamespacesOnASingleLine;
+
/// Different ways to break after the function definition return type.
/// This option is **deprecated** and is retained for backwards compatibility.
enum DefinitionReturnTypeBreakingStyle : int8_t {
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 97fac41cdd3008..495b727e609df6 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -952,6 +952,8 @@ template <> struct MappingTraits<FormatStyle> {
Style.AllowShortLambdasOnASingleLine);
IO.mapOptional("AllowShortLoopsOnASingleLine",
Style.AllowShortLoopsOnASingleLine);
+ IO.mapOptional("AllowShortNamespacesOnASingleLine",
+ Style.AllowShortNamespacesOnASingleLine);
IO.mapOptional("AlwaysBreakAfterDefinitionReturnType",
Style.AlwaysBreakAfterDefinitionReturnType);
IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
@@ -1457,6 +1459,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
LLVMStyle.AllowShortLoopsOnASingleLine = false;
+ LLVMStyle.AllowShortNamespacesOnASingleLine = false;
LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
LLVMStyle.AttributeMacros.push_back("__capability");
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 1804c1437fd41d..971eac1978bb71 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -420,6 +420,15 @@ class LineJoiner {
TheLine->First != LastNonComment) {
return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
}
+
+ if (TheLine->Last->is(tok::l_brace)) {
+ if (Style.AllowShortNamespacesOnASingleLine &&
+ TheLine->First->is(tok::kw_namespace)) {
+ if (unsigned result = tryMergeNamespace(I, E, Limit))
+ return result;
+ }
+ }
+
// Try to merge a control statement block with left brace unwrapped.
if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
@@ -616,6 +625,62 @@ class LineJoiner {
return 1;
}
+ unsigned tryMergeNamespace(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
+ SmallVectorImpl<AnnotatedLine *>::const_iterator E,
+ unsigned Limit) {
+ if (Limit == 0)
+ return 0;
+ if (I[1]->InPPDirective != (*I)->InPPDirective ||
+ (I[1]->InPPDirective && I[1]->First->HasUnescapedNewline)) {
+ return 0;
+ }
+ if (I + 2 == E || I[2]->Type == LT_Invalid)
+ return 0;
+
+ Limit = limitConsideringMacros(I + 1, E, Limit);
+
+ if (!nextTwoLinesFitInto(I, Limit))
+ return 0;
+
+ // Check if it's a namespace inside a namespace, and call recursively if so
+ // '3' is the sizes of the whitespace and closing brace for " _inner_ }"
+ if (I[1]->First->is(tok::kw_namespace)) {
+ if (I[1]->Last->is(TT_LineComment))
+ return 0;
+
+ unsigned inner_limit = Limit - I[1]->Last->TotalLength - 3;
+ unsigned inner_result = tryMergeNamespace(I + 1, E, inner_limit);
+ if (!inner_result)
+ return 0;
+ // check if there is even a line after the inner result
+ if (I + 2 + inner_result >= E)
+ return 0;
+ // check that the line after the inner result starts with a closing brace
+ // which we are permitted to merge into one line
+ if (I[2 + inner_result]->First->is(tok::r_brace) &&
+ !I[2 + inner_result]->First->MustBreakBefore &&
+ !I[1 + inner_result]->Last->is(TT_LineComment) &&
+ nextNLinesFitInto(I, I + 2 + inner_result + 1, Limit)) {
+ return 2 + inner_result;
+ }
+ return 0;
+ }
+
+ // There's no inner namespace, so we are considering to merge at most one
+ // line.
+
+ // The line which is in the namespace should end with semicolon
+ if (I[1]->Last->isNot(tok::semi))
+ return 0;
+
+ // Last, check that the third line starts with a closing brace.
+ if (I[2]->First->isNot(tok::r_brace) || I[2]->First->MustBreakBefore)
+ return 0;
+
+ // If so, merge all three lines.
+ return 2;
+ }
+
unsigned tryMergeSimpleControlStatement(
SmallVectorImpl<AnnotatedLine *>::const_iterator I,
SmallVectorImpl<AnnotatedLine *>::const_iterator E, unsigned Limit) {
@@ -916,6 +981,23 @@ class LineJoiner {
return 1 + I[1]->Last->TotalLength + 1 + I[2]->Last->TotalLength <= Limit;
}
+ bool nextNLinesFitInto(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
+ SmallVectorImpl<AnnotatedLine *>::const_iterator E,
+ unsigned Limit) {
+ unsigned joinedLength = 0;
+ for (SmallVectorImpl<AnnotatedLine *>::const_iterator J = I + 1; J != E;
+ ++J) {
+
+ if ((*J)->First->MustBreakBefore)
+ return false;
+
+ joinedLength += 1 + (*J)->Last->TotalLength;
+ if (joinedLength > Limit)
+ return false;
+ }
+ return true;
+ }
+
bool containsMustBreak(const AnnotatedLine *Line) {
assert(Line->First);
// Ignore the first token, because in this situation, it applies more to the
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 794ccab3704534..468cb6b98dd745 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27981,6 +27981,118 @@ TEST_F(FormatTest, BreakBinaryOperations) {
Style);
}
+TEST_F(FormatTest, ShortNamespacesOption) {
+ FormatStyle style = getLLVMStyle();
+ style.AllowShortNamespacesOnASingleLine = true;
+ style.FixNamespaceComments = false;
+
+ // Basic functionality
+ verifyFormat("namespace foo { class bar; }", style);
+ verifyFormat("namespace foo::bar { class baz; }", style);
+ verifyFormat("namespace { class bar; }", style);
+ verifyFormat("namespace foo {\n"
+ "class bar;\n"
+ "class baz;\n"
+ "}",
+ style);
+
+ // Trailing comments prevent merging
+ verifyFormat("namespace foo {\n"
+ "namespace baz { class qux; } // comment\n"
+ "}",
+ style);
+
+ // Make sure code doesn't walk to far on unbalanced code
+ verifyFormat("namespace foo {", style);
+ verifyFormat("namespace foo {\n"
+ "class baz;",
+ style);
+ verifyFormat("namespace foo {\n"
+ "namespace bar { class baz; }",
+ style);
+
+ // Nested namespaces
+ verifyFormat("namespace foo { namespace bar { class baz; } }", style);
+ verifyFormat("namespace foo {\n"
+ "namespace bar { class baz; }\n"
+ "namespace quar { class quaz; }\n"
+ "}",
+ style);
+
+ // Varying inner content
+ verifyFormat("namespace foo {\n"
+ "int f() { return 5; }\n"
+ "}",
+ style);
+ verifyFormat("namespace foo { template <T> struct bar; }", style);
+ verifyFormat("namespace foo { constexpr int num = 42; }", style);
+
+ // Validate wrapping scenarios around the ColumnLimit
+ style.ColumnLimit = 64;
+
+ // Validate just under the ColumnLimit
+ verifyFormat(
+ "namespace foo { namespace bar { namespace baz { class qux; } } }",
+ style);
+
+ // Validate just over the ColumnLimit
+ verifyFormat("namespace foo {\n"
+ "namespace bar { namespace baz { class quux; } }\n"
+ "}",
+ style);
+
+ verifyFormat("namespace foo {\n"
+ "namespace bar {\n"
+ "namespace baz { namespace qux { class quux; } }\n"
+ "}\n"
+ "}",
+ style);
+
+ // Validate that the ColumnLimit logic accounts for trailing content as well
+ verifyFormat("namespace foo {\n"
+ "namespace bar { namespace baz { class qux; } }\n"
+ "} // extra",
+ style);
+
+ // No ColumnLimit, allows long nested one-liners, but also leaves multi-line
+ // instances alone
+ style.ColumnLimit = 0;
+ verifyFormat("namespace foo { namespace bar { namespace baz { namespace qux "
+ "{ class quux; } } } }",
+ style);
+
+ verifyNoChange("namespace foo {\n"
+ "namespace bar {\n"
+ "namespace baz { namespace qux { class quux; } }\n"
+ "}\n"
+ "}",
+ style);
+
+ // This option doesn't really work with FixNamespaceComments and nested
+ // namespaces. Code should use the concatenated namespace syntax. e.g.
+ // 'namespace foo::bar'
+ style.FixNamespaceComments = true;
+
+ verifyFormat(
+ "namespace foo {\n"
+ "namespace bar { namespace baz { class qux; } } // namespace bar\n"
+ "} // namespace foo",
+ "namespace foo { namespace bar { namespace baz { class qux; } } }",
+ style);
+
+ // This option doesn't really make any sense with ShortNamespaceLines = 0
+ style.ShortNamespaceLines = 0;
+
+ verifyFormat(
+ "namespace foo {\n"
+ "namespace bar {\n"
+ "namespace baz { class qux; } // namespace baz\n"
+ "} // namespace bar\n"
+ "} // namespace foo",
+ "namespace foo { namespace bar { namespace baz { class qux; } } }",
+ style);
+}
+
} // namespace
} // namespace test
} // namespace format
>From 86534c3ba6a82e2782a6bf96350e1e71fa633f63 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Thu, 22 Aug 2024 09:16:32 -0700
Subject: [PATCH 02/12] Use PascalCase for local variable names
---
clang/lib/Format/UnwrappedLineFormatter.cpp | 18 +++----
clang/unittests/Format/FormatTest.cpp | 56 ++++++++++-----------
2 files changed, 37 insertions(+), 37 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 971eac1978bb71..ddb94c62d0da2c 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -648,20 +648,20 @@ class LineJoiner {
if (I[1]->Last->is(TT_LineComment))
return 0;
- unsigned inner_limit = Limit - I[1]->Last->TotalLength - 3;
- unsigned inner_result = tryMergeNamespace(I + 1, E, inner_limit);
- if (!inner_result)
+ const unsigned InnerLimit = Limit - I[1]->Last->TotalLength - 3;
+ const unsigned MergedLines = tryMergeNamespace(I + 1, E, InnerLimit);
+ if (!MergedLines)
return 0;
// check if there is even a line after the inner result
- if (I + 2 + inner_result >= E)
+ if (I + 2 + MergedLines >= E)
return 0;
// check that the line after the inner result starts with a closing brace
// which we are permitted to merge into one line
- if (I[2 + inner_result]->First->is(tok::r_brace) &&
- !I[2 + inner_result]->First->MustBreakBefore &&
- !I[1 + inner_result]->Last->is(TT_LineComment) &&
- nextNLinesFitInto(I, I + 2 + inner_result + 1, Limit)) {
- return 2 + inner_result;
+ if (I[2 + MergedLines]->First->is(tok::r_brace) &&
+ !I[2 + MergedLines]->First->MustBreakBefore &&
+ !I[1 + MergedLines]->Last->is(TT_LineComment) &&
+ nextNLinesFitInto(I, I + 2 + MergedLines + 1, Limit)) {
+ return 2 + MergedLines;
}
return 0;
}
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 468cb6b98dd745..0fb1ed2d47e31d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27982,106 +27982,106 @@ TEST_F(FormatTest, BreakBinaryOperations) {
}
TEST_F(FormatTest, ShortNamespacesOption) {
- FormatStyle style = getLLVMStyle();
- style.AllowShortNamespacesOnASingleLine = true;
- style.FixNamespaceComments = false;
+ FormatStyle Style = getLLVMStyle();
+ Style.AllowShortNamespacesOnASingleLine = true;
+ Style.FixNamespaceComments = false;
// Basic functionality
- verifyFormat("namespace foo { class bar; }", style);
- verifyFormat("namespace foo::bar { class baz; }", style);
- verifyFormat("namespace { class bar; }", style);
+ verifyFormat("namespace foo { class bar; }", Style);
+ verifyFormat("namespace foo::bar { class baz; }", Style);
+ verifyFormat("namespace { class bar; }", Style);
verifyFormat("namespace foo {\n"
"class bar;\n"
"class baz;\n"
"}",
- style);
+ Style);
// Trailing comments prevent merging
verifyFormat("namespace foo {\n"
"namespace baz { class qux; } // comment\n"
"}",
- style);
+ Style);
// Make sure code doesn't walk to far on unbalanced code
- verifyFormat("namespace foo {", style);
+ verifyFormat("namespace foo {", Style);
verifyFormat("namespace foo {\n"
"class baz;",
- style);
+ Style);
verifyFormat("namespace foo {\n"
"namespace bar { class baz; }",
- style);
+ Style);
// Nested namespaces
- verifyFormat("namespace foo { namespace bar { class baz; } }", style);
+ verifyFormat("namespace foo { namespace bar { class baz; } }", Style);
verifyFormat("namespace foo {\n"
"namespace bar { class baz; }\n"
"namespace quar { class quaz; }\n"
"}",
- style);
+ Style);
// Varying inner content
verifyFormat("namespace foo {\n"
"int f() { return 5; }\n"
"}",
- style);
- verifyFormat("namespace foo { template <T> struct bar; }", style);
- verifyFormat("namespace foo { constexpr int num = 42; }", style);
+ Style);
+ verifyFormat("namespace foo { template <T> struct bar; }", Style);
+ verifyFormat("namespace foo { constexpr int num = 42; }", Style);
// Validate wrapping scenarios around the ColumnLimit
- style.ColumnLimit = 64;
+ Style.ColumnLimit = 64;
// Validate just under the ColumnLimit
verifyFormat(
"namespace foo { namespace bar { namespace baz { class qux; } } }",
- style);
+ Style);
// Validate just over the ColumnLimit
verifyFormat("namespace foo {\n"
"namespace bar { namespace baz { class quux; } }\n"
"}",
- style);
+ Style);
verifyFormat("namespace foo {\n"
"namespace bar {\n"
"namespace baz { namespace qux { class quux; } }\n"
"}\n"
"}",
- style);
+ Style);
// Validate that the ColumnLimit logic accounts for trailing content as well
verifyFormat("namespace foo {\n"
"namespace bar { namespace baz { class qux; } }\n"
"} // extra",
- style);
+ Style);
// No ColumnLimit, allows long nested one-liners, but also leaves multi-line
// instances alone
- style.ColumnLimit = 0;
+ Style.ColumnLimit = 0;
verifyFormat("namespace foo { namespace bar { namespace baz { namespace qux "
"{ class quux; } } } }",
- style);
+ Style);
verifyNoChange("namespace foo {\n"
"namespace bar {\n"
"namespace baz { namespace qux { class quux; } }\n"
"}\n"
"}",
- style);
+ Style);
// This option doesn't really work with FixNamespaceComments and nested
// namespaces. Code should use the concatenated namespace syntax. e.g.
// 'namespace foo::bar'
- style.FixNamespaceComments = true;
+ Style.FixNamespaceComments = true;
verifyFormat(
"namespace foo {\n"
"namespace bar { namespace baz { class qux; } } // namespace bar\n"
"} // namespace foo",
"namespace foo { namespace bar { namespace baz { class qux; } } }",
- style);
+ Style);
// This option doesn't really make any sense with ShortNamespaceLines = 0
- style.ShortNamespaceLines = 0;
+ Style.ShortNamespaceLines = 0;
verifyFormat(
"namespace foo {\n"
@@ -28090,7 +28090,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
"} // namespace bar\n"
"} // namespace foo",
"namespace foo { namespace bar { namespace baz { class qux; } } }",
- style);
+ Style);
}
} // namespace
>From 2159b8c222b9e9c8e2fdf60211a171e0a58b58b1 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Wed, 28 Aug 2024 10:42:05 -0700
Subject: [PATCH 03/12] Address review comments
---
clang/lib/Format/UnwrappedLineFormatter.cpp | 22 +++++++++----------
clang/unittests/Format/FormatTest.cpp | 24 ++++++++++-----------
2 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index ddb94c62d0da2c..31bf3f484c4c39 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -643,7 +643,7 @@ class LineJoiner {
return 0;
// Check if it's a namespace inside a namespace, and call recursively if so
- // '3' is the sizes of the whitespace and closing brace for " _inner_ }"
+ // '3' is the sizes of the whitespace and closing brace for " _inner_ }".
if (I[1]->First->is(tok::kw_namespace)) {
if (I[1]->Last->is(TT_LineComment))
return 0;
@@ -652,11 +652,11 @@ class LineJoiner {
const unsigned MergedLines = tryMergeNamespace(I + 1, E, InnerLimit);
if (!MergedLines)
return 0;
- // check if there is even a line after the inner result
- if (I + 2 + MergedLines >= E)
+ // Check if there is even a line after the inner result.
+ if (std::distance(I, E) <= MergedLines + 2)
return 0;
- // check that the line after the inner result starts with a closing brace
- // which we are permitted to merge into one line
+ // Check that the line after the inner result starts with a closing brace
+ // which we are permitted to merge into one line.
if (I[2 + MergedLines]->First->is(tok::r_brace) &&
!I[2 + MergedLines]->First->MustBreakBefore &&
!I[1 + MergedLines]->Last->is(TT_LineComment) &&
@@ -669,7 +669,7 @@ class LineJoiner {
// There's no inner namespace, so we are considering to merge at most one
// line.
- // The line which is in the namespace should end with semicolon
+ // The line which is in the namespace should end with semicolon.
if (I[1]->Last->isNot(tok::semi))
return 0;
@@ -984,15 +984,13 @@ class LineJoiner {
bool nextNLinesFitInto(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
SmallVectorImpl<AnnotatedLine *>::const_iterator E,
unsigned Limit) {
- unsigned joinedLength = 0;
- for (SmallVectorImpl<AnnotatedLine *>::const_iterator J = I + 1; J != E;
- ++J) {
-
+ unsigned JoinedLength = 0;
+ for (auto J = I + 1; J != E; ++J) {
if ((*J)->First->MustBreakBefore)
return false;
- joinedLength += 1 + (*J)->Last->TotalLength;
- if (joinedLength > Limit)
+ JoinedLength += 1 + (*J)->Last->TotalLength;
+ if (JoinedLength > Limit)
return false;
}
return true;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 0fb1ed2d47e31d..e03e0a871c8d19 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27986,7 +27986,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
Style.AllowShortNamespacesOnASingleLine = true;
Style.FixNamespaceComments = false;
- // Basic functionality
+ // Basic functionality.
verifyFormat("namespace foo { class bar; }", Style);
verifyFormat("namespace foo::bar { class baz; }", Style);
verifyFormat("namespace { class bar; }", Style);
@@ -27996,13 +27996,13 @@ TEST_F(FormatTest, ShortNamespacesOption) {
"}",
Style);
- // Trailing comments prevent merging
+ // Trailing comments prevent merging.
verifyFormat("namespace foo {\n"
"namespace baz { class qux; } // comment\n"
"}",
Style);
- // Make sure code doesn't walk to far on unbalanced code
+ // Make sure code doesn't walk to far on unbalanced code.
verifyFormat("namespace foo {", Style);
verifyFormat("namespace foo {\n"
"class baz;",
@@ -28011,7 +28011,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
"namespace bar { class baz; }",
Style);
- // Nested namespaces
+ // Nested namespaces.
verifyFormat("namespace foo { namespace bar { class baz; } }", Style);
verifyFormat("namespace foo {\n"
"namespace bar { class baz; }\n"
@@ -28019,7 +28019,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
"}",
Style);
- // Varying inner content
+ // Varying inner content.
verifyFormat("namespace foo {\n"
"int f() { return 5; }\n"
"}",
@@ -28027,15 +28027,15 @@ TEST_F(FormatTest, ShortNamespacesOption) {
verifyFormat("namespace foo { template <T> struct bar; }", Style);
verifyFormat("namespace foo { constexpr int num = 42; }", Style);
- // Validate wrapping scenarios around the ColumnLimit
+ // Validate wrapping scenarios around the ColumnLimit.
Style.ColumnLimit = 64;
- // Validate just under the ColumnLimit
+ // Validate just under the ColumnLimit.
verifyFormat(
"namespace foo { namespace bar { namespace baz { class qux; } } }",
Style);
- // Validate just over the ColumnLimit
+ // Validate just over the ColumnLimit.
verifyFormat("namespace foo {\n"
"namespace bar { namespace baz { class quux; } }\n"
"}",
@@ -28048,14 +28048,14 @@ TEST_F(FormatTest, ShortNamespacesOption) {
"}",
Style);
- // Validate that the ColumnLimit logic accounts for trailing content as well
+ // Validate that the ColumnLimit logic accounts for trailing content as well.
verifyFormat("namespace foo {\n"
"namespace bar { namespace baz { class qux; } }\n"
"} // extra",
Style);
// No ColumnLimit, allows long nested one-liners, but also leaves multi-line
- // instances alone
+ // instances alone.
Style.ColumnLimit = 0;
verifyFormat("namespace foo { namespace bar { namespace baz { namespace qux "
"{ class quux; } } } }",
@@ -28070,7 +28070,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
// This option doesn't really work with FixNamespaceComments and nested
// namespaces. Code should use the concatenated namespace syntax. e.g.
- // 'namespace foo::bar'
+ // 'namespace foo::bar'.
Style.FixNamespaceComments = true;
verifyFormat(
@@ -28080,7 +28080,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
"namespace foo { namespace bar { namespace baz { class qux; } } }",
Style);
- // This option doesn't really make any sense with ShortNamespaceLines = 0
+ // This option doesn't really make any sense with ShortNamespaceLines = 0.
Style.ShortNamespaceLines = 0;
verifyFormat(
>From 07ec70d700bd061c5ccb7803d22e96a887734895 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Thu, 5 Sep 2024 07:41:49 -0700
Subject: [PATCH 04/12] Switch version of new format option to 20
---
clang/include/clang/Format/Format.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index c96fc4d1d9557b..e1d10e64c92fe0 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -974,7 +974,7 @@ struct FormatStyle {
/// If ``true``, ``namespace a { class b; }`` can be put on a single a single
/// line.
- /// \version 19
+ /// \version 20
bool AllowShortNamespacesOnASingleLine;
/// Different ways to break after the function definition return type.
>From b5f727b25352823e40d1c07723e5f0312c4c53f9 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Tue, 10 Sep 2024 08:30:37 -0700
Subject: [PATCH 05/12] Re-generate StylClangFormatStyleOptions.rst
---
clang/docs/ClangFormatStyleOptions.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index c79a635d86a6ef..53c3d26fe1d96a 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1976,6 +1976,12 @@ the configuration (without a prefix: ``Auto``).
If ``true``, ``while (true) continue;`` can be put on a single
line.
+.. _AllowShortNamespacesOnASingleLine:
+
+**AllowShortNamespacesOnASingleLine** (``Boolean``) :versionbadge:`clang-format 20` :ref:`¶ <AllowShortNamespacesOnASingleLine>`
+ If ``true``, ``namespace a { class b; }`` can be put on a single a single
+ line.
+
.. _AlwaysBreakAfterDefinitionReturnType:
**AlwaysBreakAfterDefinitionReturnType** (``DefinitionReturnTypeBreakingStyle``) :versionbadge:`clang-format 3.7` :ref:`¶ <AlwaysBreakAfterDefinitionReturnType>`
>From 986d2d54cacf9fed5b2706ea180dfbdc34153a35 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Tue, 10 Sep 2024 08:31:06 -0700
Subject: [PATCH 06/12] Add ConfigParseTest
---
clang/unittests/Format/ConfigParseTest.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 2ee0df99353ff5..37edaa5fd9c128 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -159,6 +159,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) {
CHECK_PARSE_BOOL(AllowShortCompoundRequirementOnASingleLine);
CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
+ CHECK_PARSE_BOOL(AllowShortNamespacesOnASingleLine);
CHECK_PARSE_BOOL(BinPackArguments);
CHECK_PARSE_BOOL(BinPackParameters);
CHECK_PARSE_BOOL(BreakAdjacentStringLiterals);
>From 40a85d1d0730e684cabefffb4a4356ea104ccfda Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Tue, 10 Sep 2024 08:31:21 -0700
Subject: [PATCH 07/12] Fix interaction with CompactNamespaces
---
clang/lib/Format/UnwrappedLineFormatter.cpp | 17 +++++-----
clang/unittests/Format/FormatTest.cpp | 36 ++++++++++++++-------
2 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 31bf3f484c4c39..756278b4e8ddf4 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -361,9 +361,18 @@ class LineJoiner {
const auto *FirstNonComment = TheLine->getFirstNonComment();
if (!FirstNonComment)
return 0;
+
// FIXME: There are probably cases where we should use FirstNonComment
// instead of TheLine->First.
+ if (TheLine->Last->is(tok::l_brace)) {
+ if (Style.AllowShortNamespacesOnASingleLine &&
+ TheLine->First->is(tok::kw_namespace)) {
+ if (unsigned result = tryMergeNamespace(I, E, Limit))
+ return result;
+ }
+ }
+
if (Style.CompactNamespaces) {
if (const auto *NSToken = TheLine->First->getNamespaceToken()) {
int J = 1;
@@ -421,14 +430,6 @@ class LineJoiner {
return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
}
- if (TheLine->Last->is(tok::l_brace)) {
- if (Style.AllowShortNamespacesOnASingleLine &&
- TheLine->First->is(tok::kw_namespace)) {
- if (unsigned result = tryMergeNamespace(I, E, Limit))
- return result;
- }
- }
-
// Try to merge a control statement block with left brace unwrapped.
if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index e03e0a871c8d19..0dd38b8b3d4692 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27982,9 +27982,11 @@ TEST_F(FormatTest, BreakBinaryOperations) {
}
TEST_F(FormatTest, ShortNamespacesOption) {
- FormatStyle Style = getLLVMStyle();
- Style.AllowShortNamespacesOnASingleLine = true;
- Style.FixNamespaceComments = false;
+ auto BaseStyle = getLLVMStyle();
+ BaseStyle.AllowShortNamespacesOnASingleLine = true;
+ BaseStyle.FixNamespaceComments = false;
+
+ auto Style = BaseStyle;
// Basic functionality.
verifyFormat("namespace foo { class bar; }", Style);
@@ -28002,7 +28004,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
"}",
Style);
- // Make sure code doesn't walk to far on unbalanced code.
+ // Make sure code doesn't walk too far on unbalanced code.
verifyFormat("namespace foo {", Style);
verifyFormat("namespace foo {\n"
"class baz;",
@@ -28015,7 +28017,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
verifyFormat("namespace foo { namespace bar { class baz; } }", Style);
verifyFormat("namespace foo {\n"
"namespace bar { class baz; }\n"
- "namespace quar { class quaz; }\n"
+ "namespace qux { class quux; }\n"
"}",
Style);
@@ -28057,20 +28059,32 @@ TEST_F(FormatTest, ShortNamespacesOption) {
// No ColumnLimit, allows long nested one-liners, but also leaves multi-line
// instances alone.
Style.ColumnLimit = 0;
- verifyFormat("namespace foo { namespace bar { namespace baz { namespace qux "
- "{ class quux; } } } }",
- Style);
+ verifyFormat(
+ "namespace foo { namespace bar { namespace baz { class qux; } } }",
+ Style);
verifyNoChange("namespace foo {\n"
- "namespace bar {\n"
- "namespace baz { namespace qux { class quux; } }\n"
- "}\n"
+ "namespace bar { namespace baz { class qux; } }\n"
"}",
Style);
+ Style = BaseStyle;
+ Style.CompactNamespaces = true;
+ verifyFormat("namespace foo { namespace bar { class baz; } }", Style);
+
+ // If we can't merge an outer nested namespaces, but can merge an inner
+ // nested namespace, then CompactNamespaces will merge the outer namespace
+ // first, preventing the merging of the inner namespace
+ verifyFormat("namespace foo { namespace baz {\n"
+ "class qux;\n"
+ "} // comment\n"
+ "}",
+ Style);
+
// This option doesn't really work with FixNamespaceComments and nested
// namespaces. Code should use the concatenated namespace syntax. e.g.
// 'namespace foo::bar'.
+ Style = BaseStyle;
Style.FixNamespaceComments = true;
verifyFormat(
>From ad1fdbb0131b10acf213b7fe49b0421856eadbc8 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Wed, 25 Sep 2024 16:20:46 -0700
Subject: [PATCH 08/12] Address additional PR comments
---
clang/include/clang/Format/Format.h | 3 +--
clang/lib/Format/UnwrappedLineFormatter.cpp | 20 +++++++++++---------
clang/unittests/Format/FormatTest.cpp | 2 +-
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index e1d10e64c92fe0..f09cfc90f7a4c1 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -972,8 +972,7 @@ struct FormatStyle {
/// \version 3.7
bool AllowShortLoopsOnASingleLine;
- /// If ``true``, ``namespace a { class b; }`` can be put on a single a single
- /// line.
+ /// If ``true``, ``namespace a { class b; }`` can be put on a single line.
/// \version 20
bool AllowShortNamespacesOnASingleLine;
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 756278b4e8ddf4..6b57f8005688e9 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -368,7 +368,8 @@ class LineJoiner {
if (TheLine->Last->is(tok::l_brace)) {
if (Style.AllowShortNamespacesOnASingleLine &&
TheLine->First->is(tok::kw_namespace)) {
- if (unsigned result = tryMergeNamespace(I, E, Limit))
+ unsigned result = tryMergeNamespace(I, E, Limit);
+ if (result > 0)
return result;
}
}
@@ -643,26 +644,27 @@ class LineJoiner {
if (!nextTwoLinesFitInto(I, Limit))
return 0;
- // Check if it's a namespace inside a namespace, and call recursively if so
+ // Check if it's a namespace inside a namespace, and call recursively if so.
// '3' is the sizes of the whitespace and closing brace for " _inner_ }".
if (I[1]->First->is(tok::kw_namespace)) {
- if (I[1]->Last->is(TT_LineComment))
+ if (I[1]->Last->is(tok::comment))
return 0;
const unsigned InnerLimit = Limit - I[1]->Last->TotalLength - 3;
const unsigned MergedLines = tryMergeNamespace(I + 1, E, InnerLimit);
if (!MergedLines)
return 0;
+ const auto N = MergedLines + 2;
// Check if there is even a line after the inner result.
- if (std::distance(I, E) <= MergedLines + 2)
+ if (std::distance(I, E) <= N)
return 0;
// Check that the line after the inner result starts with a closing brace
// which we are permitted to merge into one line.
- if (I[2 + MergedLines]->First->is(tok::r_brace) &&
- !I[2 + MergedLines]->First->MustBreakBefore &&
- !I[1 + MergedLines]->Last->is(TT_LineComment) &&
- nextNLinesFitInto(I, I + 2 + MergedLines + 1, Limit)) {
- return 2 + MergedLines;
+ if (I[N]->First->is(tok::r_brace) &&
+ !I[N]->First->MustBreakBefore &&
+ !I[MergedLines + 1]->Last->is(tok::comment) &&
+ nextNLinesFitInto(I, I + N + 1, Limit)) {
+ return N;
}
return 0;
}
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 0dd38b8b3d4692..434f0a0dfb6079 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -28074,7 +28074,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
// If we can't merge an outer nested namespaces, but can merge an inner
// nested namespace, then CompactNamespaces will merge the outer namespace
- // first, preventing the merging of the inner namespace
+ // first, preventing the merging of the inner namespace.
verifyFormat("namespace foo { namespace baz {\n"
"class qux;\n"
"} // comment\n"
>From b8a21f7843f496e697abc69f8e3cfae90a995d2d Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Thu, 26 Sep 2024 08:15:02 -0700
Subject: [PATCH 09/12] clang-format run
---
clang/lib/Format/UnwrappedLineFormatter.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 6b57f8005688e9..300bd770f993da 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -660,8 +660,7 @@ class LineJoiner {
return 0;
// Check that the line after the inner result starts with a closing brace
// which we are permitted to merge into one line.
- if (I[N]->First->is(tok::r_brace) &&
- !I[N]->First->MustBreakBefore &&
+ if (I[N]->First->is(tok::r_brace) && !I[N]->First->MustBreakBefore &&
!I[MergedLines + 1]->Last->is(tok::comment) &&
nextNLinesFitInto(I, I + N + 1, Limit)) {
return N;
>From e735f46f15ce62d7c63a9ad287025409208b8483 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Tue, 15 Oct 2024 14:59:50 -0700
Subject: [PATCH 10/12] Re-work interaction with 'CompactNamespaces' option
- Disable recursive merging of nested namespaces onto a single line
unless 'CompactNamespaces' is enabled.
- Fix off-by-one error in CompactNamespaces code. For nested namespaces
with 3 or more namespaces, it was incorrectly compacting lines which
were 1 or two spaces over the ColumnLimit, leading to incorrect
formatting results.
- Add release notes mention.
- Re-generate Style Options documentation
---
clang/docs/ClangFormatStyleOptions.rst | 3 +-
clang/docs/ReleaseNotes.rst | 2 +
clang/lib/Format/UnwrappedLineFormatter.cpp | 7 +-
clang/unittests/Format/FormatTest.cpp | 98 +++++++++++----------
4 files changed, 60 insertions(+), 50 deletions(-)
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 53c3d26fe1d96a..317fc99b498f8f 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1979,8 +1979,7 @@ the configuration (without a prefix: ``Auto``).
.. _AllowShortNamespacesOnASingleLine:
**AllowShortNamespacesOnASingleLine** (``Boolean``) :versionbadge:`clang-format 20` :ref:`¶ <AllowShortNamespacesOnASingleLine>`
- If ``true``, ``namespace a { class b; }`` can be put on a single a single
- line.
+ If ``true``, ``namespace a { class b; }`` can be put on a single line.
.. _AlwaysBreakAfterDefinitionReturnType:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5c156a9c073a9c..f139a196cf4941 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -410,6 +410,8 @@ clang-format
- Adds ``BreakBinaryOperations`` option.
+- Adds ``AllowShortNamespacesOnASingleLine`` option.
+
libclang
--------
- Add ``clang_isBeforeInTranslationUnit``. Given two source locations, it determines
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 300bd770f993da..d9fa2780034b45 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -383,7 +383,7 @@ class LineJoiner {
ClosingLineIndex == I[J]->MatchingClosingBlockLineIndex &&
I[J]->Last->TotalLength < Limit;
++J, --ClosingLineIndex) {
- Limit -= I[J]->Last->TotalLength;
+ Limit -= I[J]->Last->TotalLength + 1;
// Reduce indent level for bodies of namespaces which were compacted,
// but only if their content was indented in the first place.
@@ -647,9 +647,10 @@ class LineJoiner {
// Check if it's a namespace inside a namespace, and call recursively if so.
// '3' is the sizes of the whitespace and closing brace for " _inner_ }".
if (I[1]->First->is(tok::kw_namespace)) {
- if (I[1]->Last->is(tok::comment))
+ if (I[1]->Last->is(tok::comment) || !Style.CompactNamespaces)
return 0;
+ assert(Limit >= I[1]->Last->TotalLength + 3);
const unsigned InnerLimit = Limit - I[1]->Last->TotalLength - 3;
const unsigned MergedLines = tryMergeNamespace(I + 1, E, InnerLimit);
if (!MergedLines)
@@ -661,7 +662,7 @@ class LineJoiner {
// Check that the line after the inner result starts with a closing brace
// which we are permitted to merge into one line.
if (I[N]->First->is(tok::r_brace) && !I[N]->First->MustBreakBefore &&
- !I[MergedLines + 1]->Last->is(tok::comment) &&
+ I[MergedLines + 1]->Last->isNot(tok::comment) &&
nextNLinesFitInto(I, I + N + 1, Limit)) {
return N;
}
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 434f0a0dfb6079..14955342de8270 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -4484,7 +4484,7 @@ TEST_F(FormatTest, FormatsCompactNamespaces) {
"} // namespace A",
Style);
- Style.ColumnLimit = 40;
+ Style.ColumnLimit = 41;
verifyFormat("namespace aaaaaaaaaa {\n"
"namespace bbbbbbbbbb {\n"
"}} // namespace aaaaaaaaaa::bbbbbbbbbb",
@@ -4504,6 +4504,16 @@ TEST_F(FormatTest, FormatsCompactNamespaces) {
"} // namespace bbbbbb\n"
"} // namespace aaaaaa",
Style);
+
+ verifyFormat("namespace a { namespace b { namespace c {\n"
+ "}}} // namespace a::b::c",
+ Style);
+
+ verifyFormat("namespace a { namespace b {\n"
+ "namespace cc {\n"
+ "}}} // namespace a::b::cc",
+ Style);
+
Style.ColumnLimit = 80;
// Extra semicolon after 'inner' closing brace prevents merging
@@ -27985,6 +27995,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
auto BaseStyle = getLLVMStyle();
BaseStyle.AllowShortNamespacesOnASingleLine = true;
BaseStyle.FixNamespaceComments = false;
+ BaseStyle.CompactNamespaces = true;
auto Style = BaseStyle;
@@ -27999,8 +28010,9 @@ TEST_F(FormatTest, ShortNamespacesOption) {
Style);
// Trailing comments prevent merging.
- verifyFormat("namespace foo {\n"
- "namespace baz { class qux; } // comment\n"
+ verifyFormat("namespace foo { namespace baz {\n"
+ "class qux;\n"
+ "} // comment\n"
"}",
Style);
@@ -28015,12 +28027,23 @@ TEST_F(FormatTest, ShortNamespacesOption) {
// Nested namespaces.
verifyFormat("namespace foo { namespace bar { class baz; } }", Style);
+
+ // Without CompactNamespaces, we won't merge consecutive namespace
+ // declarations
+ Style.CompactNamespaces = false;
+ verifyFormat("namespace foo {\n"
+ "namespace bar { class baz; }\n"
+ "}",
+ Style);
+
verifyFormat("namespace foo {\n"
"namespace bar { class baz; }\n"
"namespace qux { class quux; }\n"
"}",
Style);
+ Style = BaseStyle;
+
// Varying inner content.
verifyFormat("namespace foo {\n"
"int f() { return 5; }\n"
@@ -28029,7 +28052,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
verifyFormat("namespace foo { template <T> struct bar; }", Style);
verifyFormat("namespace foo { constexpr int num = 42; }", Style);
- // Validate wrapping scenarios around the ColumnLimit.
+ // Validate nested namespace wrapping scenarios around the ColumnLimit.
Style.ColumnLimit = 64;
// Validate just under the ColumnLimit.
@@ -28038,22 +28061,24 @@ TEST_F(FormatTest, ShortNamespacesOption) {
Style);
// Validate just over the ColumnLimit.
- verifyFormat("namespace foo {\n"
- "namespace bar { namespace baz { class quux; } }\n"
- "}",
+ verifyFormat("namespace foo { namespace baar { namespace baaz {\n"
+ "class quux;\n"
+ "}}}",
Style);
- verifyFormat("namespace foo {\n"
- "namespace bar {\n"
- "namespace baz { namespace qux { class quux; } }\n"
- "}\n"
- "}",
- Style);
+ verifyFormat(
+ "namespace foo { namespace bar { namespace baz { namespace qux {\n"
+ "class quux;\n"
+ "}}}}",
+ Style);
// Validate that the ColumnLimit logic accounts for trailing content as well.
- verifyFormat("namespace foo {\n"
- "namespace bar { namespace baz { class qux; } }\n"
- "} // extra",
+ verifyFormat("namespace foo { namespace bar { class qux; } } // extra",
+ Style);
+
+ verifyFormat("namespace foo { namespace bar { namespace baz {\n"
+ "class qux;\n"
+ "}}} // extra",
Style);
// No ColumnLimit, allows long nested one-liners, but also leaves multi-line
@@ -28068,41 +28093,24 @@ TEST_F(FormatTest, ShortNamespacesOption) {
"}",
Style);
- Style = BaseStyle;
- Style.CompactNamespaces = true;
verifyFormat("namespace foo { namespace bar { class baz; } }", Style);
- // If we can't merge an outer nested namespaces, but can merge an inner
- // nested namespace, then CompactNamespaces will merge the outer namespace
- // first, preventing the merging of the inner namespace.
- verifyFormat("namespace foo { namespace baz {\n"
- "class qux;\n"
- "} // comment\n"
- "}",
- Style);
-
- // This option doesn't really work with FixNamespaceComments and nested
- // namespaces. Code should use the concatenated namespace syntax. e.g.
- // 'namespace foo::bar'.
+ // FIXME: Ideally AllowShortNamespacesOnASingleLine would disable the trailing
+ // namespace comment from 'FixNamespaceComments', as it's not really necessary
+ // in this scenario, but the two options work at very different layers of the
+ // formatter, so I'm not sure how to make them interact.
+ //
+ // As it stands, the trailing comment will be added and likely make the line
+ // too long to fit within the ColumnLimit, reducing the how likely the line
+ // will still fit on a single line. The recommendation for now is to use the
+ // concatenated namespace syntax instead. e.g. 'namespace foo::bar'
Style = BaseStyle;
Style.FixNamespaceComments = true;
verifyFormat(
- "namespace foo {\n"
- "namespace bar { namespace baz { class qux; } } // namespace bar\n"
- "} // namespace foo",
- "namespace foo { namespace bar { namespace baz { class qux; } } }",
- Style);
-
- // This option doesn't really make any sense with ShortNamespaceLines = 0.
- Style.ShortNamespaceLines = 0;
-
- verifyFormat(
- "namespace foo {\n"
- "namespace bar {\n"
- "namespace baz { class qux; } // namespace baz\n"
- "} // namespace bar\n"
- "} // namespace foo",
+ "namespace foo { namespace bar { namespace baz {\n"
+ "class qux;\n"
+ "}}} // namespace foo::bar::baz",
"namespace foo { namespace bar { namespace baz { class qux; } } }",
Style);
}
>From dd3cecb8381a9c8bbda0e319b0d46d541d2761a8 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Tue, 22 Oct 2024 10:59:24 -0700
Subject: [PATCH 11/12] Remove ColumnLimit=0 tests
---
clang/unittests/Format/FormatTest.cpp | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 14955342de8270..1221dabb9316c8 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -28081,20 +28081,6 @@ TEST_F(FormatTest, ShortNamespacesOption) {
"}}} // extra",
Style);
- // No ColumnLimit, allows long nested one-liners, but also leaves multi-line
- // instances alone.
- Style.ColumnLimit = 0;
- verifyFormat(
- "namespace foo { namespace bar { namespace baz { class qux; } } }",
- Style);
-
- verifyNoChange("namespace foo {\n"
- "namespace bar { namespace baz { class qux; } }\n"
- "}",
- Style);
-
- verifyFormat("namespace foo { namespace bar { class baz; } }", Style);
-
// FIXME: Ideally AllowShortNamespacesOnASingleLine would disable the trailing
// namespace comment from 'FixNamespaceComments', as it's not really necessary
// in this scenario, but the two options work at very different layers of the
>From 1733719d95d86edb01676fd6ee95abc1e6a94264 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Fri, 8 Nov 2024 15:14:18 -0800
Subject: [PATCH 12/12] Remove blank line in release notes
---
clang/docs/ReleaseNotes.rst | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e6381fa41c5ba1..5eeb92ff0dc291 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -841,7 +841,6 @@ clang-format
``Never``, and ``true`` to ``Always``.
- Adds ``RemoveEmptyLinesInUnwrappedLines`` option.
- Adds ``KeepFormFeed`` option and set it to ``true`` for ``GNU`` style.
-
- Adds ``AllowShortNamespacesOnASingleLine`` option.
libclang
More information about the cfe-commits
mailing list