Skip to content

Commit

Permalink
[clang-format] fix conflict between FormatStyle::BWACS_MultiLine and …
Browse files Browse the repository at this point in the history
…BeforeCatch/BeforeElse

Summary:
Found a bug introduced with BraceWrappingFlags AfterControlStatement MultiLine. This feature conflicts with the existing BeforeCatch and BeforeElse flags.

For example, our team uses BeforeElse.

if (foo ||
    bar) {
  doSomething();
}
else {
  doSomethingElse();
}

If we enable MultiLine (which we'd really love to do) we expect it to work like this:

if (foo ||
    bar)
{
  doSomething();
}
else {
  doSomethingElse();
}

What we actually get is:

if (foo ||
    bar)
{
  doSomething();
}
else
{
  doSomethingElse();
}

Reviewers: MyDeveloperDay, Bouska, mitchell-stellar

Patch by: pastey

Subscribers: Bouska, cfe-commits

Tags: clang

Differential Revision: https://reviews.llvm.org/D71939
  • Loading branch information
Mitchell Balan committed Jan 6, 2020
1 parent de73524 commit d45aafa
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
13 changes: 13 additions & 0 deletions clang/lib/Format/UnwrappedLineFormatter.cpp
Expand Up @@ -326,6 +326,19 @@ class LineJoiner {
FormatStyle::BWACS_Always)
? tryMergeSimpleBlock(I, E, Limit)
: 0;
} else if (I[1]->First->is(tok::l_brace) &&
TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&
Style.BraceWrapping.AfterControlStatement ==
FormatStyle::BWACS_MultiLine) {
// This case if different from the upper BWACS_MultiLine processing
// in that a preceding r_brace is not on the same line as else/catch
// most likely because of BeforeElse/BeforeCatch set to true.
// If the line length doesn't fit ColumnLimit, leave l_brace on the
// next line to respect the BWACS_MultiLine.
return (Style.ColumnLimit == 0 ||
TheLine->Last->TotalLength <= Style.ColumnLimit)
? 1
: 0;
}
// Try to merge either empty or one-line block if is precedeed by control
// statement token
Expand Down
37 changes: 36 additions & 1 deletion clang/unittests/Format/FormatTest.cpp
Expand Up @@ -1565,6 +1565,41 @@ TEST_F(FormatTest, MultiLineControlStatements) {
" baz();\n"
"}",
format("try{foo();}catch(Exception&bar){baz();}", Style));
Style.ColumnLimit =
40; // to concentrate at brace wrapping, not line wrap due to column limit
EXPECT_EQ("try {\n"
" foo();\n"
"} catch (Exception &bar) {\n"
" baz();\n"
"}",
format("try{foo();}catch(Exception&bar){baz();}", Style));
Style.ColumnLimit =
20; // to concentrate at brace wrapping, not line wrap due to column limit

Style.BraceWrapping.BeforeElse = true;
EXPECT_EQ(
"if (foo) {\n"
" bar();\n"
"}\n"
"else if (baz ||\n"
" quux)\n"
"{\n"
" foobar();\n"
"}\n"
"else {\n"
" barbaz();\n"
"}",
format("if(foo){bar();}else if(baz||quux){foobar();}else{barbaz();}",
Style));

Style.BraceWrapping.BeforeCatch = true;
EXPECT_EQ("try {\n"
" foo();\n"
"}\n"
"catch (...) {\n"
" baz();\n"
"}",
format("try{foo();}catch(...){baz();}", Style));
}

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -14881,7 +14916,7 @@ TEST_F(FormatTest, AmbersandInLamda) {
verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
}

TEST_F(FormatTest, SpacesInConditionalStatement) {
TEST_F(FormatTest, SpacesInConditionalStatement) {
FormatStyle Spaces = getLLVMStyle();
Spaces.SpacesInConditionalStatement = true;
verifyFormat("for ( int i = 0; i; i++ )\n continue;", Spaces);
Expand Down

0 comments on commit d45aafa

Please sign in to comment.