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

[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&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 @@
   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);
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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71939.235466.patch
Type: text/x-patch
Size: 2665 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191228/2d2212a8/attachment.bin>