[PATCH] D42298: [clang-format] Fix shortening blocks in macros causing merged next line

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL322954: [clang-format] Fix shortening blocks in macros 
causing merged next line (authored by krasimir, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D42298

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


Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -304,9 +304,15 @@
 if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
 I != AnnotatedLines.begin() &&
 I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  return Style.AllowShortBlocksOnASingleLine
- ? tryMergeSimpleBlock(I - 1, E, Limit)
- : 0;
+  unsigned MergedLines = 0;
+  if (Style.AllowShortBlocksOnASingleLine) {
+MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
+// If we managed to merge the block, discard the first merged line
+// since we are merging starting from I.
+if (MergedLines > 0)
+  --MergedLines;
+  }
+  return MergedLines;
 }
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -588,6 +588,23 @@
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  EXPECT_EQ("#define A  \\\n"
+"  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
+"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"X;",
+format("#define A \\\n"
+   "   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
+   "  RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; \\\n"
+   "   }\n"
+   "X;",
+   Style));
+}
+
 TEST_F(FormatTest, ParseIfElse) {
   verifyFormat("if (true)\n"
"  if (true)\n"


Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -304,9 +304,15 @@
 if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
 I != AnnotatedLines.begin() &&
 I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  return Style.AllowShortBlocksOnASingleLine
- ? tryMergeSimpleBlock(I - 1, E, Limit)
- : 0;
+  unsigned MergedLines = 0;
+  if (Style.AllowShortBlocksOnASingleLine) {
+MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
+// If we managed to merge the block, discard the first merged line
+// since we are merging starting from I.
+if (MergedLines > 0)
+  --MergedLines;
+  }
+  return MergedLines;
 }
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -588,6 +588,23 @@
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  EXPECT_EQ("#define A  \\\n"
+"  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
+"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"X;",
+format("#define A \\\n"
+   "   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
+   "  RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; \\\n"
+   "   }\n"
+   "X;",
+   Style));
+}
+
 TEST_F(FormatTest, ParseIfElse) {
   verifyFormat("if (true)\n"
"  if (true)\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D42298: [clang-format] Fix shortening blocks in macros causing merged next line

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer accepted this revision.
bkramer added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: lib/Format/UnwrappedLineFormatter.cpp:310
+MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
+// If we managed to merge the block, discard the first  merged line
+// since we are merging starting from I.

double space in comment


Repository:
  rC Clang

https://reviews.llvm.org/D42298



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

[PATCH] D42298: [clang-format] Fix shortening blocks in macros causing merged next line

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 130609.
krasimir added a comment.

- Remove double whitespace


Repository:
  rC Clang

https://reviews.llvm.org/D42298

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -588,6 +588,23 @@
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  EXPECT_EQ("#define A  \\\n"
+"  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
+"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"X;",
+format("#define A \\\n"
+   "   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
+   "  RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; \\\n"
+   "   }\n"
+   "X;",
+   Style));
+}
+
 TEST_F(FormatTest, ParseIfElse) {
   verifyFormat("if (true)\n"
"  if (true)\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -304,9 +304,15 @@
 if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
 I != AnnotatedLines.begin() &&
 I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  return Style.AllowShortBlocksOnASingleLine
- ? tryMergeSimpleBlock(I - 1, E, Limit)
- : 0;
+  unsigned MergedLines = 0;
+  if (Style.AllowShortBlocksOnASingleLine) {
+MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
+// If we managed to merge the block, discard the first merged line
+// since we are merging starting from I.
+if (MergedLines > 0)
+  --MergedLines;
+  }
+  return MergedLines;
 }
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -588,6 +588,23 @@
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  EXPECT_EQ("#define A  \\\n"
+"  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
+"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"X;",
+format("#define A \\\n"
+   "   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
+   "  RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; \\\n"
+   "   }\n"
+   "X;",
+   Style));
+}
+
 TEST_F(FormatTest, ParseIfElse) {
   verifyFormat("if (true)\n"
"  if (true)\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -304,9 +304,15 @@
 if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
 I != AnnotatedLines.begin() &&
 I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  return Style.AllowShortBlocksOnASingleLine
- ? tryMergeSimpleBlock(I - 1, E, Limit)
- : 0;
+  unsigned MergedLines = 0;
+  if (Style.AllowShortBlocksOnASingleLine) {
+MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
+// If we managed to merge the block, discard the first merged line
+// since we are merging starting from I.
+if (MergedLines > 0)
+  --MergedLines;
+  }
+  return MergedLines;
 }
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42298: [clang-format] Fix shortening blocks in macros causing merged next line

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir updated this revision to Diff 130604.
krasimir added a comment.

- Remove added newline


Repository:
  rC Clang

https://reviews.llvm.org/D42298

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -588,6 +588,23 @@
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  EXPECT_EQ("#define A  \\\n"
+"  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
+"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"X;",
+format("#define A \\\n"
+   "   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
+   "  RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; \\\n"
+   "   }\n"
+   "X;",
+   Style));
+}
+
 TEST_F(FormatTest, ParseIfElse) {
   verifyFormat("if (true)\n"
"  if (true)\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -304,9 +304,15 @@
 if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
 I != AnnotatedLines.begin() &&
 I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  return Style.AllowShortBlocksOnASingleLine
- ? tryMergeSimpleBlock(I - 1, E, Limit)
- : 0;
+  unsigned MergedLines = 0;
+  if (Style.AllowShortBlocksOnASingleLine) {
+MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
+// If we managed to merge the block, discard the first  merged line
+// since we are merging starting from I.
+if (MergedLines > 0)
+  --MergedLines;
+  }
+  return MergedLines;
 }
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -588,6 +588,23 @@
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  EXPECT_EQ("#define A  \\\n"
+"  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
+"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"X;",
+format("#define A \\\n"
+   "   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
+   "  RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; \\\n"
+   "   }\n"
+   "X;",
+   Style));
+}
+
 TEST_F(FormatTest, ParseIfElse) {
   verifyFormat("if (true)\n"
"  if (true)\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -304,9 +304,15 @@
 if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
 I != AnnotatedLines.begin() &&
 I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  return Style.AllowShortBlocksOnASingleLine
- ? tryMergeSimpleBlock(I - 1, E, Limit)
- : 0;
+  unsigned MergedLines = 0;
+  if (Style.AllowShortBlocksOnASingleLine) {
+MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
+// If we managed to merge the block, discard the first  merged line
+// since we are merging starting from I.
+if (MergedLines > 0)
+  --MergedLines;
+  }
+  return MergedLines;
 }
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42298: [clang-format] Fix shortening blocks in macros causing merged next line

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
krasimir added a reviewer: bkramer.
Herald added a subscriber: klimek.

This patch addresses bug 36002, where a combination of options causes the line
following a short block in macro to be merged with that macro.


Repository:
  rC Clang

https://reviews.llvm.org/D42298

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -495,6 +495,7 @@
"};",
AllowSimpleBracedStatements);
 
+
   AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
@@ -588,6 +589,23 @@
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  EXPECT_EQ("#define A  \\\n"
+"  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
+"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"X;",
+format("#define A \\\n"
+   "   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
+   "  RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; \\\n"
+   "   }\n"
+   "X;",
+   Style));
+}
+
 TEST_F(FormatTest, ParseIfElse) {
   verifyFormat("if (true)\n"
"  if (true)\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -304,9 +304,15 @@
 if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
 I != AnnotatedLines.begin() &&
 I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  return Style.AllowShortBlocksOnASingleLine
- ? tryMergeSimpleBlock(I - 1, E, Limit)
- : 0;
+  unsigned MergedLines = 0;
+  if (Style.AllowShortBlocksOnASingleLine) {
+MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
+// If we managed to merge the block, discard the first  merged line
+// since we are merging starting from I.
+if (MergedLines > 0)
+  --MergedLines;
+  }
+  return MergedLines;
 }
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -495,6 +495,7 @@
"};",
AllowSimpleBracedStatements);
 
+
   AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
@@ -588,6 +589,23 @@
AllowSimpleBracedStatements);
 }
 
+TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
+  FormatStyle Style = getLLVMStyleWithColumns(60);
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortIfStatementsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  EXPECT_EQ("#define A  \\\n"
+"  if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
+"  { RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; }\n"
+"X;",
+format("#define A \\\n"
+   "   if (HANDLEwernufrnuLwrmviferuvnierv) { \\\n"
+   "  RET_ERR1_ANUIREUINERUIFNIOAerwfwrvnuier; \\\n"
+   "   }\n"
+   "X;",
+   Style));
+}
+
 TEST_F(FormatTest, ParseIfElse) {
   verifyFormat("if (true)\n"
"  if (true)\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -304,9 +304,15 @@
 if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
 I != AnnotatedLines.begin() &&
 I[-1]->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
-  return Style.AllowShortBlocksOnASingleLine
- ? tryMergeSimpleBlock(I - 1, E, Limit)
- : 0;
+  unsigned MergedLines = 0;
+  if (Style.AllowShortBlocksOnASingleLine) {
+MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
+// If we managed to merge the block, discard the first  merged line
+// since we