[PATCH] D52021: Fix Bug 38926: don't merge short case labels if followed by a block

[Owen Pan via Phabricator via cfe-commits]

owenpan abandoned this revision.
owenpan added a comment.

Closed by r342708.


Repository:
  rC Clang

https://reviews.llvm.org/D52021



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

[PATCH] D52021: Fix Bug 38926: don't merge short case labels if followed by a block

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

This fix has become obvious, but I still prefer that it gets reviewed before I 
commit it.


Repository:
  rC Clang

https://reviews.llvm.org/D52021



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

[PATCH] D52021: Fix Bug 38926: don't merge short case labels if followed by a block

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 166216.
owenpan added a comment.

Simplified the fix and improved the test case.


Repository:
  rC Clang

https://reviews.llvm.org/D52021

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1241,6 +1241,30 @@
"  return false;\n"
"}",
Style));
+  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default:\n"
+   "  {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -428,6 +428,8 @@
 if (Limit == 0 || I + 1 == E ||
 I[1]->First->isOneOf(tok::kw_case, tok::kw_default))
   return 0;
+if (I[0]->Last->is(tok::l_brace) || I[1]->First->is(tok::l_brace))
+  return 0;
 unsigned NumStmts = 0;
 unsigned Length = 0;
 bool EndsWithComment = false;


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1241,6 +1241,30 @@
"  return false;\n"
"}",
Style));
+  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default:\n"
+   "  {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -428,6 +428,8 @@
 if (Limit == 0 || I + 1 == E ||
 I[1]->First->isOneOf(tok::kw_case, tok::kw_default))
   return 0;
+if (I[0]->Last->is(tok::l_brace) || I[1]->First->is(tok::l_brace))
+  return 0;
 unsigned NumStmts = 0;
 unsigned Length = 0;
 bool EndsWithComment = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52021: Fix Bug 38926: don't merge short case labels if followed by a block

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

ping any review.


Repository:
  rC Clang

https://reviews.llvm.org/D52021



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

[PATCH] D52021: Fix Bug 38926: don't merge short case labels if followed by a block

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: djasper, klimek, sammccall, krasimir.
Herald added a subscriber: cfe-commits.

https://bugs.llvm.org/show_bug.cgi?id=38926


Repository:
  rC Clang

https://reviews.llvm.org/D52021

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1241,6 +1241,29 @@
"  return false;\n"
"}",
Style));
+  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default: {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -428,6 +428,14 @@
 if (Limit == 0 || I + 1 == E ||
 I[1]->First->isOneOf(tok::kw_case, tok::kw_default))
   return 0;
+
+// Don't merge if a block follows the case label
+FormatToken *Tok = I[0]->Last;
+if (Tok->is(tok::comment))
+  Tok = Tok->getPreviousNonComment();
+if ((Tok && Tok->is(tok::l_brace)) || I[1]->First->is(tok::l_brace))
+  return 0;
+
 unsigned NumStmts = 0;
 unsigned Length = 0;
 bool EndsWithComment = false;


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1241,6 +1241,29 @@
"  return false;\n"
"}",
Style));
+  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default: {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -428,6 +428,14 @@
 if (Limit == 0 || I + 1 == E ||
 I[1]->First->isOneOf(tok::kw_case, tok::kw_default))
   return 0;
+
+// Don't merge if a block follows the case label
+FormatToken *Tok = I[0]->Last;
+if (Tok->is(tok::comment))
+  Tok = Tok->getPreviousNonComment();
+if ((Tok && Tok->is(tok::l_brace)) || I[1]->First->is(tok::l_brace))
+  return 0;
+
 unsigned NumStmts = 0;
 unsigned Length = 0;
 bool EndsWithComment = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits