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

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();
}
```

Reviewers: sammccall, owenpan, reuk, MyDeveloperDay, klimek

Reviewed By: MyDeveloperDay

Subscribers: MyDeveloperDay, cfe-commits

Patch By: mitchell-stellar

Tags: #clang-format, #clang, #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D68296

llvm-svn: 373647
This commit is contained in:
Paul Hoad 2019-10-03 18:42:31 +00:00
parent b2b43c8576
commit fb13e65acf
7 changed files with 301 additions and 60 deletions

View File

@ -778,24 +778,46 @@ the configuration (without a prefix: ``Auto``).
class foo
{};
* ``bool AfterControlStatement`` Wrap control statements (``if``/``for``/``while``/``switch``/..).
* ``BraceWrappingAfterControlStatementStyle AfterControlStatement``
Wrap control statements (``if``/``for``/``while``/``switch``/..).
.. code-block:: c++
Possible values:
true:
if (foo())
{
} else
{}
for (int i = 0; i < 10; ++i)
{}
* ``BWACS_Never`` (in configuration: ``Never``)
Never wrap braces after a control statement.
false:
if (foo()) {
} else {
}
for (int i = 0; i < 10; ++i) {
}
.. 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++
if (foo())
{
} else
{}
for (int i = 0; i < 10; ++i)
{}
* ``bool AfterEnum`` Wrap enum definitions.

View File

@ -782,6 +782,40 @@ struct FormatStyle {
/// 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:
@ -817,23 +851,7 @@ struct FormatStyle {
/// \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:

View File

@ -181,6 +181,20 @@ template <> struct ScalarEnumerationTraits<FormatStyle::BraceBreakingStyle> {
}
};
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
@ -629,9 +643,12 @@ 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;
@ -656,7 +673,7 @@ static FormatStyle expandPresets(const FormatStyle &Style) {
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;
@ -670,7 +687,7 @@ static FormatStyle expandPresets(const FormatStyle &Style) {
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;
@ -681,8 +698,12 @@ static FormatStyle expandPresets(const FormatStyle &Style) {
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;
@ -722,9 +743,12 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
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;
@ -1067,7 +1091,7 @@ FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language) {
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;

View File

@ -306,8 +306,24 @@ private:
}
// 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;
}
@ -410,7 +426,8 @@ private:
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;
@ -523,8 +540,9 @@ private:
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) &&
@ -533,8 +551,9 @@ private:
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.
@ -597,6 +616,17 @@ private:
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)) {

View File

@ -1167,7 +1167,8 @@ void UnwrappedLineParser::parseStructuralElement() {
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);
}
@ -1179,7 +1180,8 @@ void UnwrappedLineParser::parseStructuralElement() {
// 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);
}
@ -1989,7 +1991,8 @@ void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
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();
}

View File

@ -644,7 +644,8 @@ TEST_F(FormatTest, FormatShortBracedStatements) {
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);
@ -1168,7 +1169,7 @@ TEST_F(FormatTest, FormatsSwitchStatement) {
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"
@ -1370,7 +1371,7 @@ TEST_F(FormatTest, ShortCaseLabels) {
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"
@ -1441,6 +1442,131 @@ TEST_F(FormatTest, FormatsLabels) {
"}");
}
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.
//===----------------------------------------------------------------------===//
@ -2940,7 +3066,7 @@ TEST_F(FormatTest, MacroCallsWithoutTrailingSemicolon) {
"};"));
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"
@ -12261,7 +12387,6 @@ TEST_F(FormatTest, ParsesConfigurationBools) {
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);
@ -12469,6 +12594,25 @@ TEST_F(FormatTest, ParsesConfiguration) {
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);

View File

@ -207,7 +207,7 @@ TEST_F(FormatTestObjC, FormatObjCAutoreleasepool) {
" f();\n"
"}\n");
Style.BreakBeforeBraces = FormatStyle::BS_Custom;
Style.BraceWrapping.AfterControlStatement = true;
Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
verifyFormat("@autoreleasepool\n"
"{\n"
" f();\n"
@ -237,7 +237,7 @@ TEST_F(FormatTestObjC, FormatObjCSynchronized) {
" 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"