[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2020-01-06 Thread Mitchell via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd45aafa2fbcf: [clang-format] fix conflict between 
FormatStyle::BWACS_MultiLine and… (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
 "  baz();\n"
 "}",
 format("try{foo();}catch(Exception){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 ) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(Exception){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));
 }
 
 
//===--===//
@@ -14881,7 +14916,7 @@
   verifyFormat("auto lambda = [ = 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);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -326,6 +326,19 @@
   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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
 "  baz();\n"
 "}",
 format("try{foo();}catch(Exception){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 ) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(Exception){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));
 }
 
 //===--===//
@@ -14881,7 +14916,7 @@
   verifyFormat("auto lambda = [ = a]() { a = 2; };", AlignStyle);
 }
 
-

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2020-01-04 Thread Pavlo Shkrabliuk via Phabricator via cfe-commits 
pastey added a comment.

Guys, sorry for a dumb question – so the fix can be pushed now? If yes, then 
there's one issue – I don't think I have write access to push the fix – may I 
ask one of you to push it?

ERROR: Permission to llvm/llvm-project.git denied to pastey12.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-30 Thread Pavlo Shkrabliuk via Phabricator via cfe-commits 
pastey marked an inline comment as done.
pastey added a comment.

@MyDeveloperDay, thanks for looking into this. Added a comment


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-30 Thread Pavlo Shkrabliuk via Phabricator via cfe-commits 
pastey updated this revision to Diff 235621.

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
 "  baz();\n"
 "}",
 format("try{foo();}catch(Exception){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 ) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(Exception){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));
 }
 
 
//===--===//
@@ -14881,7 +14916,7 @@
   verifyFormat("auto lambda = [ = 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);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -326,6 +326,19 @@
   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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
 "  baz();\n"
 "}",
 format("try{foo();}catch(Exception){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 ) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(Exception){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));
 }
 
 //===--===//
@@ -14881,7 +14916,7 @@
   verifyFormat("auto lambda = [ = a]() { a = 2; };", AlignStyle);
 }
 
- TEST_F(FormatTest, SpacesInConditionalStatement) {
+TEST_F(FormatTest, SpacesInConditionalStatement) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInConditionalStatement =

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-30 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

I think generally speaking this looks ok, I'm ok with the extra clause, as long 
as it's clear what it's doing




Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:329
  : 0;
+} else if (I[1]->First->is(tok::l_brace) &&
+   TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&

nit: maybe leave a comment explaining the need for the extra clause


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-28 Thread Pavlo Shkrabliuk via Phabricator via cfe-commits 
pastey added a comment.

Thanks for your feedback. Let's see what other reviewers say.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-28 Thread Pablo Martin-Gomez via Phabricator via cfe-commits 
Bouska added a comment.

I'm not sure creating a special case for else/catch is the best idea. I would 
seem better to change the control statement @ line 309 to add it the case where 
we break before else or catch (add a new || test in the middle condition). I'm 
really new to this code too, so I don't know if it's better to check that 
previous line as a right curly brace and that BeforeElse/Catch is true and that 
first token of the line is else or catch or if just checking that the first 
token of the line is else or catch (or a mix in between). I'll let a more 
senior reviewer advise on this.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-28 Thread Pavlo Shkrabliuk via Phabricator via cfe-commits 
pastey updated this revision to Diff 235466.
pastey added a comment.

Bouska, thank you! Don't know how I missed that failing test. Never commit on 
friday :)

However, seeing this test result in a browser window gave me that "a-ha". I 
understood that I had wrong perception of what MultiLine feature is all about. 
As I wrote before, I would expect it to format code like this:

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

In my head it didn't have anything to do with ColumnLimit – it was all about 
more than one line in 'if' condition expression.

After I understood that MultiLine is bound to ColumnLimit, I could understand 
what was going on @ line 308 on UnwrappedLineFormatter.cpp.

Also found that UnwrappedLine actually wraps every line, which is ... confusing.

Uploaded the new diff. Removed my previous "fix" from UnwrappedLineParser.cpp. 
Added special case for else/catch in UnwrappedLineFormatter.cpp. Tests pass.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
 "  baz();\n"
 "}",
 format("try{foo();}catch(Exception){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 ) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(Exception){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));
 }
 
 
//===--===//
@@ -14881,7 +14916,7 @@
   verifyFormat("auto lambda = [ = 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);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -326,6 +326,14 @@
   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) {
+  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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
 "  baz();\n"
 "}",
 format("try{foo();}catch(Exception){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 ) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(Exception){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));
+
+

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-27 Thread Pablo Martin-Gomez via Phabricator via cfe-commits 
Bouska requested changes to this revision.
Bouska added a comment.
This revision now requires changes to proceed.

So, you are trying to fix the issue at the wrong place. Contrary from what we 
should expect from a UnwrappedLine, BWACS_Multiline works by always wrapping 
the brace in UnwrappedLineParser, and afterwards in UnwrappedLineFormatter 
merging it back to the control statement if it's not in multi-line. The bug is 
on the control statement @ line 308 on UnwrappedLineFormatter.cpp 
.
 That's the block that merge the brace back to its control statement if it is 
not a multiline and it's executed only with a line with a else or a catch that 
begins with right curly brace.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-27 Thread Pablo Martin-Gomez via Phabricator via cfe-commits 
Bouska added a comment.

Your code still breaks MultiLine

  [ RUN  ] FormatTest.MultiLineControlStatements
  /home/pablomg/dev/llvm-project/clang/unittests/Format/FormatTest.cpp:1562: 
Failure
Expected: "try {\n" "  foo();\n" "} catch (\n" "Exception )\n" 
"{\n" "  baz();\n" "}"
Which is: "try {\n  foo();\n} catch (\nException )\n{\n  
baz();\n}"
  To be equal to: format("try{foo();}catch(Exception){baz();}", Style)
Which is: "try {\n  foo();\n} catch (Exception\n ) {\n  
baz();\n}"
  With diff:
  @@ -1,7 +1,6 @@
   try {
 foo();
  -} catch (
  -Exception )
  -{
  +} catch (Exception
  + ) {
 baz();
   }
  
  [  FAILED  ] FormatTest.MultiLineControlStatements (28 ms)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

2019-12-27 Thread Pavlo Shkrabliuk via Phabricator via cfe-commits 
pastey created this revision.
pastey added a reviewer: MyDeveloperDay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Hi,

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

}

I added two tests that show this issue. One for if/else and one for try/catch 
(which is affected in the same way as if/else).

One more test with ColumnLimit set to 40 is to check that a suggested fix 
doesn't break existing cases. This test is the copy of an existing test from 
the same case, but it removes line wrap from the stage.

I suggest the fix, but I'm new to this code, so maybe you could suggest 
something better.

As far as I understood, the problem is caused by the following code:

  CompoundStatementIndenter(UnwrappedLineParser *Parser,
const FormatStyle , unsigned )
  : CompoundStatementIndenter(Parser, LineLevel,
  Style.BraceWrapping.AfterControlStatement,  
// <- here
  Style.BraceWrapping.IndentBraces) {}
  CompoundStatementIndenter(UnwrappedLineParser *Parser, unsigned ,
bool WrapBrace, bool IndentBrace)
  : LineLevel(LineLevel), OldLineLevel(LineLevel) {
if (WrapBrace)
// <- and here
  Parser->addUnwrappedLine();
if (IndentBrace)
  ++LineLevel;
  }

Style.BraceWrapping.AfterControlStatement used to be boolean, now it's an enum. 
MultiLine and Always both turn into true, thus MultiLine leads to a call of 
addUnwrappedLine().
I tried to place Style.BraceWrapping.AfterControlStatement == 
FormatStyle::BWACS_Always right in the CompoundStatementIndenter constructor, 
but that breaks MultiLine.

As I said, I'm new to this code, so maybe my fix is not at the right place, so 
I would be glad if we find a better fix.


Repository:
  rC Clang

https://reviews.llvm.org/D71939

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
 "  baz();\n"
 "}",
 format("try{foo();}catch(Exception){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 ) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(Exception){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));
 }
 
 
//===--===//
@@ -14881,7 +14916,7 @@
   verifyFormat("auto lambda = [ = 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);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1785,7 +1785,11 @@
   if (FormatTok->Tok.is(tok::kw_else)) {
 nextToken();
 if (FormatTok->Tok.is(tok::l_brace)) {
-  CompoundStatementIndenter Indenter(this, Style, Line->Level);
+  CompoundStatementIndenter Indenter(
+  this, Line->Level,
+  Style.BraceWrapping.AfterControlStatement ==
+  FormatStyle::BWACS_Always,
+  Style.BraceWrapping.IndentBraces);
   parseBlock(/*MustBeDeclaration=*/false);