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

[Mitchell Balan via cfe-commits]

Author: Mitchell Balan
Date: 2020-01-06T09:21:41-05:00
New Revision: d45aafa2fbcf66f3dafdc7c5e0a0ce3709914cbc

URL: https://github.com/llvm/llvm-project/commit/d45aafa2fbcf66f3dafdc7c5e0a0ce3709914cbc
DIFF: https://github.com/llvm/llvm-project/commit/d45aafa2fbcf66f3dafdc7c5e0a0ce3709914cbc.diff

LOG: [clang-format] fix conflict between FormatStyle::BWACS_MultiLine and 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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 8b8d357d9cbe..fec85f1174da 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -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 
diff erent 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

diff  --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 069542683c0d..2d67b9759d7f 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -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));
 }
 
 //===----------------------------------------------------------------------===//
@@ -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);