Hi,

On Sun, 2013-12-08 at 21:32 +0100, Daniel Jasper wrote:
> I think we might just want to change the existing
> option BreakConstructorInitializersBeforeComma to do exactly this. WebKit
> style is the only style using it and I agree that the style guide is
> explicit in this regard.

that seems reasonable to me. Please find attached a patch which
implements that behavior.

Unlike the old one, the new patch also works if ColumnLimit is 0, so it
does work for WebKit, too.

Florian Sowade

> On Thu, Dec 5, 2013 at 10:50 PM, Nico Weber <[email protected]> wrote:
> 
> > (This is what WebKit style asks for too:
> > http://www.webkit.org/coding/coding-style.html , "Other Punctuation"
> > point 1. So if this goes in, maybe you can set to true for webkit style.)
> >
> >
> > On Thu, Dec 5, 2013 at 10:23 AM, Florian Sowade <[email protected]> wrote:
> >
> >> Hi,
> >>
> >> the attached patch adds a clang-format option to always break
> >> constructor initializers before the initial colon.
> >>
> >> So this patch adds an option to turn
> >>
> >> Constructor : foo(bar)
> >> {}
> >>
> >> into
> >>
> >> Constructor
> >>     : foo(bar)
> >> {}
> >>
> >> I don't know if this style is widely distributed, but I use it to
> >> minimize the diff when I add new members to the list of constructor
> >> initializers, since I can add a second member to the list without
> >> modifying any of the existing lines:
> >>
> >> Constructor
> >>     : foo(bar)
> >>     , baz()
> >> {}
> >>
> >> From my understanding this is not possible with the current set of
> >> options.
> >>
> >> Florian Sowade
diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index e63f72d..0c5a94b 100644
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -183,9 +183,13 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
       Current.LongestObjCSelectorName == 0 &&
       State.Stack.back().BreakBeforeParameter)
     return true;
-  if ((Current.Type == TT_CtorInitializerColon ||
-       (Previous.ClosesTemplateDeclaration && State.ParenLevel == 0 &&
-        !Current.isTrailingComment())))
+  if (Current.Type == TT_CtorInitializerColon &&
+      !(Style.AllowShortFunctionsOnASingleLine &&
+        !Style.BreakConstructorInitializersBeforeComma &&
+        Style.ColumnLimit == 0))
+    return true;
+  if ((Previous.ClosesTemplateDeclaration && State.ParenLevel == 0 &&
+       !Current.isTrailingComment()))
     return true;
 
   if ((Current.Type == TT_StartOfName || Current.is(tok::kw_operator)) &&
diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 87c2fd7..8661927 100644
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -455,13 +455,12 @@ public:
 
   /// \brief Formats the line starting at \p State, simply keeping all of the
   /// input's line breaking decisions.
-  void format(unsigned FirstIndent, const AnnotatedLine *Line,
-              bool LineIsMerged) {
+  void format(unsigned FirstIndent, const AnnotatedLine *Line) {
     LineState State =
         Indenter->getInitialState(FirstIndent, Line, /*DryRun=*/false);
     while (State.NextToken != NULL) {
       bool Newline =
-          (!LineIsMerged && Indenter->mustBreak(State)) ||
+          Indenter->mustBreak(State) ||
           (Indenter->canBreak(State) && State.NextToken->NewlinesBefore > 0);
       Indenter->addTokenToState(State, Newline, /*DryRun=*/false);
     }
@@ -728,8 +727,7 @@ public:
           // FIXME: Implement nested blocks for ColumnLimit = 0.
           NoColumnLimitFormatter Formatter(Indenter);
           if (!DryRun)
-            Formatter.format(Indent, &TheLine,
-                             /*LineIsMerged=*/MergedLines > 0);
+            Formatter.format(Indent, &TheLine);
         } else {
           Penalty += format(TheLine, Indent, DryRun);
         }
diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index 19759af..e2615db 100644
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -1407,7 +1407,8 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
              Right.Previous->MatchingParen->NestingLevel == 0 &&
              Style.AlwaysBreakTemplateDeclarations) {
     return true;
-  } else if (Right.Type == TT_CtorInitializerComma &&
+  } else if ((Right.Type == TT_CtorInitializerComma ||
+              Right.Type == TT_CtorInitializerColon) &&
              Style.BreakConstructorInitializersBeforeComma &&
              !Style.ConstructorInitializerAllOnOneLineOrOnePerLine) {
     return true;
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index bd9d108..5e38f59 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -7524,7 +7524,38 @@ TEST_F(FormatTest, ConstructorInitializerIndentWidth) {
                "    , b(b)\n"
                "    , c(c) {}",
                Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : a(a) {}",
+               Style);
+
+  Style.ColumnLimit = 0;
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : a(a) {}",
+               Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : a(a)\n"
+               "    , b(b)\n"
+               "    , c(c) {}",
+               Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : a(a) {\n"
+               "  foo();\n"
+               "  bar();\n"
+               "}",
+               Style);
+
+  Style.AllowShortFunctionsOnASingleLine = false;
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : a(a)\n"
+               "    , b(b)\n"
+               "    , c(c) {\n}",
+               Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : a(a) {\n}",
+               Style);
 
+  Style.ColumnLimit = 80;
+  Style.AllowShortFunctionsOnASingleLine = true;
   Style.ConstructorInitializerIndentWidth = 2;
   verifyFormat("SomeClass::Constructor()\n"
                "  : a(a)\n"
@@ -7541,6 +7572,10 @@ TEST_F(FormatTest, ConstructorInitializerIndentWidth) {
 
   Style.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
   Style.ConstructorInitializerIndentWidth = 4;
+  verifyFormat("SomeClass::Constructor() : aaaaaaaa(aaaaaaaa) {}", Style);
+  verifyFormat(
+      "SomeClass::Constructor() : aaaaa(aaaaa), aaaaa(aaaaa), aaaaa(aaaaa)\n",
+      Style);
   verifyFormat(
       "SomeClass::Constructor()\n"
       "    : aaaaaaaa(aaaaaaaa), aaaaaaaa(aaaaaaaa), aaaaaaaa(aaaaaaaa) {}",
@@ -7600,18 +7635,27 @@ TEST_F(FormatTest, FormatsWithWebKitStyle) {
 
   // Constructor initializers are formatted one per line with the "," on the
   // new line.
-  EXPECT_EQ(
-      "Constructor()\n"
-      "    : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-      "    , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n"
-      "                               aaaaaaaaaaaaaa)\n"
-      "    , aaaaaaaaaaaaaaaaaaaaaaa() {}",
-      format("Constructor()\n"
-             "    : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
-             "    , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n"
-             "                               aaaaaaaaaaaaaa)\n"
-             "    , aaaaaaaaaaaaaaaaaaaaaaa() {}",
-             Style));
+  verifyFormat("Constructor()\n"
+               "    : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"
+               "    , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n"
+               "                               aaaaaaaaaaaaaa)\n"
+               "    , aaaaaaaaaaaaaaaaaaaaaaa() {}",
+               Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : a(a) {}",
+               Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : a(a)\n"
+               "    , b(b)\n"
+               "    , c(c) {}",
+               Style);
+  verifyFormat("SomeClass::Constructor()\n"
+               "    : a(a)\n"
+               "{\n"
+               "    foo();\n"
+               "    bar();\n"
+               "}",
+               Style);
 
   // Access specifiers should be aligned left.
   verifyFormat("class C {\n"
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to