[clang-format] Add ability to wrap braces after multi-line control statements

[mitchell-stellar]

Summary

Change the BraceWrappingFlags' AfterControlStatement from a bool to an enum with three values:

• "Never": This is the default, and does not do any brace wrapping after control statements.
• "MultiLine": This only wraps braces after multi-line control statements (this really only happens when a ColumnLimit is specified).
• "Always": This always wraps braces after control statements.

The first and last options are backwards-compatible with "false" and "true", respectively.

The new "MultiLine" option is useful for when a wrapped control statement's indentation matches the subsequent block's indentation. It makes it easier to see at a glance where the control statement ends and where the block's code begins. For example:

if (
  foo
  && bar )
{
  baz();
}

vs.

if (
  foo
  && bar ) {
  baz();
}

Short control statements (1 line) do not wrap the brace to the next line, e.g.

if (foo) {
  bar();
} else {
  baz();
}

[MyDeveloperDay]

Firstly thank you for the patch..

Prior authors have pointed out that new styles need to be supported by a large public style guide as evidence as to why clang-format should support such changes. (if you can find one that points to this being a preference I would reference it here for completeness, see note below)

My personal view is we are getting to the point where so many large projects are already clang-formated to some extent, the definition of the style guide could be based on what clang-format can do and not on necessarily makes the code more readable. What I have begun to notice is some projects on github where clang-format can't support their style simply don't have a .clang-format file. (and that could often be because of 1 or 2 issues which prevent them from adopting it fully.) what is also strange is that that may have a .clang-tidy file which points to them potentially being keen to adopt the clang tools, but why not clang-format when its clearly the simplest to adaopt first)

The reason I ask is to uncover the motivation for adding the change? is it something you need or a nice to have? (adding that to the revision descriptrion helps the reviewers understand the motivation)

We'd normally ask that people be willing to support their revision should there be any need to fix issues, I'd also say that clang-format desperately needs additional developers to help fix issues and in particular to do code reviews,
I only say this because this is seemingly your first revision and whilst I think this revision LGTM I'd hope we could call on you to help maintain clang-format moving forward (given you have an ability to understand how it works in order to do this patch)

Now regarding this revision. It wasn't initially clear what this change gave us, I now I look deeper I see it as being the ability to break the brace when the condition (if,for,catch,etc..) gets itself broken over multiple lines, leading to the view that:

if (aaaaaaaaaaaaaaaaaaaaa 
    && bbbbbbbbbbbbbbb 
    && cccccccccccccc)
{
    return true;
}
else {
    return false;
}

is easier to read than:

if (aaaaaaaaaaaaaaaaaaaaa 
    && bbbbbbbbbbbbbbb 
    && cccccccccccccc) {
    return true;
}
else {
    return false;
}

whilst still recognizing the desire for the compact version when the condition is small.

if (true) {
    return true;
}
else {
    return false;
}

To be honest, I'd be more than happy to see this style as it most closely matches what I used to do all the time before I switched to clang-formatting on save all the time (which meant I couldn't), That was to use a different bracing style based on the complexity of the condition.

I guess my question is do you thin any of the other bracing wrapping rules need the same level of control? (I can't think of any off the top of my head)

Apart from the missing documentation this LGTM (which is why I requested changes), I'm not sure if others have a different view (maybe give them some time to review this), but you'll find it can be quite quiet here. (which is why we need more volunteers to do reviews).

If you can find that style guide evidence!

clang/include/clang/Format/Format.h

817 ↗

(On Diff #222695)

You are missing a doc change in the clang/docs/ClangStyleOptions.rst file (which I believe you can autogenerate from the Format.h), this needs to be part of the patch

clang/lib/Format/Format.cpp

643 ↗

(On Diff #222695)

Nit: this a drive by comment, but I feel this is really hard to read.. and not something that needs to be done in this revision but this feels like it needs to be formatted differently

{
/*AfterClass=*/false,
/*AfterEnum=*/false
/*AfterControlStatement=*/FormatStyle::BWACS_Never,
.....
}

because without going back and checking that the AfterControl statement is the 3rd bool I simply can't see what is on and what is off.

[mitchell-stellar]

Thanks for the review. I have added documentation updates.

I do not have a public style guide to reference. My company just switched to auto-clang-formatting all of our code, and this patch addresses one of our biggest pain points: readability. When the opening brace of a multi-line control statement is not wrapped, we simply cannot tell at first glance where the control statement ends and where the body begins due to our indentation settings.

I think the fact that you yourself see the benefit of this change speaks to its viability.

I cannot think of any other brace wrapping rules that need this level of control. Our code base consists of millions of lines of code, so I'm pretty sure we would have found another instance.

I would be willing to support this patch and further revisions if this gets into master, as our company will depend on this feature.

[klimek]

In D68296#1691393, @mitchell-stellar wrote:

Thanks for the review. I have added documentation updates.

I do not have a public style guide to reference. My company just switched to auto-clang-formatting all of our code, and this patch addresses one of our biggest pain points: readability. When the opening brace of a multi-line control statement is not wrapped, we simply cannot tell at first glance where the control statement ends and where the body begins due to our indentation settings.

I think the fact that you yourself see the benefit of this change speaks to its viability.

I cannot think of any other brace wrapping rules that need this level of control. Our code base consists of millions of lines of code, so I'm pretty sure we would have found another instance.

I would be willing to support this patch and further revisions if this gets into master, as our company will depend on this feature.

I personally don't think the feature is worth it (readability is something we really like to assert on without good measurements), but I will defer to MyDeveloperDay.

[MyDeveloperDay]

As I mentioned, its worth getting evidence from a large public project...

https://wiki.blender.org/index.php/Dev:Doc/Code_Style

Blender public repo on github.com has 90 Forks, 299 * and 90,000 commits

From their style guide

Exceptions to Braces on Same Line
When flow control (if, for, while) is broken up into multiple lines, it reads better to have the brace on its own lines.

/* Don't: */
    if ((very_long_function_check(that, has, many, arguments)) &&
        (another_very_long_function(some, more, arguments)) &&
        some_more_checks_this_gets_tedious(but, its, readable)) {
        some_other_check_this_could_be_confusing ? func_a() : func_b();
    }

/* Do: */
    if ((very_long_function_check(that, has, many, arguments)) &&
        (another_very_long_function(some, more, arguments)) &&
        some_more_checks_this_gets_tedious(but, its, readable))
    {
        (some_other_check_this_could_be_confusing && n) ? func_a() : func_b();
    }

https://developer.blender.org/T53211

# NOTE: At time of writing (10/30/2017) not all formatting can be made exactly
# like current Blender sources, so a few compromises are made:
#
#   1. Newline after : in switch statements: clang-format will put the { on
#      the same line. This is due to a limitation in clang-format; it does not
#      support adding the newline after cases in switch statements.
#   2. Nested preprocessor directives don't get proper indentation after the
#      '#'. See IndentPPDirectives, which is supported in clang-format from
#      LLVM6, but not LLVM5. It is included below but commented out.
#   3. Special case of { position on if statements where the condition covers
#      more than one line. clang-format is an all or nothing formatter in this
#      case; so unfortunately the case of
#
#      if (long_condition_here() ||
#          long_condition_here() ||
#          long_condition_here() ||
#          long_condition_here())
#      {
#          ...
#      }
#
#      will become
#
#      if (long_condition_here() ||
#          long_condition_here() ||
#          long_condition_here() ||
#          long_condition_here()) {
#          ...
#      }
#

[MyDeveloperDay]

@klimek thanks for offering for me to make the decision. I think given the fact that blender has it in their style guide and that they are clearly working around clang-formats inability to handle this case, I'm minded to LGTM this revision

I hope you are ok with that. given that @mitchell-stellar is prepared to support it, and I'm happy enough that I could fix something with that I hope you don't mind.

@mitchell-stellar are you happy with the name OnlyMultiLine? the reason I ask is:

AlwaysBreakBeforeMultilineStrings uses just MultiLine

BTDS_MultiLine (in configuration: MultiLine) Force break after template declaration only when the following declaration spans multiple lines.

I think given this won't be a standard true/false parameter users having to remember the "Only" part seems unnecessary

[mitchell-stellar]

@MyDeveloperDay I am not wedded to "OnlyMultiLine". I picked it, as it seemed reasonable to me, but if you think another string expresses the intent better, then feel free to use it.

[MyDeveloperDay]

Looks good but I think the option would be better as just MultiLine, remember to mark the comments "Done" when you are complete

clang/docs/ClangFormatStyleOptions.rst

798 ↗

(On Diff #222835)

Change to MultiLine to match AlwaysBreakBeforeMultilineStrings

[mitchell-stellar]

Renamed "OnlyMultiLine" option to "MultiLine", per reviewer request.

[mitchell-stellar]

clang/lib/Format/Format.cpp

643 ↗

(On Diff #222695)

I'm going to leave this as-is, because before, I was still forced to look-up what each true or false value was assigned to.

[MyDeveloperDay]

LGTM and Thank you.

clang/lib/Format/Format.cpp

643 ↗

(On Diff #222695)

that's fine, it's always the problem with comments they can also get out of date, to be honest, if every element was an enum then it would be:

{
FormatStyle::BWAC::Never,
FormatStyle::BWAE::Never,
.....

and when the structure changes the compiler would tell us if we had it wrong, but thats for another time, your enum will help begin that process.

[mitchell-stellar]

Rebased against master, and also added test case for 'do' statements.

[MyDeveloperDay]

cfe/trunk/docs/ClangFormatStyleOptions.rst

812

This text cannot be generated by dump_format_style.py in clang/docs/tools from the Format.h

I think this is the first time we've seen an enum being used in a structure, as such dump_format_style doesn't support pulling the text in automatically.

The next person who runs dump_format_style could easily end up blowing away your documentation.

cfe/trunk/docs/ClangFormatStyleOptions.rst

.. code-block:: c++
 
       true:
       class foo {};
 
       false:
       class foo
       {};
 
-  * ``bool AfterControlStatement`` Wrap control statements (``if``/``for``/``while``/``switch``/..).
+  * ``BraceWrappingAfterControlStatementStyle AfterControlStatement``
+    Wrap control statements (``if``/``for``/``while``/``switch``/..).
+
+    Possible values:
+
+    * ``BWACS_Never`` (in configuration: ``Never``)
+      Never wrap braces after a control statement.
+
+      .. code-block:: c++
+
+        if (foo()) {
+        } else {
+        }
+        for (int i = 0; i < 10; ++i) {
+        }
+
+    * ``BWACS_MultiLine`` (in configuration: ``MultiLine``)
+      Only wrap braces after a multi-line control statement.
+
+      .. code-block:: c++
+
+        if (foo && bar &&
+            baz)
+        {
+          quux();
+        }
+        while (foo || bar) {
+        }
+
+    * ``BWACS_Always`` (in configuration: ``Always``)
+      Always wrap braces after a control statement.
 
       .. code-block:: c++
 
-      true:
         if (foo())
         {
         } else
         {}
         for (int i = 0; i < 10; ++i)
         {}
 
-      false:
-      if (foo()) {
-      } else {
-      }
-      for (int i = 0; i < 10; ++i) {
-      }
-
   * ``bool AfterEnum`` Wrap enum definitions.
 
     .. code-block:: c++
 
       true:
       enum X : int
       {
         B

cfe/trunk/include/clang/Format/Format.h

enum BraceBreakingStyle {
     BS_WebKit,
     /// Configure each individual brace in `BraceWrapping`.
     BS_Custom
   };
 
   /// The brace breaking style to use.
   BraceBreakingStyle BreakBeforeBraces;
 
+  // Different ways to wrap braces after control statements.
+  enum BraceWrappingAfterControlStatementStyle {
+    /// Never wrap braces after a control statement.
+    /// \code
+    ///   if (foo()) {
+    ///   } else {
+    ///   }
+    ///   for (int i = 0; i < 10; ++i) {
+    ///   }
+    /// \endcode
+    BWACS_Never,
+    /// Only wrap braces after a multi-line control statement.
+    /// \code
+    ///   if (foo && bar &&
+    ///       baz)
+    ///   {
+    ///     quux();
+    ///   }
+    ///   while (foo || bar) {
+    ///   }
+    /// \endcode
+    BWACS_MultiLine,
+    /// Always wrap braces after a control statement.
+    /// \code
+    ///   if (foo())
+    ///   {
+    ///   } else
+    ///   {}
+    ///   for (int i = 0; i < 10; ++i)
+    ///   {}
+    /// \endcode
+    BWACS_Always
+  };
+
   /// Precise control over the wrapping of braces.
   /// \code
   ///   # Should be declared this way:
   ///   BreakBeforeBraces: Custom
   ///   BraceWrapping:
   ///       AfterClass: true
   /// \endcode
   struct BraceWrappingFlags {
struct BraceWrappingFlags {
     ///   class foo {};
     ///
     ///   false:
     ///   class foo
     ///   {};
     /// \endcode
     bool AfterClass;
     /// Wrap control statements (``if``/``for``/``while``/``switch``/..).
-    /// \code
-    ///   true:
-    ///   if (foo())
-    ///   {
-    ///   } else
-    ///   {}
-    ///   for (int i = 0; i < 10; ++i)
-    ///   {}
-    ///
-    ///   false:
-    ///   if (foo()) {
-    ///   } else {
-    ///   }
-    ///   for (int i = 0; i < 10; ++i) {
-    ///   }
-    /// \endcode
-    bool AfterControlStatement;
+    BraceWrappingAfterControlStatementStyle AfterControlStatement;
     /// Wrap enum definitions.
     /// \code
     ///   true:
     ///   enum X : int
     ///   {
     ///     B
     ///   };
     ///

cfe/trunk/lib/Format/Format.cpp

static void enumeration(IO &IO, FormatStyle::BraceBreakingStyle &Value) {
     IO.enumCase(Value, "Whitesmiths", FormatStyle::BS_Whitesmiths);
     IO.enumCase(Value, "GNU", FormatStyle::BS_GNU);
     IO.enumCase(Value, "WebKit", FormatStyle::BS_WebKit);
     IO.enumCase(Value, "Custom", FormatStyle::BS_Custom);
   }
 };
 
 template <>
+struct ScalarEnumerationTraits<
+    FormatStyle::BraceWrappingAfterControlStatementStyle> {
+  static void
+  enumeration(IO &IO,
+              FormatStyle::BraceWrappingAfterControlStatementStyle &Value) {
+    IO.enumCase(Value, "false", FormatStyle::BWACS_Never);
+    IO.enumCase(Value, "true", FormatStyle::BWACS_Always);
+    IO.enumCase(Value, "Never", FormatStyle::BWACS_Never);
+    IO.enumCase(Value, "MultiLine", FormatStyle::BWACS_MultiLine);
+    IO.enumCase(Value, "Always", FormatStyle::BWACS_Always);
+  }
+};
+
+template <>
 struct ScalarEnumerationTraits<FormatStyle::BreakConstructorInitializersStyle> {
   static void
   enumeration(IO &IO, FormatStyle::BreakConstructorInitializersStyle &Value) {
     IO.enumCase(Value, "BeforeColon", FormatStyle::BCIS_BeforeColon);
     IO.enumCase(Value, "BeforeComma", FormatStyle::BCIS_BeforeComma);
     IO.enumCase(Value, "AfterColon", FormatStyle::BCIS_AfterColon);
   }
 };
std::string ParseErrorCategory::message(int EV) const {
   }
   llvm_unreachable("unexpected parse error");
 }
 
 static FormatStyle expandPresets(const FormatStyle &Style) {
   if (Style.BreakBeforeBraces == FormatStyle::BS_Custom)
     return Style;
   FormatStyle Expanded = Style;
-  Expanded.BraceWrapping = {false, false, false, false, false, false,
-                            false, false, false, false, false, false,
-                            false, true,  true,  true};
+  Expanded.BraceWrapping = {false, false, FormatStyle::BWACS_Never,
+                            false, false, false,
+                            false, false, false,
+                            false, false, false,
+                            false, true,  true,
+                            true};
   switch (Style.BreakBeforeBraces) {
   case FormatStyle::BS_Linux:
     Expanded.BraceWrapping.AfterClass = true;
     Expanded.BraceWrapping.AfterFunction = true;
     Expanded.BraceWrapping.AfterNamespace = true;
     break;
   case FormatStyle::BS_Mozilla:
     Expanded.BraceWrapping.AfterClass = true;
     Expanded.BraceWrapping.AfterEnum = true;
     Expanded.BraceWrapping.AfterFunction = true;
     Expanded.BraceWrapping.AfterStruct = true;
     Expanded.BraceWrapping.AfterUnion = true;
     Expanded.BraceWrapping.AfterExternBlock = true;
     Expanded.BraceWrapping.SplitEmptyFunction = true;
     Expanded.BraceWrapping.SplitEmptyRecord = false;
     break;
   case FormatStyle::BS_Stroustrup:
     Expanded.BraceWrapping.AfterFunction = true;
     Expanded.BraceWrapping.BeforeCatch = true;
     Expanded.BraceWrapping.BeforeElse = true;
     break;
   case FormatStyle::BS_Allman:
     Expanded.BraceWrapping.AfterCaseLabel = true;
     Expanded.BraceWrapping.AfterClass = true;
-    Expanded.BraceWrapping.AfterControlStatement = true;
+    Expanded.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
     Expanded.BraceWrapping.AfterEnum = true;
     Expanded.BraceWrapping.AfterFunction = true;
     Expanded.BraceWrapping.AfterNamespace = true;
     Expanded.BraceWrapping.AfterObjCDeclaration = true;
     Expanded.BraceWrapping.AfterStruct = true;
     Expanded.BraceWrapping.AfterUnion = true;
     Expanded.BraceWrapping.AfterExternBlock = true;
     Expanded.BraceWrapping.BeforeCatch = true;
     Expanded.BraceWrapping.BeforeElse = true;
     break;
   case FormatStyle::BS_Whitesmiths:
     Expanded.BraceWrapping.AfterCaseLabel = true;
     Expanded.BraceWrapping.AfterClass = true;
-    Expanded.BraceWrapping.AfterControlStatement = true;
+    Expanded.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
     Expanded.BraceWrapping.AfterEnum = true;
     Expanded.BraceWrapping.AfterFunction = true;
     Expanded.BraceWrapping.AfterNamespace = true;
     Expanded.BraceWrapping.AfterObjCDeclaration = true;
     Expanded.BraceWrapping.AfterStruct = true;
     Expanded.BraceWrapping.AfterExternBlock = true;
     Expanded.BraceWrapping.BeforeCatch = true;
     Expanded.BraceWrapping.BeforeElse = true;
     break;
   case FormatStyle::BS_GNU:
-    Expanded.BraceWrapping = {true, true, true, true, true, true, true, true,
-                              true, true, true, true, true, true, true, true};
+    Expanded.BraceWrapping = {true, true, FormatStyle::BWACS_Always,
+                              true, true, true,
+                              true, true, true,
+                              true, true, true,
+                              true, true, true,
+                              true};
     break;
   case FormatStyle::BS_WebKit:
     Expanded.BraceWrapping.AfterFunction = true;
     break;
   default:
     break;
   }
   return Expanded;
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_MultiLine;
   LLVMStyle.BinPackArguments = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
-  LLVMStyle.BraceWrapping = {false, false, false, false, false, false,
-                             false, false, false, false, false, false,
-                             false, true,  true,  true};
+  LLVMStyle.BraceWrapping = {false, false, FormatStyle::BWACS_Never,
+                             false, false, false,
+                             false, false, false,
+                             false, false, false,
+                             false, true,  true,
+                             true};
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
   LLVMStyle.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
   LLVMStyle.BreakStringLiterals = true;
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
   LLVMStyle.CompactNamespaces = false;
   LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;

 FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language) {
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 120;
   Style.TabWidth = 4;
   Style.IndentWidth = 4;
   Style.UseTab = FormatStyle::UT_Never;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterClass = true;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   Style.BraceWrapping.AfterEnum = true;
   Style.BraceWrapping.AfterFunction = true;
   Style.BraceWrapping.AfterNamespace = true;
   Style.BraceWrapping.AfterObjCDeclaration = true;
   Style.BraceWrapping.AfterStruct = true;
   Style.BraceWrapping.AfterExternBlock = true;
   Style.BraceWrapping.BeforeCatch = true;
   Style.BraceWrapping.BeforeElse = true;

cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp

tryFitMultipleLinesInOne(LevelIndentTracker &IndentTracker,
     if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
         TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
       return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
                  ? tryMergeSimpleBlock(I, E, Limit)
                  : 0;
     }
     // Try to merge a control statement block with left brace wrapped
     if (I[1]->First->is(tok::l_brace) &&
-        TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-      return Style.BraceWrapping.AfterControlStatement
+        (TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
+                                 tok::kw_switch, tok::kw_try, tok::kw_do) ||
+         (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
+          TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
+        Style.BraceWrapping.AfterControlStatement ==
+            FormatStyle::BWACS_MultiLine) {
+      // If possible, merge the next line's wrapped left brace with the current
+      // line. Otherwise, leave it on the next line, as this is a multi-line
+      // control statement.
+      return (Style.ColumnLimit == 0 ||
+              TheLine->Last->TotalLength <= Style.ColumnLimit)
+                 ? 1
+                 : 0;
+    } else if (I[1]->First->is(tok::l_brace) &&
+               TheLine->First->isOneOf(tok::kw_if, tok::kw_while,
+                                       tok::kw_for)) {
+      return (Style.BraceWrapping.AfterControlStatement ==
+              FormatStyle::BWACS_Always)
                  ? tryMergeSimpleBlock(I, E, Limit)
                  : 0;
     }
     // Try to merge either empty or one-line block if is precedeed by control
     // statement token
     if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
         I != AnnotatedLines.begin() &&
         I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
tryMergeSimplePPDirective(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
     return 1;
   }
 
   unsigned tryMergeSimpleControlStatement(
       SmallVectorImpl<AnnotatedLine *>::const_iterator I,
       SmallVectorImpl<AnnotatedLine *>::const_iterator E, unsigned Limit) {
     if (Limit == 0)
       return 0;
-    if (Style.BraceWrapping.AfterControlStatement &&
+    if (Style.BraceWrapping.AfterControlStatement ==
+            FormatStyle::BWACS_Always &&
         I[1]->First->is(tok::l_brace) &&
         Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
       return 0;
     if (I[1]->InPPDirective != (*I)->InPPDirective ||
         (I[1]->InPPDirective && I[1]->First->HasUnescapedNewline))
       return 0;
     Limit = limitConsideringMacros(I + 1, E, Limit);
     AnnotatedLine &Line = **I;
if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::kw_try,
       // the control statement block is empty
       if (!Style.AllowShortIfStatementsOnASingleLine &&
           Line.startsWith(tok::kw_if) &&
           !Style.BraceWrapping.AfterControlStatement &&
           !I[1]->First->is(tok::r_brace))
         return 0;
       if (!Style.AllowShortIfStatementsOnASingleLine &&
           Line.startsWith(tok::kw_if) &&
-          Style.BraceWrapping.AfterControlStatement && I + 2 != E &&
-          !I[2]->First->is(tok::r_brace))
+          Style.BraceWrapping.AfterControlStatement ==
+              FormatStyle::BWACS_Always &&
+          I + 2 != E && !I[2]->First->is(tok::r_brace))
         return 0;
       if (!Style.AllowShortLoopsOnASingleLine &&
           Line.First->isOneOf(tok::kw_while, tok::kw_do, tok::kw_for) &&
           !Style.BraceWrapping.AfterControlStatement &&
           !I[1]->First->is(tok::r_brace))
         return 0;
       if (!Style.AllowShortLoopsOnASingleLine &&
           Line.First->isOneOf(tok::kw_while, tok::kw_do, tok::kw_for) &&
-          Style.BraceWrapping.AfterControlStatement && I + 2 != E &&
-          !I[2]->First->is(tok::r_brace))
+          Style.BraceWrapping.AfterControlStatement ==
+              FormatStyle::BWACS_Always &&
+          I + 2 != E && !I[2]->First->is(tok::r_brace))
         return 0;
       // FIXME: Consider an option to allow short exception handling clauses on
       // a single line.
       // FIXME: This isn't covered by tests.
       // FIXME: For catch, __except, __finally the first token on the line
       // is '}', so this isn't correct here.
       if (Line.First->isOneOf(tok::kw_try, tok::kw___try, tok::kw_catch,
                               Keywords.kw___except, tok::kw___finally))
if (Line.Last->is(tok::l_brace)) {
         Tok = I[2]->First;
         if (Tok->isNot(tok::r_brace))
           return 0;
 
         // Don't merge "if (a) { .. } else {".
         if (Tok->Next && Tok->Next->is(tok::kw_else))
           return 0;
 
+        // Don't merge a trailing multi-line control statement block like:
+        // } else if (foo &&
+        //            bar)
+        // { <-- current Line
+        //   baz();
+        // }
+        if (Line.First == Line.Last &&
+            Style.BraceWrapping.AfterControlStatement ==
+                FormatStyle::BWACS_MultiLine)
+          return 0;
+
         return 2;
       }
     } else if (I[1]->First->is(tok::l_brace)) {
       if (I[1]->Last->is(TT_LineComment))
         return 0;
 
       // Check for Limit <= 2 to account for the " {".
       if (Limit <= 2 || (Style.ColumnLimit == 0 && containsMustBreak(*I)))

cfe/trunk/lib/Format/UnwrappedLineParser.cpp

case tok::at:
       case tok::objc_optional:
       case tok::objc_required:
         nextToken();
         addUnwrappedLine();
         return;
       case tok::objc_autoreleasepool:
         nextToken();
         if (FormatTok->Tok.is(tok::l_brace)) {
-          if (Style.BraceWrapping.AfterControlStatement)
+          if (Style.BraceWrapping.AfterControlStatement ==
+              FormatStyle::BWACS_Always)
             addUnwrappedLine();
           parseBlock(/*MustBeDeclaration=*/false);
         }
         addUnwrappedLine();
         return;
       case tok::objc_synchronized:
         nextToken();
         if (FormatTok->Tok.is(tok::l_paren))
           // Skip synchronization object
           parseParens();
         if (FormatTok->Tok.is(tok::l_brace)) {
-          if (Style.BraceWrapping.AfterControlStatement)
+          if (Style.BraceWrapping.AfterControlStatement ==
+              FormatStyle::BWACS_Always)
             addUnwrappedLine();
           parseBlock(/*MustBeDeclaration=*/false);
         }
         addUnwrappedLine();
         return;
       case tok::objc_try:
         // This branch isn't strictly necessary (the kw_try case below would
         // do this too after the tok::at is parsed above).  But be explicit.
void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
   if (LeftAlignLabel)
     Line->Level = 0;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
     CompoundStatementIndenter Indenter(this, Line->Level,
                                        Style.BraceWrapping.AfterCaseLabel,
                                        Style.BraceWrapping.IndentBraces);
     parseBlock(/*MustBeDeclaration=*/false);
     if (FormatTok->Tok.is(tok::kw_break)) {
-      if (Style.BraceWrapping.AfterControlStatement)
+      if (Style.BraceWrapping.AfterControlStatement ==
+          FormatStyle::BWACS_Always)
         addUnwrappedLine();
       parseStructuralElement();
     }
     addUnwrappedLine();
   } else {
     if (FormatTok->is(tok::semi))
       nextToken();
     addUnwrappedLine();

cfe/trunk/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, FormatShortBracedStatements) {
   verifyFormat("for (;;) {\n"
                "  f();\n"
                "}",
                AllowSimpleBracedStatements);
 
   AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
       FormatStyle::SIS_WithoutElse;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
-  AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
+  AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement =
+      FormatStyle::BWACS_Always;
 
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if constexpr (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if CONSTEXPR (true) {}", AllowSimpleBracedStatements);
   verifyFormat("while (true) {}", AllowSimpleBracedStatements);
   verifyFormat("for (;;) {}", AllowSimpleBracedStatements);
   verifyFormat("if (true) { f(); }", AllowSimpleBracedStatements);
   verifyFormat("if constexpr (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("switch (a) {\n"
                "}",
                getLLVMStyleWithColumns(34));
 
   FormatStyle Style = getLLVMStyle();
   Style.IndentCaseLabels = true;
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   EXPECT_EQ("switch (n)\n"
             "{\n"
             "  case 0:\n"
             "  {\n"
             "    return false;\n"
             "  }\n"
             "  default:\n"
             "  {\n"
EXPECT_EQ("switch (n) {\n"
                    "  return true;\n"
                    "case 0:\n"
                    "  return false;\n"
                    "}",
                    Style));
   Style.AllowShortCaseLabelsOnASingleLine = true;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   EXPECT_EQ("switch (n)\n"
             "{\n"
             "  case 0:\n"
             "  {\n"
             "    return false;\n"
             "  }\n"
             "  default:\n"
             "  {\n"
verifyFormat("{\n"
                Style);
   verifyFormat("{\n"
                "  some_code();\n"
                "test_label:;\n"
                "  int i = 0;\n"
                "}");
 }
 
+TEST_F(FormatTest, MultiLineControlStatements) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BraceBreakingStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_MultiLine;
+  Style.ColumnLimit = 20;
+  // Short lines should keep opening brace on same line.
+  EXPECT_EQ("if (foo) {\n"
+            "  bar();\n"
+            "}",
+            format("if(foo){bar();}", Style));
+  EXPECT_EQ("if (foo) {\n"
+            "  bar();\n"
+            "} else {\n"
+            "  baz();\n"
+            "}",
+            format("if(foo){bar();}else{baz();}", Style));
+  EXPECT_EQ("if (foo && bar) {\n"
+            "  baz();\n"
+            "}",
+            format("if(foo&&bar){baz();}", Style));
+  EXPECT_EQ("if (foo) {\n"
+            "  bar();\n"
+            "} else if (baz) {\n"
+            "  quux();\n"
+            "}",
+            format("if(foo){bar();}else if(baz){quux();}", Style));
+  EXPECT_EQ(
+      "if (foo) {\n"
+      "  bar();\n"
+      "} else if (baz) {\n"
+      "  quux();\n"
+      "} else {\n"
+      "  foobar();\n"
+      "}",
+      format("if(foo){bar();}else if(baz){quux();}else{foobar();}", Style));
+  EXPECT_EQ("for (;;) {\n"
+            "  foo();\n"
+            "}",
+            format("for(;;){foo();}"));
+  EXPECT_EQ("while (1) {\n"
+            "  foo();\n"
+            "}",
+            format("while(1){foo();}", Style));
+  EXPECT_EQ("switch (foo) {\n"
+            "case bar:\n"
+            "  return;\n"
+            "}",
+            format("switch(foo){case bar:return;}", Style));
+  EXPECT_EQ("try {\n"
+            "  foo();\n"
+            "} catch (...) {\n"
+            "  bar();\n"
+            "}",
+            format("try{foo();}catch(...){bar();}", Style));
+  EXPECT_EQ("do {\n"
+            "  foo();\n"
+            "} while (bar &&\n"
+            "         baz);",
+            format("do{foo();}while(bar&&baz);", Style));
+  // Long lines should put opening brace on new line.
+  EXPECT_EQ("if (foo && bar &&\n"
+            "    baz)\n"
+            "{\n"
+            "  quux();\n"
+            "}",
+            format("if(foo&&bar&&baz){quux();}", Style));
+  EXPECT_EQ("if (foo && bar &&\n"
+            "    baz)\n"
+            "{\n"
+            "  quux();\n"
+            "}",
+            format("if (foo && bar &&\n"
+                   "    baz) {\n"
+                   "  quux();\n"
+                   "}",
+                   Style));
+  EXPECT_EQ("if (foo) {\n"
+            "  bar();\n"
+            "} else if (baz ||\n"
+            "           quux)\n"
+            "{\n"
+            "  foobar();\n"
+            "}",
+            format("if(foo){bar();}else if(baz||quux){foobar();}", Style));
+  EXPECT_EQ(
+      "if (foo) {\n"
+      "  bar();\n"
+      "} else if (baz ||\n"
+      "           quux)\n"
+      "{\n"
+      "  foobar();\n"
+      "} else {\n"
+      "  barbaz();\n"
+      "}",
+      format("if(foo){bar();}else if(baz||quux){foobar();}else{barbaz();}",
+             Style));
+  EXPECT_EQ("for (int i = 0;\n"
+            "     i < 10; ++i)\n"
+            "{\n"
+            "  foo();\n"
+            "}",
+            format("for(int i=0;i<10;++i){foo();}", Style));
+  EXPECT_EQ("while (foo || bar ||\n"
+            "       baz)\n"
+            "{\n"
+            "  quux();\n"
+            "}",
+            format("while(foo||bar||baz){quux();}", Style));
+  EXPECT_EQ("switch (\n"
+            "    foo = barbaz)\n"
+            "{\n"
+            "case quux:\n"
+            "  return;\n"
+            "}",
+            format("switch(foo=barbaz){case quux:return;}", Style));
+  EXPECT_EQ("try {\n"
+            "  foo();\n"
+            "} catch (\n"
+            "    Exception &bar)\n"
+            "{\n"
+            "  baz();\n"
+            "}",
+            format("try{foo();}catch(Exception&bar){baz();}", Style));
+}
+
 //===----------------------------------------------------------------------===//
 // Tests for classes, namespaces, etc.
 //===----------------------------------------------------------------------===//
 
 TEST_F(FormatTest, DoesNotBreakSemiAfterClassDecl) {
   verifyFormat("class A {};");
 }
 
EXPECT_EQ("class A {\n"
             format("class A {\n"
                    "  A()\n : t(0) {}\n"
                    "  A(int i)\n noexcept() : {}\n"
                    "  A(X x)\n"
                    "  try : t(0) {} catch (...) {}\n"
                    "};"));
   FormatStyle Style = getLLVMStyle();
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   Style.BraceWrapping.AfterFunction = true;
   EXPECT_EQ("void f()\n"
             "try\n"
             "{\n"
             "}",
             format("void f() try {\n"
                    "}",
                    Style));
TEST_F(FormatTest, ParsesConfigurationBools) {
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
   CHECK_PARSE_BOOL(SpaceBeforeCpp11BracedList);
   CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
   CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
   CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
 
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterCaseLabel);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
-  CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterControlStatement);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterEnum);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterNamespace);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterObjCDeclaration);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterStruct);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterUnion);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterExternBlock);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
TEST_F(FormatTest, ParsesConfiguration) {
   CHECK_PARSE("BreakBeforeBraces: Whitesmiths", BreakBeforeBraces,
               FormatStyle::BS_Whitesmiths);
   CHECK_PARSE("BreakBeforeBraces: GNU", BreakBeforeBraces, FormatStyle::BS_GNU);
   CHECK_PARSE("BreakBeforeBraces: WebKit", BreakBeforeBraces,
               FormatStyle::BS_WebKit);
   CHECK_PARSE("BreakBeforeBraces: Custom", BreakBeforeBraces,
               FormatStyle::BS_Custom);
 
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
+  CHECK_PARSE("BraceWrapping:\n"
+              "  AfterControlStatement: MultiLine",
+              BraceWrapping.AfterControlStatement,
+              FormatStyle::BWACS_MultiLine);
+  CHECK_PARSE("BraceWrapping:\n"
+              "  AfterControlStatement: Always",
+              BraceWrapping.AfterControlStatement, FormatStyle::BWACS_Always);
+  CHECK_PARSE("BraceWrapping:\n"
+              "  AfterControlStatement: Never",
+              BraceWrapping.AfterControlStatement, FormatStyle::BWACS_Never);
+  // For backward compatibility:
+  CHECK_PARSE("BraceWrapping:\n"
+              "  AfterControlStatement: true",
+              BraceWrapping.AfterControlStatement, FormatStyle::BWACS_Always);
+  CHECK_PARSE("BraceWrapping:\n"
+              "  AfterControlStatement: false",
+              BraceWrapping.AfterControlStatement, FormatStyle::BWACS_Never);
+
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
   CHECK_PARSE("AlwaysBreakAfterReturnType: None", AlwaysBreakAfterReturnType,
               FormatStyle::RTBS_None);
   CHECK_PARSE("AlwaysBreakAfterReturnType: All", AlwaysBreakAfterReturnType,
               FormatStyle::RTBS_All);
   CHECK_PARSE("AlwaysBreakAfterReturnType: TopLevel",
               AlwaysBreakAfterReturnType, FormatStyle::RTBS_TopLevel);
   CHECK_PARSE("AlwaysBreakAfterReturnType: AllDefinitions",

cfe/trunk/unittests/Format/FormatTestObjC.cpp

 TEST_F(FormatTestObjC, FormatObjCAutoreleasepool) {
   verifyFormat("@autoreleasepool {\n"
                "  f();\n"
                "}\n"
                "@autoreleasepool {\n"
                "  f();\n"
                "}\n");
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   verifyFormat("@autoreleasepool\n"
                "{\n"
                "  f();\n"
                "}\n"
                "@autoreleasepool\n"
                "{\n"
                "  f();\n"
                "}\n");

 TEST_F(FormatTestObjC, FormatObjCSynchronized) {
   verifyFormat("@synchronized(self) {\n"
                "  f();\n"
                "}\n"
                "@synchronized(self) {\n"
                "  f();\n"
                "}\n");
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   verifyFormat("@synchronized(self)\n"
                "{\n"
                "  f();\n"
                "}\n"
                "@synchronized(self)\n"
                "{\n"
                "  f();\n"
                "}\n");