catskul created this revision.
catskul added a reviewer: clang-format.
catskul added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
catskul requested review of this revision.

I'm trying to get D14484 <https://reviews.llvm.org/D14484> past the finish line 
so I've adopted it.

This revision:

- fixes the unit tests
- resolves cases where the colon is attached (where the original did not)
- removes non-sensical 'backward compatibility'


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90232

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4667,8 +4667,51 @@
                "      aaaaaaaaaaa(aaaaaaaaaaa),\n"
                "      aaaaaaaaaaaaaaaaaaaaat(aaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}");
 
+  FormatStyle BestFit = getLLVMStyle();
+  BestFit.ConstructorInitializer = FormatStyle::CI_BestFit;
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
+               "      aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
+               "      aaaaaaaaaaaaa(aaaaaaaaaaaaaa) {}",
+               BestFit);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : aaaaaaaaaaaaa(aaaaaaaaaaaaaa), // Some comment\n"
+               "      aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
+               "      aaaaaaaaaaaaa(aaaaaaaaaaaaaa) {}",
+               BestFit);
+  verifyFormat("MyClass::MyClass(int var)\n"
+               "    : some_var_(var),            // 4 space indent\n"
+               "      some_other_var_(var + 1) { // lined up\n"
+               "}",
+               BestFit);
+  verifyFormat("Constructor() : a(a), a(a) {}\n",
+               BestFit);
+  verifyFormat("Constructor()\n"
+               "    : aaaaa(aaaaaa),\n"
+               "      aaaaa(aaaaaa),\n"
+               "      aaaaa(aaaaaa),\n"
+               "      aaaaa(aaaaaa),\n"
+               "      aaaaa(aaaaaa) {}",
+               BestFit);
+  verifyFormat("Constructor()\n"
+               "    : aaaaa(aaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaa,\n"
+               "            aaaaaaaaaaaaaaaaaaaaaa) {}",
+               BestFit);
+  BestFit.ColumnLimit = 60;
+  verifyFormat("Constructor()\n"
+               "    : aaaaaaaaaaaaaaaaaaaa(a),\n"
+               "      bbbbbbbbbbbbbbbbbbbbbbbb(b) {}",
+               BestFit);
+
+  EXPECT_EQ("Constructor()\n"
+            "    : // Comment forcing unwanted break.\n"
+            "      aaaa(aaaa) {}",
+            format("Constructor() :\n"
+                   "    // Comment forcing unwanted break.\n"
+                   "    aaaa(aaaa) {}"));
+
   FormatStyle OnePerLine = getLLVMStyle();
-  OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  OnePerLine.ConstructorInitializer = FormatStyle::CI_OnePerLine;
   OnePerLine.AllowAllParametersOfDeclarationOnNextLine = false;
   verifyFormat("SomeClass::Constructor()\n"
                "    : aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
@@ -4685,6 +4728,14 @@
                "      some_other_var_(var + 1) { // lined up\n"
                "}",
                OnePerLine);
+  verifyFormat("Constructor()\n"
+               "    : a(a),\n"
+               "      a(a) {}",
+               OnePerLine);
+  verifyFormat("Constructor()\n"
+               "    : a(a),\n"
+               "      a(a) {}",
+               OnePerLine);
   verifyFormat("Constructor()\n"
                "    : aaaaa(aaaaaa),\n"
                "      aaaaa(aaaaaa),\n"
@@ -4721,7 +4772,7 @@
   FormatStyle Style = getLLVMStyle();
   Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeComma;
   Style.ColumnLimit = 60;
-  Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  Style.ConstructorInitializer = FormatStyle::CI_BestFit;
   Style.AllowAllConstructorInitializersOnNextLine = true;
   Style.BinPackParameters = false;
 
@@ -4919,13 +4970,13 @@
   verifyFormat("template <typename T>\n"
                "Constructor() : Initializer(FitsOnTheLine) {}",
                getStyleWithColumns(Style, 50));
-  Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  Style.ConstructorInitializer = FormatStyle::CI_BestFit;
   verifyFormat(
       "SomeClass::Constructor() :\n"
       "    aaaaaaaaaaaaa(aaaaaaaaaaaaaa), aaaaaaaaaaaaaaa(aaaaaaaaaaaa) {}",
       Style);
 
-  Style.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
+  Style.ConstructorInitializer = FormatStyle::CI_Compact;
   verifyFormat(
       "SomeClass::Constructor() :\n"
       "    aaaaaaaaaaaaa(aaaaaaaaaaaaaa), aaaaaaaaaaaaaaa(aaaaaaaaaaaa) {}",
@@ -4980,7 +5031,7 @@
                Style);
 
   FormatStyle OnePerLine = Style;
-  OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  Style.ConstructorInitializer = FormatStyle::CI_BestFit;
   OnePerLine.AllowAllConstructorInitializersOnNextLine = false;
   verifyFormat("SomeClass::Constructor() :\n"
                "    aaaaaaaaaaaaa(aaaaaaaaaaaaaa),\n"
@@ -5158,7 +5209,7 @@
 
   // This test takes VERY long when memoization is broken.
   FormatStyle OnePerLine = getLLVMStyle();
-  OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  OnePerLine.ConstructorInitializer = FormatStyle::CI_BestFit;
   OnePerLine.BinPackParameters = false;
   std::string input = "Constructor()\n"
                       "    : aaaa(a,\n";
@@ -13658,7 +13709,6 @@
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
-  CHECK_PARSE_BOOL(ConstructorInitializerAllOnOneLineOrOnePerLine);
   CHECK_PARSE_BOOL(DeriveLineEnding);
   CHECK_PARSE_BOOL(DerivePointerAlignment);
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
@@ -13893,6 +13943,14 @@
   CHECK_PARSE("SpaceAfterControlStatementKeyword: true", SpaceBeforeParens,
               FormatStyle::SBPO_ControlStatements);
 
+  Style.ConstructorInitializer = FormatStyle::CI_BestFit;
+  CHECK_PARSE("ConstructorInitializer: Compact", ConstructorInitializer,
+              FormatStyle::CI_Compact);
+  CHECK_PARSE("ConstructorInitializer: BestFit", ConstructorInitializer,
+              FormatStyle::CI_BestFit);
+  CHECK_PARSE("ConstructorInitializer: OnePerLine", ConstructorInitializer,
+              FormatStyle::CI_OnePerLine);
+
   Style.ColumnLimit = 123;
   FormatStyle BaseStyle = getLLVMStyle();
   CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);
@@ -14455,7 +14513,7 @@
                ", c(c) {}",
                Style);
 
-  Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  Style.ConstructorInitializer = FormatStyle::CI_BestFit;
   Style.ConstructorInitializerIndentWidth = 4;
   verifyFormat("SomeClass::Constructor() : aaaaaaaa(aaaaaaaa) {}", Style);
   verifyFormat(
@@ -14472,6 +14530,52 @@
                "    , aaaaaaaa(aaaaaaaa)\n"
                "    , aaaaaaaa(aaaaaaaa) {}",
                Style);
+
+  Style.ConstructorInitializer = FormatStyle::CI_OnePerLine;
+  Style.ConstructorInitializerIndentWidth = 4;
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : aaaaaaaa(aaaaaaaa) {}",
+               Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : aaaaa(aaaaa)\n"
+               "    , aaaaa(aaaaa)\n"
+               "    , aaaaa(aaaaa)\n",
+               Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : aaaaaaaa(aaaaaaaa)\n"
+               "    , aaaaaaaa(aaaaaaaa)\n"
+               "    , aaaaaaaa(aaaaaaaa) {}",
+               Style);
+  Style.ConstructorInitializerIndentWidth = 4;
+  Style.ColumnLimit = 60;
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : aaaaaaaa(aaaaaaaa)\n"
+               "    , aaaaaaaa(aaaaaaaa)\n"
+               "    , aaaaaaaa(aaaaaaaa) {}",
+               Style);
+
+  Style.ConstructorInitializer = FormatStyle::CI_OnePerLine;
+  Style.ConstructorInitializerIndentWidth = 4;
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : aaaaaaaa(aaaaaaaa) {}",
+               Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : aaaaa(aaaaa)\n"
+               "    , aaaaa(aaaaa)\n"
+               "    , aaaaa(aaaaa)\n",
+               Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : aaaaaaaa(aaaaaaaa)\n"
+               "    , aaaaaaaa(aaaaaaaa)\n"
+               "    , aaaaaaaa(aaaaaaaa) {}",
+               Style);
+  Style.ConstructorInitializerIndentWidth = 4;
+  Style.ColumnLimit = 60;
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : aaaaaaaa(aaaaaaaa)\n"
+               "    , aaaaaaaa(aaaaaaaa)\n"
+               "    , aaaaaaaa(aaaaaaaa) {}",
+               Style);
 }
 
 TEST_F(FormatTest, Destructors) {
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3524,12 +3524,26 @@
     return true;
   if (Right.is(TT_CtorInitializerComma) &&
       Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma &&
-      !Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
+      Style.ConstructorInitializer == FormatStyle::CI_Compact)
     return true;
   if (Right.is(TT_CtorInitializerColon) &&
       Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma &&
-      !Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
+      Style.ConstructorInitializer == FormatStyle::CI_Compact)
     return true;
+  if (Right.is(TT_CtorInitializerColon) &&
+      Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeColon &&
+      Style.ConstructorInitializer == FormatStyle::CI_OnePerLine)
+    return true;
+  if (Right.isOneOf(TT_CtorInitializerComma, TT_CtorInitializerColon) &&
+      Style.BreakConstructorInitializers == FormatStyle::BCIS_BeforeComma &&
+      Style.ConstructorInitializer == FormatStyle::CI_Compact)
+    return true;
+
+  if (Left.is(TT_CtorInitializerComma) &&
+      Style.BreakConstructorInitializers != FormatStyle::BCIS_BeforeComma &&
+      Style.ConstructorInitializer == FormatStyle::CI_OnePerLine)
+    return true;
+
   // Break only if we have multiple inheritance.
   if (Style.BreakInheritanceList == FormatStyle::BILS_BeforeComma &&
       Right.is(TT_InheritanceComma))
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -366,6 +366,18 @@
   }
 };
 
+
+template <>
+struct ScalarEnumerationTraits<FormatStyle::ConstructorInitializerKind> {
+  static void enumeration(IO &IO,
+                          FormatStyle::ConstructorInitializerKind &Value) {
+    IO.enumCase(Value, "Compact", FormatStyle::CI_Compact);
+    IO.enumCase(Value, "BestFit", FormatStyle::CI_BestFit);
+    IO.enumCase(Value, "OnePerLine", FormatStyle::CI_OnePerLine);
+  }
+};
+
+
 template <> struct MappingTraits<FormatStyle> {
   static void mapping(IO &IO, FormatStyle &Style) {
     // When reading, read the language first, we need it for getPredefinedStyle.
@@ -502,8 +514,15 @@
     IO.mapOptional("ColumnLimit", Style.ColumnLimit);
     IO.mapOptional("CommentPragmas", Style.CommentPragmas);
     IO.mapOptional("CompactNamespaces", Style.CompactNamespaces);
+
+    // Provide backward compatibility vs ConstructorInitializer
+    bool ConstructorInitializerAllOnOneLineOrOnePerLine = false;
     IO.mapOptional("ConstructorInitializerAllOnOneLineOrOnePerLine",
-                   Style.ConstructorInitializerAllOnOneLineOrOnePerLine);
+                   ConstructorInitializerAllOnOneLineOrOnePerLine);
+    if (ConstructorInitializerAllOnOneLineOrOnePerLine)
+      Style.ConstructorInitializer = FormatStyle::CI_BestFit;
+
+    IO.mapOptional("ConstructorInitializer", Style.ConstructorInitializer);
     IO.mapOptional("ConstructorInitializerIndentWidth",
                    Style.ConstructorInitializerIndentWidth);
     IO.mapOptional("ContinuationIndentWidth", Style.ContinuationIndentWidth);
@@ -861,7 +880,7 @@
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
   LLVMStyle.CompactNamespaces = false;
-  LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
+  LLVMStyle.ConstructorInitializer = FormatStyle::CI_Compact;
   LLVMStyle.ConstructorInitializerIndentWidth = 4;
   LLVMStyle.ContinuationIndentWidth = 4;
   LLVMStyle.Cpp11BracedListStyle = true;
@@ -964,7 +983,7 @@
   GoogleStyle.AllowShortLoopsOnASingleLine = true;
   GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
   GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
-  GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
+  GoogleStyle.ConstructorInitializer = FormatStyle::CI_BestFit;
   GoogleStyle.DerivePointerAlignment = true;
   GoogleStyle.IncludeStyle.IncludeCategories = {{"^<ext/.*\\.h>", 2, 0},
                                                 {"^<.*\\.h>", 1, 0},
Index: clang/lib/Format/ContinuationIndenter.cpp
===================================================================
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1215,7 +1215,7 @@
              ? 0
              : 2);
     State.Stack.back().NestedBlockIndent = State.Stack.back().Indent;
-    if (Style.ConstructorInitializerAllOnOneLineOrOnePerLine) {
+    if (Style.ConstructorInitializer != FormatStyle::CI_Compact) {
       State.Stack.back().AvoidBinPacking = true;
       State.Stack.back().BreakBeforeParameter =
           !Style.AllowAllConstructorInitializersOnNextLine;
@@ -1228,8 +1228,10 @@
     State.Stack.back().Indent =
         State.FirstIndent + Style.ConstructorInitializerIndentWidth;
     State.Stack.back().NestedBlockIndent = State.Stack.back().Indent;
-    if (Style.ConstructorInitializerAllOnOneLineOrOnePerLine)
+    if (Style.ConstructorInitializer != FormatStyle::CI_Compact)
       State.Stack.back().AvoidBinPacking = true;
+    State.Stack.back().BreakBeforeParameter =
+        Style.ConstructorInitializer == FormatStyle::CI_OnePerLine;
   }
   if (Current.is(TT_InheritanceColon))
     State.Stack.back().Indent =
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1295,8 +1295,17 @@
   ///     return 0;
   ///   }
   /// \endcode
-  bool ConstructorInitializerAllOnOneLineOrOnePerLine;
+  /// which should not be split into lines or otherwise changed.
   // clang-format on
+  enum ConstructorInitializerKind {
+    CI_Compact,    ///< Put all initializers in a compact block
+    CI_BestFit,    ///< Use the compact if all fits on a single line,
+                   ///< otherwise put one per line.
+    CI_OnePerLine, ///< Put all initializers on their own line
+  };
+
+  /// The placement of the constructor initializers.
+  ConstructorInitializerKind ConstructorInitializer;
 
   /// The number of characters to use for indentation of constructor
   /// initializer lists as well as inheritance lists.
@@ -2329,8 +2338,7 @@
            BreakStringLiterals == R.BreakStringLiterals &&
            ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas &&
            BreakInheritanceList == R.BreakInheritanceList &&
-           ConstructorInitializerAllOnOneLineOrOnePerLine ==
-               R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
+           ConstructorInitializer == R.ConstructorInitializer &&
            ConstructorInitializerIndentWidth ==
                R.ConstructorInitializerIndentWidth &&
            ContinuationIndentWidth == R.ContinuationIndentWidth &&
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1470,10 +1470,20 @@
     namespace Extra {
     }}}
 
-**ConstructorInitializerAllOnOneLineOrOnePerLine** (``bool``)
-  If the constructor initializers don't fit on a line, put each
-  initializer on its own line.
+**ConstructorInitializer** (``ConstructorInitializerKind``)
+  Formatting of the constructor initializer.
 
+  Possible values:
+
+  * ``CI_Compact`` (in configuration: ``Compact``)
+    Puts all the initializers in a compact block of multiple lines with
+    multiple initializers on the same line.
+  * ``CI_BestFit`` (in configuration: ``BestFit``)
+    Eiter all initializers are on a single line (it this fits),
+    otherwise every initializer has its own line.
+  * ``CI_OnePerLine`` (in configuration: ``OnePerLine``)
+    Always put the initializers on their own line, regardless if they would fit
+    on a single line.
   .. code-block:: c++
 
     true:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to