[clang] clang-format: Add "AllowShortNamespacesOnASingleLine" option (PR #105597)
Galen Elias via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 21 16:31:46 PDT 2024
https://github.com/galenelias updated https://github.com/llvm/llvm-project/pull/105597
>From 3819c15c8c6258d1d29a38fcfb71db8567fb34e5 Mon Sep 17 00:00:00 2001
From: Galen Elias <gelias at microsoft.com>
Date: Wed, 21 Aug 2024 15:46:35 -0700
Subject: [PATCH] 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 fea5d2e17a0e28..9d72b82549afc2 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 7fd42e46e0ccb7..1272aed7e9b042 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -941,6 +941,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",
@@ -1445,6 +1447,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 2a754a29e81e73..3343174d25446c 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27659,6 +27659,118 @@ TEST_F(FormatTest, SpaceBetweenKeywordAndLiteral) {
verifyFormat("return sizeof \"5\";");
}
+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
More information about the cfe-commits
mailing list