https://github.com/galenelias created https://github.com/llvm/llvm-project/pull/105597
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. >From 3819c15c8c6258d1d29a38fcfb71db8567fb34e5 Mon Sep 17 00:00:00 2001 From: Galen Elias <gel...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
