clang-format: Add "AllowShortNamespacesOnASingleLine" option

[cbeck88]

Summary

Rationale:

I sometimes use a different clang tool, iwyu ("include what you use"), to clean up header file inclusions in my C++ projects. Iwyu seeks to correct the includes of a header or cpp unit so that definitions which are needed are included, and definitions which only need to be forward declared are forward declared. It often generates code like this at the top of your file:

namespace foo { class bar; }
namespace baz { struct quaz; }
...

Unfortunately, clang-format dislikes braces which are arranged this way and always wants to break after them, and after the forward declaration, no matter what configuration options are used (as far as I can tell).

I wrote this small patch so that short namespaces like these can be set on a single line regardless of chosen brace-style if a boolean option "AllowShortNamespacesOnASingleLine" is enabled.

[djasper]

include/clang/Format/Format.h

115

Don't use markup here. Use a \code block instead.

lib/Format/UnwrappedLineFormatter.cpp

204

No braces.

267

How is this different from tryMergeSimpleBlock, in particular, which behavior differences do you want?

unittests/Format/FormatTest.cpp

10441

Please insert a pair of quotes ("") after each '\n' and format, so that this becomes a concatenated multiline string.

What about:

namespace A { namespace B { class C; }}

I think this is what many people would use. It's fine not to fix this in this patch, but at least denote the behavior with a corresponding test.

[cbeck88]

• Fixed formatting in header
• Fixed handling of nested namespaces
• Added unit tests

djasper: I thought about it, I guess I don't have a particularly good reason that the namespaces logic should be a separate function from "tryMergeSimpleBlock", and maybe it should indeed be refactored to be part of the case analysis of that function. When I began I thought this would just be simpler and make it easier to customize the namespace handling vs. other blocks, but now I'm less sure of this. At least with the way I'm now trying to handle the nested namespaces, this seems a bit simpler, but maybe that part should be changed.

include/clang/Format/Format.h

   /// single line.
   bool AllowShortLoopsOnASingleLine;
 
+  /// \brief If \c true, <tt> namespace a { class b; } </tt> can be put
+  /// on a single line
+  bool AllowShortNamespacesOnASingleLine;
+
   /// \brief Different ways to break after the function definition return type.
   enum DefinitionReturnTypeBreakingStyle {
     /// Break after return type automatically.

lib/Format/Format.cpp

                    Style.AllowShortIfStatementsOnASingleLine);
     IO.mapOptional("AllowShortLoopsOnASingleLine",
                    Style.AllowShortLoopsOnASingleLine);
+    IO.mapOptional("AllowShortNamespacesOnASingleLine",
+                   Style.AllowShortNamespacesOnASingleLine);
     IO.mapOptional("AlwaysBreakAfterDefinitionReturnType",
                    Style.AlwaysBreakAfterDefinitionReturnType);
     IO.mapOptional("AlwaysBreakBeforeMultilineStrings",

   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
+  LLVMStyle.AllowShortNamespacesOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = false;

lib/Format/UnwrappedLineFormatter.cpp

       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;
+        }
+      }
       return Style.BreakBeforeBraces == FormatStyle::BS_Attach
                  ? tryMergeSimpleBlock(I, E, Limit)
                  : 0;

     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;
+
+    Limit = limitConsideringMacros(I + 1, E, Limit);
+
+    if (1 + I[1]->Last->TotalLength > Limit)
+      return 0;
+
+    // An assumption of the function is that the first line begins with
+    // keyword namespace (i.e. I[0]->First->is(tok::kw_namespace))
+
+    // 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))
+      return 0;
+
+    // If so, merge all three lines.
+    return 2;
+  }
+
   unsigned tryMergeSimpleControlStatement(
       SmallVectorImpl<AnnotatedLine *>::const_iterator I,
       SmallVectorImpl<AnnotatedLine *>::const_iterator E, unsigned Limit) {

unittests/Format/FormatTest.cpp

   verifyNoCrash("#define a\\\n /**/}");
 }
 
+TEST_F(FormatTest, ShortNamespacesOption) {
+  FormatStyle style = getLLVMStyle();
+  style.AllowShortNamespacesOnASingleLine = true;
+
+  verifyFormat("namespace foo { class bar; }", style);
+  verifyFormat("namespace foo {\nclass bar;\nclass baz;\n}", style);
+  verifyFormat("namespace foo {\nint f() { return 5; }\n}", style);
+  verifyFormat("namespace foo { template <T> struct bar; }", style);
+  verifyFormat("namespace foo { constexpr int num = 42; }", style);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang