Skip to content

[clang-format] Add AllowShortNamespacesOnASingleLine option #105597

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang/docs/ClangFormatStyleOptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2088,6 +2088,11 @@ 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 line.

.. _AlwaysBreakAfterDefinitionReturnType:

**AlwaysBreakAfterDefinitionReturnType** (``DefinitionReturnTypeBreakingStyle``) :versionbadge:`clang-format 3.7` :ref:`¶ <AlwaysBreakAfterDefinitionReturnType>`
Expand Down
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,7 @@ 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
--------
Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Format/Format.h
Original file line number Diff line number Diff line change
Expand Up @@ -988,6 +988,10 @@ struct FormatStyle {
/// \version 3.7
bool AllowShortLoopsOnASingleLine;

/// If ``true``, ``namespace a { class b; }`` can be put on a single line.
/// \version 20
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 {
Expand Down Expand Up @@ -5168,6 +5172,8 @@ struct FormatStyle {
R.AllowShortIfStatementsOnASingleLine &&
AllowShortLambdasOnASingleLine == R.AllowShortLambdasOnASingleLine &&
AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
AllowShortNamespacesOnASingleLine ==
R.AllowShortNamespacesOnASingleLine &&
AlwaysBreakBeforeMultilineStrings ==
R.AlwaysBreakBeforeMultilineStrings &&
AttributeMacros == R.AttributeMacros &&
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Format/Format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -975,6 +975,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",
Expand Down Expand Up @@ -1480,6 +1482,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");
Expand Down
92 changes: 91 additions & 1 deletion clang/lib/Format/UnwrappedLineFormatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 (Style.AllowShortNamespacesOnASingleLine &&
TheLine->First->is(tok::kw_namespace) &&
TheLine->Last->is(tok::l_brace)) {
const auto result = tryMergeNamespace(I, E, Limit);
if (result > 0)
return result;
}

if (Style.CompactNamespaces) {
if (const auto *NSToken = TheLine->First->getNamespaceToken()) {
int J = 1;
Expand All @@ -373,7 +382,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.
Expand Down Expand Up @@ -420,6 +429,7 @@ class LineJoiner {
TheLine->First != LastNonComment) {
return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
}

// 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,
Expand Down Expand Up @@ -616,6 +626,71 @@ class LineJoiner {
return 1;
}

unsigned tryMergeNamespace(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
SmallVectorImpl<AnnotatedLine *>::const_iterator E,
unsigned Limit) {
if (Limit == 0)
return 0;
assert(I[1]);
const auto &L1 = *I[1];
if (L1.InPPDirective != (*I)->InPPDirective ||
(L1.InPPDirective && L1.First->HasUnescapedNewline)) {
return 0;
}

if (std::distance(I, E) <= 2)
return 0;

assert(I[2]);
const auto &L2 = *I[2];
if (L2.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 (L1.First->is(tok::kw_namespace)) {
if (L1.Last->is(tok::comment) || !Style.CompactNamespaces)
return 0;

assert(Limit >= L1.Last->TotalLength + 3);
const auto InnerLimit = Limit - L1.Last->TotalLength - 3;
const auto MergedLines = tryMergeNamespace(I + 1, E, InnerLimit);
if (MergedLines == 0)
return 0;
const auto N = MergedLines + 2;
// Check if there is even a line after the inner result.
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[N]->First->is(tok::r_brace) && !I[N]->First->MustBreakBefore &&
I[MergedLines + 1]->Last->isNot(tok::comment) &&
nextNLinesFitInto(I, I + N + 1, Limit)) {
return N;
}
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 (L1.Last->isNot(tok::semi))
return 0;

// Last, check that the third line starts with a closing brace.
if (L2.First->isNot(tok::r_brace) || L2.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) {
Expand Down Expand Up @@ -916,6 +991,21 @@ 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 (const auto *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
Expand Down
1 change: 1 addition & 0 deletions clang/unittests/Format/ConfigParseTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(BreakAdjacentStringLiterals);
CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
Expand Down
116 changes: 116 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4504,6 +4504,12 @@ TEST_F(FormatTest, FormatsCompactNamespaces) {
"} // namespace bbbbbb\n"
"} // namespace aaaaaa",
Style);

verifyFormat("namespace a { namespace b {\n"
"namespace c {\n"
"}}} // namespace a::b::c",
Style);

Style.ColumnLimit = 80;

// Extra semicolon after 'inner' closing brace prevents merging
Expand Down Expand Up @@ -28314,6 +28320,116 @@ TEST_F(FormatTest, KeepFormFeed) {
Style);
}

TEST_F(FormatTest, ShortNamespacesOption) {
auto BaseStyle = getLLVMStyle();
BaseStyle.AllowShortNamespacesOnASingleLine = true;
BaseStyle.FixNamespaceComments = false;
BaseStyle.CompactNamespaces = true;

auto Style = BaseStyle;

// 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 { namespace baz {\n"
"class qux;\n"
"} // comment\n"
"}",
Style);

// Make sure code doesn't walk too 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);

// 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"
"}",
Style);
verifyFormat("namespace foo { template <T> struct bar; }", Style);
verifyFormat("namespace foo { constexpr int num = 42; }", Style);

// Validate nested namespace 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 { namespace baar { namespace baaz {\n"
"class quux;\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 { namespace bar { class qux; } } // extra",
Style);

verifyFormat("namespace foo { namespace bar { namespace baz {\n"
"class qux;\n"
"}}} // extra",
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
// 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 { namespace bar { namespace baz {\n"
"class qux;\n"
"}}} // namespace foo::bar::baz",
"namespace foo { namespace bar { namespace baz { class qux; } } }",
Style);
}

} // namespace
} // namespace test
} // namespace format
Expand Down
Loading