mitchell-stellar created this revision.
mitchell-stellar added reviewers: sammccall, owenpan, reuk.
mitchell-stellar added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
- "OnlyMultiLine": 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 "OnlyMultiLine" 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();
}
Repository:
rC Clang
https://reviews.llvm.org/D68296
Files:
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/FormatTestObjC.cpp
Index: clang/unittests/Format/FormatTestObjC.cpp
===================================================================
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -207,7 +207,7 @@
" 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 @@
" 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"
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -644,7 +644,8 @@
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 @@
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 @@
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,126 @@
"}");
}
+TEST_F(FormatTest, MultiLineControlStatements) {
+ FormatStyle Style = getLLVMStyle();
+ Style.BreakBeforeBraces = FormatStyle::BraceBreakingStyle::BS_Custom;
+ Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_OnlyMultiLine;
+ 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));
+ // 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 +3061,7 @@
"};"));
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"
@@ -12248,7 +12369,6 @@
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);
@@ -12449,6 +12569,25 @@
CHECK_PARSE("BreakBeforeBraces: Custom", BreakBeforeBraces,
FormatStyle::BS_Custom);
+ Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
+ CHECK_PARSE("BraceWrapping:\n"
+ " AfterControlStatement: OnlyMultiLine",
+ BraceWrapping.AfterControlStatement,
+ FormatStyle::BWACS_OnlyMultiLine);
+ 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);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1167,7 +1167,8 @@
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 @@
// 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 @@
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();
}
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -306,8 +306,24 @@
}
// 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) ||
+ (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
+ TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
+ Style.BraceWrapping.AfterControlStatement ==
+ FormatStyle::BWACS_OnlyMultiLine) {
+ // 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 @@
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 @@
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 @@
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 @@
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_OnlyMultiLine)
+ return 0;
+
return 2;
}
} else if (I[1]->First->is(tok::l_brace)) {
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -172,6 +172,20 @@
}
};
+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, "OnlyMultiLine", FormatStyle::BWACS_OnlyMultiLine);
+ IO.enumCase(Value, "Always", FormatStyle::BWACS_Always);
+ }
+};
+
template <>
struct ScalarEnumerationTraits<FormatStyle::BreakConstructorInitializersStyle> {
static void
@@ -620,9 +634,12 @@
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;
@@ -647,7 +664,7 @@
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;
@@ -661,7 +678,7 @@
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;
@@ -672,8 +689,12 @@
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;
@@ -713,9 +734,12 @@
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;
@@ -1058,7 +1082,7 @@
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;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -782,6 +782,40 @@
/// 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_OnlyMultiLine,
+ /// 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 @@
/// \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:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
