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

Author: Galen Elias

clang/docs/ClangFormatStyleOptions.rst

@@ -2091,8 +2091,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:
 

clang/docs/ReleaseNotes.rst

@@ -1122,6 +1122,8 @@ clang-format
 - Adds ``RemoveEmptyLinesInUnwrappedLines`` option.
 - Adds ``KeepFormFeed`` option and set it to ``true`` for ``GNU`` style.
 
+- Adds ``AllowShortNamespacesOnASingleLine`` option.
+
 libclang
 --------
 - Add ``clang_isBeforeInTranslationUnit``. Given two source locations, it determines

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;
       }

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
@@ -28318,6 +28328,7 @@ TEST_F(FormatTest, ShortNamespacesOption) {
   auto BaseStyle = getLLVMStyle();
   BaseStyle.AllowShortNamespacesOnASingleLine = true;
   BaseStyle.FixNamespaceComments = false;
+  BaseStyle.CompactNamespaces = true;
 
   auto Style = BaseStyle;
 
@@ -28332,8 +28343,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);
 
@@ -28348,12 +28360,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"
@@ -28362,7 +28385,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.
@@ -28371,22 +28394,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
@@ -28401,41 +28426,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);
 }