Hi.
When indent is four spaces, "if" conditions spanning multiple lines
could be difficult to discern from the "if" body. Indentation is the
same, so they are visually blending. (See example below). One of the
solutions is to place opening brace on a separate line when "if"
condition spans multiple lines. Always putting it on a separate line
in undesired, as there are usually no so many "if"s with multiline
conditions.
To make that possible, proposed patch adds another setting,
BraceWrapping.AfterMultilineControlStatement which acts like
BraceWrapping.AfterControlStatement, but only if condition in control
statement spans over multiple lines.
I didn't see a way to detach brace and move it to the next line once
unwrapped lines are constructed. So when AfterMultilineControlStatement
is enabled, all opening braces go to a separate unwrapped line. Then,
if length of the line is not higher that column limit, code tries
to reattach brace back.
To make what I said above a bit clearer, here are examples:
if (0 + 1 + 2 + 3 + 4 + 5 +
6 + 7 + 8 + 9 + 10 + 11 +
12 + 13 + 14 + 15) {
some_code();
some_other_code();
}
This patch allows to change that to:
if (0 + 1 + 2 + 3 + 4 + 5 +
6 + 7 + 8 + 9 + 10 + 11 +
12 + 13 + 14 + 15)
{
some_code();
some_other_code();
}
while keeping opening brace for short conditions on the same line:
if (0) {
some_code();
}
---
Rinat---
docs/ClangFormatStyleOptions.rst | 1 +
include/clang/Format/Format.h | 2 +
lib/Format/Format.cpp | 7 +-
lib/Format/UnwrappedLineFormatter.cpp | 28 +++++-
lib/Format/UnwrappedLineParser.cpp | 14 +--
test/Format/multiline-control-statements.cpp | 123 +++++++++++++++++++++++++++
unittests/Format/FormatTest.cpp | 1 +
7 files changed, 167 insertions(+), 9 deletions(-)
create mode 100644 test/Format/multiline-control-statements.cpp
diff --git a/docs/ClangFormatStyleOptions.rst b/docs/ClangFormatStyleOptions.rst
index a548e83..e83f144 100644
--- a/docs/ClangFormatStyleOptions.rst
+++ b/docs/ClangFormatStyleOptions.rst
@@ -340,6 +340,7 @@ the configuration (without a prefix: ``Auto``).
* ``bool AfterControlStatement`` Wrap control statements (``if``/``for``/``while``/``switch``/..).
* ``bool AfterEnum`` Wrap enum definitions.
* ``bool AfterFunction`` Wrap function definitions.
+ * ``bool AfterMultilineControlStatement`` Wrap control statements that span over more than one line.
* ``bool AfterNamespace`` Wrap namespace definitions.
* ``bool AfterObjCDeclaration`` Wrap ObjC definitions (``@autoreleasepool``, interfaces, ..).
* ``bool AfterStruct`` Wrap struct definitions.
diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h
index 1ff305d..80434dc 100644
--- a/include/clang/Format/Format.h
+++ b/include/clang/Format/Format.h
@@ -257,6 +257,8 @@ struct FormatStyle {
bool AfterEnum;
/// \brief Wrap function definitions.
bool AfterFunction;
+ /// \bried Wrap control statements spanning over more than one line.
+ bool AfterMultilineControlStatement;
/// \brief Wrap namespace definitions.
bool AfterNamespace;
/// \brief Wrap ObjC definitions (``@autoreleasepool``, interfaces, ..).
diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index b0f64e2..3afacd2 100644
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -363,6 +363,8 @@ template <> struct MappingTraits<FormatStyle::BraceWrappingFlags> {
IO.mapOptional("AfterControlStatement", Wrapping.AfterControlStatement);
IO.mapOptional("AfterEnum", Wrapping.AfterEnum);
IO.mapOptional("AfterFunction", Wrapping.AfterFunction);
+ IO.mapOptional("AfterMultilineControlStatement",
+ Wrapping.AfterMultilineControlStatement);
IO.mapOptional("AfterNamespace", Wrapping.AfterNamespace);
IO.mapOptional("AfterObjCDeclaration", Wrapping.AfterObjCDeclaration);
IO.mapOptional("AfterStruct", Wrapping.AfterStruct);
@@ -440,7 +442,7 @@ static FormatStyle expandPresets(const FormatStyle &Style) {
return Style;
FormatStyle Expanded = Style;
Expanded.BraceWrapping = {false, false, false, false, false, false,
- false, false, false, false, false};
+ false, false, false, false, false, false};
switch (Style.BreakBeforeBraces) {
case FormatStyle::BS_Linux:
Expanded.BraceWrapping.AfterClass = true;
@@ -464,6 +466,7 @@ static FormatStyle expandPresets(const FormatStyle &Style) {
Expanded.BraceWrapping.AfterControlStatement = true;
Expanded.BraceWrapping.AfterEnum = true;
Expanded.BraceWrapping.AfterFunction = true;
+ Expanded.BraceWrapping.AfterMultilineControlStatement = true;
Expanded.BraceWrapping.AfterNamespace = true;
Expanded.BraceWrapping.AfterObjCDeclaration = true;
Expanded.BraceWrapping.AfterStruct = true;
@@ -472,7 +475,7 @@ static FormatStyle expandPresets(const FormatStyle &Style) {
break;
case FormatStyle::BS_GNU:
Expanded.BraceWrapping = {true, true, true, true, true, true,
- true, true, true, true, true};
+ true, true, true, true, true, true};
break;
case FormatStyle::BS_WebKit:
Expanded.BraceWrapping.AfterFunction = true;
diff --git a/lib/Format/UnwrappedLineFormatter.cpp b/lib/Format/UnwrappedLineFormatter.cpp
index 07bfe3e..068836a 100644
--- a/lib/Format/UnwrappedLineFormatter.cpp
+++ b/lib/Format/UnwrappedLineFormatter.cpp
@@ -224,6 +224,31 @@ private:
}
return MergedLines;
}
+
+ // try to reattach left brace to singleline control statement
+ if (!Style.BraceWrapping.AfterControlStatement &&
+ Style.BraceWrapping.AfterMultilineControlStatement &&
+ I[1]->First->is(tok::l_brace) &&
+ (Style.ColumnLimit == 0 ||
+ TheLine->Last->TotalLength + Indent < Style.ColumnLimit)) {
+ if (TheLine->First->isOneOf(tok::kw_if, tok::kw_for, tok::kw_while,
+ TT_ForEachMacro, tok::kw_switch, tok::kw_try,
+ tok::kw___try))
+ return 1;
+
+ // merge left brace in "else if (...) {"
+ if (TheLine->First->is(tok::kw_else) && TheLine->First->Next &&
+ TheLine->First->Next->is(tok::kw_if))
+ return 1;
+
+ // merge left brace in "} else if (...) {"
+ if (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
+ TheLine->First->Next->is(tok::kw_else) &&
+ TheLine->First->Next->Next &&
+ TheLine->First->Next->Next->is(tok::kw_if))
+ return 1;
+ }
+
if (TheLine->First->is(tok::kw_if)) {
return Style.AllowShortIfStatementsOnASingleLine
? tryMergeSimpleControlStatement(I, E, Limit)
@@ -264,7 +289,8 @@ private:
SmallVectorImpl<AnnotatedLine *>::const_iterator E, unsigned Limit) {
if (Limit == 0)
return 0;
- if (Style.BraceWrapping.AfterControlStatement &&
+ if ((Style.BraceWrapping.AfterControlStatement ||
+ Style.BraceWrapping.AfterMultilineControlStatement) &&
(I[1]->First->is(tok::l_brace) && !Style.AllowShortBlocksOnASingleLine))
return 0;
if (I[1]->InPPDirective != (*I)->InPPDirective ||
diff --git a/lib/Format/UnwrappedLineParser.cpp b/lib/Format/UnwrappedLineParser.cpp
index 9f79ba6..65f06ec 100644
--- a/lib/Format/UnwrappedLineParser.cpp
+++ b/lib/Format/UnwrappedLineParser.cpp
@@ -152,9 +152,11 @@ private:
class CompoundStatementIndenter {
public:
CompoundStatementIndenter(UnwrappedLineParser *Parser,
- const FormatStyle &Style, unsigned &LineLevel)
+ const FormatStyle &Style, unsigned &LineLevel,
+ bool multiline = false)
: LineLevel(LineLevel), OldLineLevel(LineLevel) {
- if (Style.BraceWrapping.AfterControlStatement)
+ if (Style.BraceWrapping.AfterControlStatement ||
+ (multiline && Style.BraceWrapping.AfterMultilineControlStatement))
Parser->addUnwrappedLine();
if (Style.BraceWrapping.IndentBraces)
++LineLevel;
@@ -1424,7 +1426,7 @@ void UnwrappedLineParser::parseIfThenElse() {
parseParens();
bool NeedsUnwrappedLine = false;
if (FormatTok->Tok.is(tok::l_brace)) {
- CompoundStatementIndenter Indenter(this, Style, Line->Level);
+ CompoundStatementIndenter Indenter(this, Style, Line->Level, true);
parseBlock(/*MustBeDeclaration=*/false);
if (Style.BraceWrapping.BeforeElse)
addUnwrappedLine();
@@ -1477,7 +1479,7 @@ void UnwrappedLineParser::parseTryCatch() {
parseParens();
}
if (FormatTok->is(tok::l_brace)) {
- CompoundStatementIndenter Indenter(this, Style, Line->Level);
+ CompoundStatementIndenter Indenter(this, Style, Line->Level, true);
parseBlock(/*MustBeDeclaration=*/false);
if (Style.BraceWrapping.BeforeCatch) {
addUnwrappedLine();
@@ -1582,7 +1584,7 @@ void UnwrappedLineParser::parseForOrWhileLoop() {
if (FormatTok->Tok.is(tok::l_paren))
parseParens();
if (FormatTok->Tok.is(tok::l_brace)) {
- CompoundStatementIndenter Indenter(this, Style, Line->Level);
+ CompoundStatementIndenter Indenter(this, Style, Line->Level, true);
parseBlock(/*MustBeDeclaration=*/false);
addUnwrappedLine();
} else {
@@ -1659,7 +1661,7 @@ void UnwrappedLineParser::parseSwitch() {
if (FormatTok->Tok.is(tok::l_paren))
parseParens();
if (FormatTok->Tok.is(tok::l_brace)) {
- CompoundStatementIndenter Indenter(this, Style, Line->Level);
+ CompoundStatementIndenter Indenter(this, Style, Line->Level, true);
parseBlock(/*MustBeDeclaration=*/false);
addUnwrappedLine();
} else {
diff --git a/test/Format/multiline-control-statements.cpp b/test/Format/multiline-control-statements.cpp
new file mode 100644
index 0000000..9d31e66
--- /dev/null
+++ b/test/Format/multiline-control-statements.cpp
@@ -0,0 +1,123 @@
+// RUN: grep -Ev "// *[A-Z0-9-]+:" %s | grep -Ev "^//$" \
+// RUN: | clang-format -style="{BasedOnStyle: LLVM, IndentWidth: 4, \
+// RUN: ColumnLimit: 40}" \
+// RUN: | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
+//
+// RUN: grep -Ev "// *[A-Z0-9-]+:" %s | grep -Ev "^//$" \
+// RUN: | clang-format -style="{BasedOnStyle: LLVM, IndentWidth: 4, \
+// RUN: ColumnLimit: 40, BreakBeforeBraces: Custom, \
+// RUN: BraceWrapping: { \
+// RUN: AfterMultilineControlStatement: true}}" \
+// RUN: | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
+//
+/**/
+if (short_condition) { some_code(1); some_code(2); }
+/**/
+if (long_condition + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8) { some_code(3); some_code(4); }
+/**/
+switch (short_condition) { case 1: break; }
+/**/
+switch (long_condition + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8) { case 1: break; }
+/**/
+while (short_condition) { i++; j++; }
+/**/
+while (long_condition + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8) { i++; j++; }
+/**/
+for (i = 0; short_condition; i ++) { j++; k++; }
+/**/
+for (i = 0; long_condition + 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8; i ++) { j++; k++; }
+// CHECK1: /**/
+// CHECK1-NEXT: if (short_condition) {
+// CHECK1-NEXT: some_code(1);
+// CHECK1-NEXT: some_code(2);
+// CHECK1-NEXT: }
+// CHECK1-NEXT: /**/
+// CHECK1-NEXT: if (long_condition + 1 + 2 + 3 + 4 + 5 +
+// CHECK1-NEXT: 6 + 7 + 8) {
+// CHECK1-NEXT: some_code(3);
+// CHECK1-NEXT: some_code(4);
+// CHECK1-NEXT: }
+// CHECK1-NEXT: /**/
+// CHECK1-NEXT: switch (short_condition) {
+// CHECK1-NEXT: case 1:
+// CHECK1-NEXT: break;
+// CHECK1-NEXT: }
+// CHECK1-NEXT: /**/
+// CHECK1-NEXT: switch (long_condition + 1 + 2 + 3 + 4 +
+// CHECK1-NEXT: 5 + 6 + 7 + 8) {
+// CHECK1-NEXT: case 1:
+// CHECK1-NEXT: break;
+// CHECK1-NEXT: }
+// CHECK1-NEXT: /**/
+// CHECK1-NEXT: while (short_condition) {
+// CHECK1-NEXT: i++;
+// CHECK1-NEXT: j++;
+// CHECK1-NEXT: }
+// CHECK1-NEXT: /**/
+// CHECK1-NEXT: while (long_condition + 1 + 2 + 3 + 4 +
+// CHECK1-NEXT: 5 + 6 + 7 + 8) {
+// CHECK1-NEXT: i++;
+// CHECK1-NEXT: j++;
+// CHECK1-NEXT: }
+// CHECK1-NEXT: /**/
+// CHECK1-NEXT: for (i = 0; short_condition; i++) {
+// CHECK1-NEXT: j++;
+// CHECK1-NEXT: k++;
+// CHECK1-NEXT: }
+// CHECK1-NEXT: /**/
+// CHECK1-NEXT: for (i = 0; long_condition + 1 + 2 + 3 +
+// CHECK1-NEXT: 4 + 5 + 6 + 7 + 8;
+// CHECK1-NEXT: i++) {
+// CHECK1-NEXT: j++;
+// CHECK1-NEXT: k++;
+// CHECK1-NEXT: }
+//
+// CHECK2: /**/
+// CHECK2-NEXT: if (short_condition) {
+// CHECK2-NEXT: some_code(1);
+// CHECK2-NEXT: some_code(2);
+// CHECK2-NEXT: }
+// CHECK2-NEXT: /**/
+// CHECK2-NEXT: if (long_condition + 1 + 2 + 3 + 4 + 5 +
+// CHECK2-NEXT: 6 + 7 + 8)
+// CHECK2-NEXT: {
+// CHECK2-NEXT: some_code(3);
+// CHECK2-NEXT: some_code(4);
+// CHECK2-NEXT: }
+// CHECK2-NEXT: /**/
+// CHECK2-NEXT: switch (short_condition) {
+// CHECK2-NEXT: case 1:
+// CHECK2-NEXT: break;
+// CHECK2-NEXT: }
+// CHECK2-NEXT: /**/
+// CHECK2-NEXT: switch (long_condition + 1 + 2 + 3 + 4 +
+// CHECK2-NEXT: 5 + 6 + 7 + 8)
+// CHECK2-NEXT: {
+// CHECK2-NEXT: case 1:
+// CHECK2-NEXT: break;
+// CHECK2-NEXT: }
+// CHECK2-NEXT: /**/
+// CHECK2-NEXT: while (short_condition) {
+// CHECK2-NEXT: i++;
+// CHECK2-NEXT: j++;
+// CHECK2-NEXT: }
+// CHECK2-NEXT: /**/
+// CHECK2-NEXT: while (long_condition + 1 + 2 + 3 + 4 +
+// CHECK2-NEXT: 5 + 6 + 7 + 8)
+// CHECK2-NEXT: {
+// CHECK2-NEXT: i++;
+// CHECK2-NEXT: j++;
+// CHECK2-NEXT: }
+// CHECK2-NEXT: /**/
+// CHECK2-NEXT: for (i = 0; short_condition; i++) {
+// CHECK2-NEXT: j++;
+// CHECK2-NEXT: k++;
+// CHECK2-NEXT: }
+// CHECK2-NEXT: /**/
+// CHECK2-NEXT: for (i = 0; long_condition + 1 + 2 + 3 +
+// CHECK2-NEXT: 4 + 5 + 6 + 7 + 8;
+// CHECK2-NEXT: i++)
+// CHECK2-NEXT: {
+// CHECK2-NEXT: j++;
+// CHECK2-NEXT: k++;
+// CHECK2-NEXT: }
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index f2b5bd6..ade55ce 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -10197,6 +10197,7 @@ TEST_F(FormatTest, ParsesConfigurationBools) {
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterControlStatement);
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterEnum);
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterFunction);
+ CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterMultilineControlStatement);
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterNamespace);
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterObjCDeclaration);
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterStruct);
--
2.8.1
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
