Skip to content

Commit 4118b7d

Browse files
author
Galen Elias
committed
clang-format: Add "AllowShortNamespacesOnASingleLine" option
This addresses: #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.
1 parent c557d85 commit 4118b7d

File tree

4 files changed

+202
-0
lines changed

4 files changed

+202
-0
lines changed

‎clang/include/clang/Format/Format.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,11 @@ struct FormatStyle {
972972
/// \version 3.7
973973
bool AllowShortLoopsOnASingleLine;
974974

975+
/// If ``true``, ``namespace a { class b; }`` can be put on a single a single
976+
/// line.
977+
/// \version 19
978+
bool AllowShortNamespacesOnASingleLine;
979+
975980
/// Different ways to break after the function definition return type.
976981
/// This option is **deprecated** and is retained for backwards compatibility.
977982
enum DefinitionReturnTypeBreakingStyle : int8_t {

‎clang/lib/Format/Format.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,8 @@ template <> struct MappingTraits<FormatStyle> {
952952
Style.AllowShortLambdasOnASingleLine);
953953
IO.mapOptional("AllowShortLoopsOnASingleLine",
954954
Style.AllowShortLoopsOnASingleLine);
955+
IO.mapOptional("AllowShortNamespacesOnASingleLine",
956+
Style.AllowShortNamespacesOnASingleLine);
955957
IO.mapOptional("AlwaysBreakAfterDefinitionReturnType",
956958
Style.AlwaysBreakAfterDefinitionReturnType);
957959
IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
@@ -1457,6 +1459,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
14571459
LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
14581460
LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
14591461
LLVMStyle.AllowShortLoopsOnASingleLine = false;
1462+
LLVMStyle.AllowShortNamespacesOnASingleLine = false;
14601463
LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
14611464
LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
14621465
LLVMStyle.AttributeMacros.push_back("__capability");

‎clang/lib/Format/UnwrappedLineFormatter.cpp

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,15 @@ class LineJoiner {
420420
TheLine->First != LastNonComment) {
421421
return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
422422
}
423+
424+
if (TheLine->Last->is(tok::l_brace)) {
425+
if (Style.AllowShortNamespacesOnASingleLine &&
426+
TheLine->First->is(tok::kw_namespace)) {
427+
if (unsigned result = tryMergeNamespace(I, E, Limit))
428+
return result;
429+
}
430+
}
431+
423432
// Try to merge a control statement block with left brace unwrapped.
424433
if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
425434
FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
@@ -616,6 +625,62 @@ class LineJoiner {
616625
return 1;
617626
}
618627

628+
unsigned tryMergeNamespace(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
629+
SmallVectorImpl<AnnotatedLine *>::const_iterator E,
630+
unsigned Limit) {
631+
if (Limit == 0)
632+
return 0;
633+
if (I[1]->InPPDirective != (*I)->InPPDirective ||
634+
(I[1]->InPPDirective && I[1]->First->HasUnescapedNewline)) {
635+
return 0;
636+
}
637+
if (I + 2 == E || I[2]->Type == LT_Invalid)
638+
return 0;
639+
640+
Limit = limitConsideringMacros(I + 1, E, Limit);
641+
642+
if (!nextTwoLinesFitInto(I, Limit))
643+
return 0;
644+
645+
// Check if it's a namespace inside a namespace, and call recursively if so
646+
// '3' is the sizes of the whitespace and closing brace for " _inner_ }"
647+
if (I[1]->First->is(tok::kw_namespace)) {
648+
if (I[1]->Last->is(TT_LineComment))
649+
return 0;
650+
651+
unsigned inner_limit = Limit - I[1]->Last->TotalLength - 3;
652+
unsigned inner_result = tryMergeNamespace(I + 1, E, inner_limit);
653+
if (!inner_result)
654+
return 0;
655+
// check if there is even a line after the inner result
656+
if (I + 2 + inner_result >= E)
657+
return 0;
658+
// check that the line after the inner result starts with a closing brace
659+
// which we are permitted to merge into one line
660+
if (I[2 + inner_result]->First->is(tok::r_brace) &&
661+
!I[2 + inner_result]->First->MustBreakBefore &&
662+
!I[1 + inner_result]->Last->is(TT_LineComment) &&
663+
nextNLinesFitInto(I, I + 2 + inner_result + 1, Limit)) {
664+
return 2 + inner_result;
665+
}
666+
return 0;
667+
}
668+
669+
// There's no inner namespace, so we are considering to merge at most one
670+
// line.
671+
672+
// The line which is in the namespace should end with semicolon
673+
if (I[1]->Last->isNot(tok::semi))
674+
return 0;
675+
676+
// Last, check that the third line starts with a closing brace.
677+
if (I[2]->First->isNot(tok::r_brace) || I[2]->First->MustBreakBefore)
678+
return 0;
679+
680+
// If so, merge all three lines.
681+
return 2;
682+
}
683+
619684
unsigned tryMergeSimpleControlStatement(
620685
SmallVectorImpl<AnnotatedLine *>::const_iterator I,
621686
SmallVectorImpl<AnnotatedLine *>::const_iterator E, unsigned Limit) {
@@ -916,6 +981,23 @@ class LineJoiner {
916981
return 1 + I[1]->Last->TotalLength + 1 + I[2]->Last->TotalLength <= Limit;
917982
}
918983

984+
bool nextNLinesFitInto(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
985+
SmallVectorImpl<AnnotatedLine *>::const_iterator E,
986+
unsigned Limit) {
987+
unsigned joinedLength = 0;
988+
for (SmallVectorImpl<AnnotatedLine *>::const_iterator J = I + 1; J != E;
989+
++J) {
990+
991+
if ((*J)->First->MustBreakBefore)
992+
return false;
993+
994+
joinedLength += 1 + (*J)->Last->TotalLength;
995+
if (joinedLength > Limit)
996+
return false;
997+
}
998+
return true;
999+
}
1000+
9191001
bool containsMustBreak(const AnnotatedLine *Line) {
9201002
assert(Line->First);
9211003
// Ignore the first token, because in this situation, it applies more to the

‎clang/unittests/Format/FormatTest.cpp

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27981,6 +27981,118 @@ TEST_F(FormatTest, BreakBinaryOperations) {
2798127981
Style);
2798227982
}
2798327983

27984+
TEST_F(FormatTest, ShortNamespacesOption) {
27985+
FormatStyle style = getLLVMStyle();
27986+
style.AllowShortNamespacesOnASingleLine = true;
27987+
style.FixNamespaceComments = false;
27988+
27989+
// Basic functionality
27990+
verifyFormat("namespace foo { class bar; }", style);
27991+
verifyFormat("namespace foo::bar { class baz; }", style);
27992+
verifyFormat("namespace { class bar; }", style);
27993+
verifyFormat("namespace foo {\n"
27994+
"class bar;\n"
27995+
"class baz;\n"
27996+
"}",
27997+
style);
27998+
27999+
// Trailing comments prevent merging
28000+
verifyFormat("namespace foo {\n"
28001+
"namespace baz { class qux; } // comment\n"
28002+
"}",
28003+
style);
28004+
28005+
// Make sure code doesn't walk to far on unbalanced code
28006+
verifyFormat("namespace foo {", style);
28007+
verifyFormat("namespace foo {\n"
28008+
"class baz;",
28009+
style);
28010+
verifyFormat("namespace foo {\n"
28011+
"namespace bar { class baz; }",
28012+
style);
28013+
28014+
// Nested namespaces
28015+
verifyFormat("namespace foo { namespace bar { class baz; } }", style);
28016+
verifyFormat("namespace foo {\n"
28017+
"namespace bar { class baz; }\n"
28018+
"namespace quar { class quaz; }\n"
28019+
"}",
28020+
style);
28021+
28022+
// Varying inner content
28023+
verifyFormat("namespace foo {\n"
28024+
"int f() { return 5; }\n"
28025+
"}",
28026+
style);
28027+
verifyFormat("namespace foo { template <T> struct bar; }", style);
28028+
verifyFormat("namespace foo { constexpr int num = 42; }", style);
28029+
28030+
// Validate wrapping scenarios around the ColumnLimit
28031+
style.ColumnLimit = 64;
28032+
28033+
// Validate just under the ColumnLimit
28034+
verifyFormat(
28035+
"namespace foo { namespace bar { namespace baz { class qux; } } }",
28036+
style);
28037+
28038+
// Validate just over the ColumnLimit
28039+
verifyFormat("namespace foo {\n"
28040+
"namespace bar { namespace baz { class quux; } }\n"
28041+
"}",
28042+
style);
28043+
28044+
verifyFormat("namespace foo {\n"
28045+
"namespace bar {\n"
28046+
"namespace baz { namespace qux { class quux; } }\n"
28047+
"}\n"
28048+
"}",
28049+
style);
28050+
28051+
// Validate that the ColumnLimit logic accounts for trailing content as well
28052+
verifyFormat("namespace foo {\n"
28053+
"namespace bar { namespace baz { class qux; } }\n"
28054+
"} // extra",
28055+
style);
28056+
28057+
// No ColumnLimit, allows long nested one-liners, but also leaves multi-line
28058+
// instances alone
28059+
style.ColumnLimit = 0;
28060+
verifyFormat("namespace foo { namespace bar { namespace baz { namespace qux "
28061+
"{ class quux; } } } }",
28062+
style);
28063+
28064+
verifyNoChange("namespace foo {\n"
28065+
"namespace bar {\n"
28066+
"namespace baz { namespace qux { class quux; } }\n"
28067+
"}\n"
28068+
"}",
28069+
style);
28070+
28071+
// This option doesn't really work with FixNamespaceComments and nested
28072+
// namespaces. Code should use the concatenated namespace syntax. e.g.
28073+
// 'namespace foo::bar'
28074+
style.FixNamespaceComments = true;
28075+
28076+
verifyFormat(
28077+
"namespace foo {\n"
28078+
"namespace bar { namespace baz { class qux; } } // namespace bar\n"
28079+
"} // namespace foo",
28080+
"namespace foo { namespace bar { namespace baz { class qux; } } }",
28081+
style);
28082+
28083+
// This option doesn't really make any sense with ShortNamespaceLines = 0
28084+
style.ShortNamespaceLines = 0;
28085+
28086+
verifyFormat(
28087+
"namespace foo {\n"
28088+
"namespace bar {\n"
28089+
"namespace baz { class qux; } // namespace baz\n"
28090+
"} // namespace bar\n"
28091+
"} // namespace foo",
28092+
"namespace foo { namespace bar { namespace baz { class qux; } } }",
28093+
style);
28094+
}
28095+
2798428096
} // namespace
2798528097
} // namespace test
2798628098
} // namespace format

0 commit comments

Comments
 (0)