[PATCH] D50699: Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added a project: clang.

See bug report https://bugs.llvm.org/show_bug.cgi?id=38525 for more details.


Repository:
  rC Clang

https://reviews.llvm.org/D50699

Files:
  lib/Format/ContinuationIndenter.cpp


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50699: Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Test case:
F6937162: BreakBeforeBinaryOperators.cpp 


Repository:
  rC Clang

https://reviews.llvm.org/D50699



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

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

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

Updated patch created by "svn diff --diff-cmd=diff -x -U99"


Repository:
  rC Clang

https://reviews.llvm.org/D50699

Files:
  ContinuationIndenter.cpp


Index: ContinuationIndenter.cpp
===
--- ContinuationIndenter.cpp
+++ ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;


Index: ContinuationIndenter.cpp
===
--- ContinuationIndenter.cpp
+++ ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

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

Updated the patch to include lib/Format path prefix.


Repository:
  rC Clang

https://reviews.llvm.org/D50699

Files:
  lib/Format/ContinuationIndenter.cpp


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;


Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

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

Added a test case.


Repository:
  rC Clang

https://reviews.llvm.org/D50699

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3375,6 +3375,22 @@
"= b\n"
"  >> (aa);",
Style);
+
+  Style.ColumnLimit = 80;
+  Style.IndentWidth = 4;
+  Style.TabWidth = 4;
+  Style.UseTab = FormatStyle::UT_Always;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.AlignOperands = false;
+  EXPECT_EQ("{\n"
+"\treturn someVeryVeryLongConditionThatBarelyFitsOnALine\n"
+"\t\t&& (someOtherLongishConditionPart1\n"
+"\t\t\t|| someOtherEvenLongerNestedConditionPart2);\n"
+"}",
+format("{\n"
+   "\treturn someVeryVeryLongConditionThatBarelyFitsOnALine && 
(someOtherLongishConditionPart1 || someOtherEvenLongerNestedConditionPart2);\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, EnforcedOperatorWraps) {
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3375,6 +3375,22 @@
"= b\n"
"  >> (aa);",
Style);
+
+  Style.ColumnLimit = 80;
+  Style.IndentWidth = 4;
+  Style.TabWidth = 4;
+  Style.UseTab = FormatStyle::UT_Always;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.AlignOperands = false;
+  EXPECT_EQ("{\n"
+"\treturn someVeryVeryLongConditionThatBarelyFitsOnALine\n"
+"\t\t&& (someOtherLongishConditionPart1\n"
+"\t\t\t|| someOtherEvenLongerNestedConditionPart2);\n"
+"}",
+format("{\n"
+   "\treturn someVeryVeryLongConditionThatBarelyFitsOnALine && (someOtherLongishConditionPart1 || someOtherEvenLongerNestedConditionPart2);\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, EnforcedOperatorWraps) {
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier

[Owen Pan via Phabricator via cfe-commits]

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

Updated the test case.


Repository:
  rC Clang

https://reviews.llvm.org/D50697

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1145,6 +1145,22 @@
"  break;\n"
"}",
Style);
+  Style.ColumnLimit = 80;
+  Style.AllowShortCaseLabelsOnASingleLine = false;
+  Style.IndentCaseLabels = true;
+  EXPECT_EQ("switch (n) {\n"
+"  default /*comments*/:\n"
+"return true;\n"
+"  case 0:\n"
+"return false;\n"
+"}",
+format("switch (n) {\n"
+   "default/*comments*/:\n"
+   "  return true;\n"
+   "case 0:\n"
+   "  return false;\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -350,7 +350,10 @@
   break;
 case tok::kw_default: {
   unsigned StoredPosition = Tokens->getPosition();
-  FormatToken *Next = Tokens->getNextToken();
+  FormatToken *Next;
+  do {
+Next = Tokens->getNextToken();
+  } while (Next && Next->is(tok::comment));
   FormatTok = Tokens->setPosition(StoredPosition);
   if (Next && Next->isNot(tok::colon)) {
 // default not followed by ':' is not a case label; treat it like


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1145,6 +1145,22 @@
"  break;\n"
"}",
Style);
+  Style.ColumnLimit = 80;
+  Style.AllowShortCaseLabelsOnASingleLine = false;
+  Style.IndentCaseLabels = true;
+  EXPECT_EQ("switch (n) {\n"
+"  default /*comments*/:\n"
+"return true;\n"
+"  case 0:\n"
+"return false;\n"
+"}",
+format("switch (n) {\n"
+   "default/*comments*/:\n"
+   "  return true;\n"
+   "case 0:\n"
+   "  return false;\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -350,7 +350,10 @@
   break;
 case tok::kw_default: {
   unsigned StoredPosition = Tokens->getPosition();
-  FormatToken *Next = Tokens->getNextToken();
+  FormatToken *Next;
+  do {
+Next = Tokens->getNextToken();
+  } while (Next && Next->is(tok::comment));
   FormatTok = Tokens->setPosition(StoredPosition);
   if (Next && Next->isNot(tok::colon)) {
 // default not followed by ':' is not a case label; treat it like
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

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

Simplified the test case.


Repository:
  rC Clang

https://reviews.llvm.org/D50699

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3375,6 +3375,18 @@
"= b\n"
"  >> (aa);",
Style);
+
+  Style.ColumnLimit = 80;
+  Style.IndentWidth = 4;
+  Style.TabWidth = 4;
+  Style.UseTab = FormatStyle::UT_Always;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.AlignOperands = false;
+  EXPECT_EQ("return someVeryVeryLongConditionThatBarelyFitsOnALine\n"
+"\t&& (someOtherLongishConditionPart1\n"
+"\t\t|| someOtherEvenLongerNestedConditionPart2);",
+format("return someVeryVeryLongConditionThatBarelyFitsOnALine && 
(someOtherLongishConditionPart1 || someOtherEvenLongerNestedConditionPart2);",
+   Style));
 }
 
 TEST_F(FormatTest, EnforcedOperatorWraps) {
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3375,6 +3375,18 @@
"= b\n"
"  >> (aa);",
Style);
+
+  Style.ColumnLimit = 80;
+  Style.IndentWidth = 4;
+  Style.TabWidth = 4;
+  Style.UseTab = FormatStyle::UT_Always;
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
+  Style.AlignOperands = false;
+  EXPECT_EQ("return someVeryVeryLongConditionThatBarelyFitsOnALine\n"
+"\t&& (someOtherLongishConditionPart1\n"
+"\t\t|| someOtherEvenLongerNestedConditionPart2);",
+format("return someVeryVeryLongConditionThatBarelyFitsOnALine && (someOtherLongishConditionPart1 || someOtherEvenLongerNestedConditionPart2);",
+   Style));
 }
 
 TEST_F(FormatTest, EnforcedOperatorWraps) {
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

[Owen Pan via Phabricator via cfe-commits]

owenpan requested changes to this revision.
owenpan added a comment.
This revision now requires changes to proceed.

By the way, I didn't review the test cases.




Comment at: lib/Format/FormatToken.h:524-525
+// Detect "(inline|export)? namespace" in the beginning of a line.
+if (NamespaceTok &&
+(NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export)))
   NamespaceTok = NamespaceTok->getNextNonComment();

```
if (NamespaceTok && NamespaceTok->isOneOf(tok::kw_inline, tok::kw_export))
```



Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:128-133
+  // Detect "(inline|export)? namespace" in the beginning of a line.
+  if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export))
 NamespaceTok = NamespaceTok->getNextNonComment();
   if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))
 return nullptr;
   return NamespaceTok;

These lines are functionally the same as lines 523-528 in FormatToken.h. 
Refactor them?



Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:129-130
+  // Detect "(inline|export)? namespace" in the beginning of a line.
+  if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export))
 NamespaceTok = NamespaceTok->getNextNonComment();
   if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))

```
  if (NamespaceTok && NamespaceTok->isOneOf(tok::kw_inline, tok::kw_export))
```



Comment at: lib/Format/UnwrappedLineParser.cpp:992-998
   case tok::kw_inline:
 nextToken();
 if (FormatTok->Tok.is(tok::kw_namespace)) {
   parseNamespace();
   return;
 }
 break;

Move this case to after the case tok::kw_export below.



Comment at: lib/Format/UnwrappedLineParser.cpp:1066-1072
+if (Style.isCpp()) {
+  nextToken();
+  if (FormatTok->Tok.is(tok::kw_namespace)) {
+parseNamespace();
+return;
+  }
+}

```
if (!Style.isCpp())
  break;
  case tok::kw_inline:
nextToken();
if (FormatTok->Tok.is(tok::kw_namespace)) {
  parseNamespace();
  return;
}
```


Repository:
  rC Clang

https://reviews.llvm.org/D51036



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

[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:129-130
+  // Detect "(inline|export)? namespace" in the beginning of a line.
+  if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export))
 NamespaceTok = NamespaceTok->getNextNonComment();
   if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))

owenpan wrote:
> ```
>   if (NamespaceTok && NamespaceTok->isOneOf(tok::kw_inline, tok::kw_export))
> ```
Actually, I meant it for line 129 only.


Repository:
  rC Clang

https://reviews.llvm.org/D51036



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

[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:128-133
+  // Detect "(inline|export)? namespace" in the beginning of a line.
+  if (NamespaceTok->is(tok::kw_inline) || NamespaceTok->is(tok::kw_export))
 NamespaceTok = NamespaceTok->getNextNonComment();
   if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))
 return nullptr;
   return NamespaceTok;

owenpan wrote:
> These lines are functionally the same as lines 523-528 in FormatToken.h. 
> Refactor them?
I think this is better:

```
  return NamespaceTok->getNamespaceToken();
```


Repository:
  rC Clang

https://reviews.llvm.org/D51036



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

[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:1066-1072
+if (Style.isCpp()) {
+  nextToken();
+  if (FormatTok->Tok.is(tok::kw_namespace)) {
+parseNamespace();
+return;
+  }
+}

owenpan wrote:
> ```
> if (!Style.isCpp())
>   break;
>   case tok::kw_inline:
> nextToken();
> if (FormatTok->Tok.is(tok::kw_namespace)) {
>   parseNamespace();
>   return;
> }
> ```
I forgot to include LLVM_FALLTHROUGH to suppress the warning:

```
if (!Style.isCpp())
  break;
LLVM_FALLTHROUGH;
  case tok::kw_inline:
nextToken();
if (FormatTok->Tok.is(tok::kw_namespace)) {
  parseNamespace();
  return;
}
```


Repository:
  rC Clang

https://reviews.llvm.org/D51036



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

[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: lib/Format/Format.cpp:1312
   auto &Line = *AnnotatedLines[i];
   if (Line.startsWith(tok::kw_namespace) ||
+  Line.startsWith(tok::kw_inline, tok::kw_namespace) ||

sammccall wrote:
> these 3 startswith checks appear 4 times now, you could pull out a helper 
> function `startsWithNamespace` in FormatToken.h or something like that.
> Up to you.
I missed it. Good catch!


Repository:
  rC Clang

https://reviews.llvm.org/D51036



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

[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:533-535
+ !(Line.startsWith(tok::kw_namespace) ||
+   Line.startsWith(tok::kw_inline, tok::kw_namespace) ||
+   Line.startsWith(tok::kw_export, tok::kw_namespace)) &&

Maybe add a test case (or modify an existing one) for this fix, with a C/C++ 
style comment before `namespace`, `inline`, and/or `export`?


Repository:
  rC Clang

https://reviews.llvm.org/D51036



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

[PATCH] D50699: [clang-format] fix PR38525 - Extraneous continuation indent spaces with BreakBeforeBinaryOperators set to All

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

@JonasToth Yes, please. Also https://reviews.llvm.org/D50697 :)
Thanks again for getting me started!


Repository:
  rC Clang

https://reviews.llvm.org/D50699



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

[PATCH] D33029: [clang-format] add option for dangling parenthesis

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

@stringham Here is what I did to run the unit tests:

  cd build
  make FormatTests
  tools/clang/unittests/Format/FormatTests


https://reviews.llvm.org/D33029



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

[PATCH] D51294: Fix Bug 38713: clang-format mishandles a short block after "default:" in a switch statement

[Owen Pan via Phabricator via cfe-commits]

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D51294

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -999,6 +999,24 @@
"  }\n"
"});",
getLLVMStyle()));
+  EXPECT_EQ("switch (n) {\n"
+"case 0: {\n"
+"  return false;\n"
+"}\n"
+"default: {\n"
+"  return true;\n"
+"}\n"
+"}",
+format("switch (n)\n"
+   "{\n"
+   "case 0: {\n"
+   "  return false;\n"
+   "}\n"
+   "default: {\n"
+   "  return true;\n"
+   "}\n"
+   "}",
+   getLLVMStyle()));
   verifyFormat("switch (a) {\n"
"case (b):\n"
"  return;\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -483,6 +483,11 @@
 if (Line.First->isOneOf(tok::kw_else, tok::kw_case) ||
 (Line.First->Next && Line.First->Next->is(tok::kw_else)))
   return 0;
+if (Line.First->is(tok::kw_default)) {
+  const FormatToken *Tok = Line.First->getNextNonComment();
+  if (Tok && Tok->is(tok::colon))
+return 0;
+}
 if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::kw_try,
 tok::kw___try, tok::kw_catch, tok::kw___finally,
 tok::kw_for, tok::r_brace, Keywords.kw___except)) {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -999,6 +999,24 @@
"  }\n"
"});",
getLLVMStyle()));
+  EXPECT_EQ("switch (n) {\n"
+"case 0: {\n"
+"  return false;\n"
+"}\n"
+"default: {\n"
+"  return true;\n"
+"}\n"
+"}",
+format("switch (n)\n"
+   "{\n"
+   "case 0: {\n"
+   "  return false;\n"
+   "}\n"
+   "default: {\n"
+   "  return true;\n"
+   "}\n"
+   "}",
+   getLLVMStyle()));
   verifyFormat("switch (a) {\n"
"case (b):\n"
"  return;\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -483,6 +483,11 @@
 if (Line.First->isOneOf(tok::kw_else, tok::kw_case) ||
 (Line.First->Next && Line.First->Next->is(tok::kw_else)))
   return 0;
+if (Line.First->is(tok::kw_default)) {
+  const FormatToken *Tok = Line.First->getNextNonComment();
+  if (Tok && Tok->is(tok::colon))
+return 0;
+}
 if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::kw_try,
 tok::kw___try, tok::kw_catch, tok::kw___finally,
 tok::kw_for, tok::r_brace, Keywords.kw___except)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51294: Fix Bug 38713: clang-format mishandles a short block after "default:" in a switch statement

[Owen Pan via Phabricator via cfe-commits]

owenpan marked an inline comment as done.
owenpan added inline comments.



Comment at: lib/Format/UnwrappedLineFormatter.cpp:486
   return 0;
+if (Line.First->is(tok::kw_default)) {
+  const FormatToken *Tok = Line.First->getNextNonComment();

sammccall wrote:
> Just to check my understanding...
> we want to treat `default` the same as `case`, but the heuristics are 
> different:
>  - `case` should only appear in a switch (but might be followed by a complex 
> expression)
>  - `default` has lots of meanings (but we can disambiguate: check if it's 
> followed by a colon)
> 
> You could consider `// default: in switch statement` above this line.
Exactly!



Comment at: unittests/Format/FormatTest.cpp:1012
+   "{\n"
+   "case 0: {\n"
+   "  return false;\n"

sammccall wrote:
> the intent of this test might be clearer if the cases were formatted as `case 
> 0: { return false; }` on one line
I used the same test case that exposed this bug. Please see it in the bug 
report.

Because of the bug, "case" and "default" case labels were handled differently. 
The former was correctly left alone, but the latter was merged into one line. 
Therefore, the test case checks that neither is merged.


Repository:
  rC Clang

https://reviews.llvm.org/D51294



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

[PATCH] D51294: Fix Bug 38713: clang-format mishandles a short block after "default:" in a switch statement

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 162989.
owenpan marked an inline comment as done.
owenpan added a comment.

Added a comment suggested by @sammccall


Repository:
  rC Clang

https://reviews.llvm.org/D51294

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -999,6 +999,24 @@
"  }\n"
"});",
getLLVMStyle()));
+  EXPECT_EQ("switch (n) {\n"
+"case 0: {\n"
+"  return false;\n"
+"}\n"
+"default: {\n"
+"  return true;\n"
+"}\n"
+"}",
+format("switch (n)\n"
+   "{\n"
+   "case 0: {\n"
+   "  return false;\n"
+   "}\n"
+   "default: {\n"
+   "  return true;\n"
+   "}\n"
+   "}",
+   getLLVMStyle()));
   verifyFormat("switch (a) {\n"
"case (b):\n"
"  return;\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -483,6 +483,12 @@
 if (Line.First->isOneOf(tok::kw_else, tok::kw_case) ||
 (Line.First->Next && Line.First->Next->is(tok::kw_else)))
   return 0;
+// default: in switch statement
+if (Line.First->is(tok::kw_default)) {
+  const FormatToken *Tok = Line.First->getNextNonComment();
+  if (Tok && Tok->is(tok::colon))
+return 0;
+}
 if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::kw_try,
 tok::kw___try, tok::kw_catch, tok::kw___finally,
 tok::kw_for, tok::r_brace, Keywords.kw___except)) {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -999,6 +999,24 @@
"  }\n"
"});",
getLLVMStyle()));
+  EXPECT_EQ("switch (n) {\n"
+"case 0: {\n"
+"  return false;\n"
+"}\n"
+"default: {\n"
+"  return true;\n"
+"}\n"
+"}",
+format("switch (n)\n"
+   "{\n"
+   "case 0: {\n"
+   "  return false;\n"
+   "}\n"
+   "default: {\n"
+   "  return true;\n"
+   "}\n"
+   "}",
+   getLLVMStyle()));
   verifyFormat("switch (a) {\n"
"case (b):\n"
"  return;\n"
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -483,6 +483,12 @@
 if (Line.First->isOneOf(tok::kw_else, tok::kw_case) ||
 (Line.First->Next && Line.First->Next->is(tok::kw_else)))
   return 0;
+// default: in switch statement
+if (Line.First->is(tok::kw_default)) {
+  const FormatToken *Tok = Line.First->getNextNonComment();
+  if (Tok && Tok->is(tok::colon))
+return 0;
+}
 if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::kw_try,
 tok::kw___try, tok::kw_catch, tok::kw___finally,
 tok::kw_for, tok::r_brace, Keywords.kw___except)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier

[Owen Pan via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D51036#1223254, @sammccall wrote:

> In https://reviews.llvm.org/D51036#1223230, @melver wrote:
>
> > Awaiting remaining reviewer acceptance.
> >
> > FYI: I do not have commit commit access -- what is the procedure to commit 
> > once diff is accepted?
> >
> > Many thanks!
>
>
> Anyone with commit access can land it for you - I'm happy to do this.
>  @owenpan any concerns?


@sammccall Go ahead! :)


Repository:
  rC Clang

https://reviews.llvm.org/D51036



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

[PATCH] D51719: Wrapped block after case label should not be merged into a single line (Bug 38854)

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rC Clang

https://reviews.llvm.org/D51719

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1064,6 +1064,32 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentCaseLabels = true;
+  Style.AllowShortBlocksOnASingleLine = false;
+  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, CaseRanges) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -323,6 +323,10 @@
   kwId == clang::tok::objc_synchronized)
 return 0;
 }
+// Don't merge block with left brace wrapped after case labels
+if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
+I[-1]->First->isOneOf(tok::kw_case, tok::kw_default))
+  return 0;
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
   return !Style.BraceWrapping.AfterFunction ||


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1064,6 +1064,32 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentCaseLabels = true;
+  Style.AllowShortBlocksOnASingleLine = false;
+  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, CaseRanges) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -323,6 +323,10 @@
   kwId == clang::tok::objc_synchronized)
 return 0;
 }
+// Don't merge block with left brace wrapped after case labels
+if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
+I[-1]->First->isOneOf(tok::kw_case, tok::kw_default))
+  return 0;
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
   return !Style.BraceWrapping.AfterFunction ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51719: Wrapped block after case label should not be merged into a single line (Bug 38854)

[Owen Pan via Phabricator via cfe-commits]

owenpan added a reviewer: krasimir.
owenpan added a comment.

Can one of the reviews have a quick look at this one? Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D51719



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

[PATCH] D51719: Wrapped block after case label should not be merged into a single line (Bug 38854)

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342116: [clang-format] Wrapped block after case label should 
not be merged into one line (authored by owenpan, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51719?vs=164153&id=165208#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51719

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
@@ -323,6 +323,10 @@
   kwId == clang::tok::objc_synchronized)
 return 0;
 }
+// Don't merge block with left brace wrapped after case labels
+if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
+I[-1]->First->isOneOf(tok::kw_case, tok::kw_default))
+  return 0;
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
   return !Style.BraceWrapping.AfterFunction ||
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1064,6 +1064,32 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentCaseLabels = true;
+  Style.AllowShortBlocksOnASingleLine = false;
+  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, CaseRanges) {


Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -323,6 +323,10 @@
   kwId == clang::tok::objc_synchronized)
 return 0;
 }
+// Don't merge block with left brace wrapped after case labels
+if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
+I[-1]->First->isOneOf(tok::kw_case, tok::kw_default))
+  return 0;
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
   return !Style.BraceWrapping.AfterFunction ||
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1064,6 +1064,32 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentCaseLabels = true;
+  Style.AllowShortBlocksOnASingleLine = false;
+  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, CaseRanges) {
___
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

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

ping


Repository:
  rC Clang

https://reviews.llvm.org/D52527



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

[PATCH] D60308: Leave alone the semicolon after the MacroBlockBegin name

[Owen Pan via Phabricator via cfe-commits]

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

This patch fixes the bug below.

The code:

  FOO_BEGIN();
FOO_ENTRY
  FOO_END();

is erroneously formatted to:

  FOO_BEGIN()
;
FOO_ENTRY
  FOO_END();


Repository:
  rC Clang

https://reviews.llvm.org/D60308

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
@@ -3480,6 +3480,9 @@
"  int x;\n"
"  x = 1;\n"
"FOO_END(Baz)", Style);
+  verifyFormat("FOO_BEGIN();\n"
+   "  FOO_ENTRY\n"
+   "FOO_END();", Style);
 }
 
 
//===--===//
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -540,6 +540,9 @@
   if (MacroBlock && FormatTok->is(tok::l_paren))
 parseParens();
 
+  if (MunchSemi && FormatTok->Tok.is(tok::semi))
+nextToken();
+
   size_t NbPreprocessorDirectives =
   CurrentLines == &Lines ? PreprocessorDirectives.size() : 0;
   addUnwrappedLine();


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3480,6 +3480,9 @@
"  int x;\n"
"  x = 1;\n"
"FOO_END(Baz)", Style);
+  verifyFormat("FOO_BEGIN();\n"
+   "  FOO_ENTRY\n"
+   "FOO_END();", Style);
 }
 
 //===--===//
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -540,6 +540,9 @@
   if (MacroBlock && FormatTok->is(tok::l_paren))
 parseParens();
 
+  if (MunchSemi && FormatTok->Tok.is(tok::semi))
+nextToken();
+
   size_t NbPreprocessorDirectives =
   CurrentLines == &Lines ? PreprocessorDirectives.size() : 0;
   addUnwrappedLine();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60308: Leave alone the semicolon after the MacroBlockBegin name

[Owen Pan via Phabricator via cfe-commits]

owenpan abandoned this revision.
owenpan added a comment.

The semicolon is redundant but makes the MacroBlockBegin name stand out more. 
Nonetheless, I agree it's not a bug and we should leave the current behavior as 
is.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60308



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

[PATCH] D60359: [clang-format] Fix incorrect indentation of C++ constructor initializer list

[Owen Pan via Phabricator via cfe-commits]

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

See https://bugs.llvm.org/show_bug.cgi?id=41407 for more info.


Repository:
  rC Clang

https://reviews.llvm.org/D60359

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11910,6 +11910,13 @@
   "bool smaller = 1 < 
(\n"
   "   
a);",
   Style);
+
+  Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon;
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  "(),\n"
+  "() {}",
+  Style);
 }
 
 TEST_F(FormatTest, BreakConstructorInitializersBeforeComma) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1057,7 +1057,7 @@
   if (Current.is(TT_ProtoExtensionLSquare))
 return State.Stack.back().Indent;
   if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
-  PreviousNonComment->isNot(tok::r_brace))
+  !PreviousNonComment->isOneOf(tok::r_brace, TT_CtorInitializerComma))
 // Ensure that we fall back to the continuation indent width instead of
 // just flushing continuations left.
 return State.Stack.back().Indent + Style.ContinuationIndentWidth;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11910,6 +11910,13 @@
   "bool smaller = 1 < (\n"
   "   a);",
   Style);
+
+  Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon;
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  "(),\n"
+  "() {}",
+  Style);
 }
 
 TEST_F(FormatTest, BreakConstructorInitializersBeforeComma) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1057,7 +1057,7 @@
   if (Current.is(TT_ProtoExtensionLSquare))
 return State.Stack.back().Indent;
   if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
-  PreviousNonComment->isNot(tok::r_brace))
+  !PreviousNonComment->isOneOf(tok::r_brace, TT_CtorInitializerComma))
 // Ensure that we fall back to the continuation indent width instead of
 // just flushing continuations left.
 return State.Stack.back().Indent + Style.ContinuationIndentWidth;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60359: [clang-format] Fix incorrect indentation of C++ constructor initializer list

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357851: [clang-format] Fix Bug 41407 (authored by owenpan, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60359?vs=194007&id=194040#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60359

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


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1057,7 +1057,7 @@
   if (Current.is(TT_ProtoExtensionLSquare))
 return State.Stack.back().Indent;
   if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
-  PreviousNonComment->isNot(tok::r_brace))
+  !PreviousNonComment->isOneOf(tok::r_brace, TT_CtorInitializerComma))
 // Ensure that we fall back to the continuation indent width instead of
 // just flushing continuations left.
 return State.Stack.back().Indent + Style.ContinuationIndentWidth;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11946,6 +11946,13 @@
   "bool smaller = 1 < 
(\n"
   "   
a);",
   Style);
+
+  Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon;
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  "(),\n"
+  "() {}",
+  Style);
 }
 
 TEST_F(FormatTest, BreakConstructorInitializersBeforeComma) {


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -1057,7 +1057,7 @@
   if (Current.is(TT_ProtoExtensionLSquare))
 return State.Stack.back().Indent;
   if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
-  PreviousNonComment->isNot(tok::r_brace))
+  !PreviousNonComment->isOneOf(tok::r_brace, TT_CtorInitializerComma))
 // Ensure that we fall back to the continuation indent width instead of
 // just flushing continuations left.
 return State.Stack.back().Indent + Style.ContinuationIndentWidth;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11946,6 +11946,13 @@
   "bool smaller = 1 < (\n"
   "   a);",
   Style);
+
+  Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon;
+  verifyFormat(
+  "SomeClass::Constructor() :\n"
+  "(),\n"
+  "() {}",
+  Style);
 }
 
 TEST_F(FormatTest, BreakConstructorInitializersBeforeComma) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

@MyDeveloperDay: Can you add a link to the bugzilla report?

I stopped pushing my solution because as @krasimir said it's probably not a bug 
although the current documentation doesn't spell it out. Nonetheless, I'm with 
you and would like to see this improvement accepted.


Repository:
  rC Clang

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

https://reviews.llvm.org/D52527



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

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

[Owen Pan via Phabricator via cfe-commits]

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D60374

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12822,6 +12822,23 @@
 format("int i = longFunction(arg);", SixIndent));
 }
 
+TEST_F(FormatTest, WrappedClosingParenthesisIndent) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat(
+  "Foo::Foo(\n"
+  "//\n"
+  ")\n"
+  ": foo(0) {}",
+  Style);
+  verifyFormat(
+  "int Foo::getter(\n"
+  "//\n"
+  ") const {\n"
+  "  return foo;\n"
+  "}",
+  Style);
+}
+
 TEST_F(FormatTest, SpacesInAngles) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInAngles = true;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -945,18 +945,28 @@
   return State.Stack[State.Stack.size() - 2].LastSpace;
 return State.FirstIndent;
   }
-  // Indent a closing parenthesis at the previous level if followed by a semi 
or
-  // opening brace. This allows indentations such as:
+  // Indent a closing parenthesis at the previous level if followed by a semi,
+  // colon, const, or opening brace. This allows indentations such as:
   // foo(
   //   a,
   // );
+  // Foo::Foo(
+  // //
+  // )
+  // : foo(0) {}
+  // int Foo::getter(
+  // //
+  // ) const {
+  //   return foo;
+  // }
   // function foo(
   //   a,
   // ) {
   //   code(); //
   // }
   if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
-  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace)))
+  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::colon,
+  tok::kw_const, tok::l_brace)))
 return State.Stack[State.Stack.size() - 2].LastSpace;
   if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
 return State.Stack[State.Stack.size() - 2].LastSpace;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12822,6 +12822,23 @@
 format("int i = longFunction(arg);", SixIndent));
 }
 
+TEST_F(FormatTest, WrappedClosingParenthesisIndent) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat(
+  "Foo::Foo(\n"
+  "//\n"
+  ")\n"
+  ": foo(0) {}",
+  Style);
+  verifyFormat(
+  "int Foo::getter(\n"
+  "//\n"
+  ") const {\n"
+  "  return foo;\n"
+  "}",
+  Style);
+}
+
 TEST_F(FormatTest, SpacesInAngles) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInAngles = true;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -945,18 +945,28 @@
   return State.Stack[State.Stack.size() - 2].LastSpace;
 return State.FirstIndent;
   }
-  // Indent a closing parenthesis at the previous level if followed by a semi or
-  // opening brace. This allows indentations such as:
+  // Indent a closing parenthesis at the previous level if followed by a semi,
+  // colon, const, or opening brace. This allows indentations such as:
   // foo(
   //   a,
   // );
+  // Foo::Foo(
+  // //
+  // )
+  // : foo(0) {}
+  // int Foo::getter(
+  // //
+  // ) const {
+  //   return foo;
+  // }
   // function foo(
   //   a,
   // ) {
   //   code(); //
   // }
   if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
-  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace)))
+  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::colon,
+  tok::kw_const, tok::l_brace)))
 return State.Stack[State.Stack.size() - 2].LastSpace;
   if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
 return State.Stack[State.Stack.size() - 2].LastSpace;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D60374#1457638 , @klimek wrote:

> The previous behavior looks intentional, and much more regular. I'd be 
> curious why you think the proposed behavior is more readable.


I see the bug in the inconsistency for the code below:

  int Foo::getter(
  //
  ) const {
return foo;
  }
  
  void Foo::setter(
  //
  ) {
foo = 1;
  }

The closing parenthesis is indented in `getter` but not `setter`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60374



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

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D60374#1457639 , @MyDeveloperDay 
wrote:

> maybe add the following as a test because I think it shows the inconsistency
>
>   void Foo::bar( // some comment
>   ) {}
>  
>   void Foo::bar( // some comment
>   ) const {}
>


I had the `setter` code in the new test case to show the inconsistency but then 
took it out; I don't want to add a (possible) duplicate even though I didn't 
find the test case for Lines 953-957 of `ContinuationIndenter.cpp`. I will put 
my `setter` test case back in.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60374



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

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

[Owen Pan via Phabricator via cfe-commits]

owenpan planned changes to this revision.
owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:968
   if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
-  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace)))
+  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::colon,
+  tok::kw_const, tok::l_brace)))

MyDeveloperDay wrote:
> Do you think the colon you have here in your example is really a 
> TT_CtorInitializerColon, I'm not sure if you need to be specific?
> 
> I wonder how this might interfere with the  
> Style.BreakConstructorInitializers,  I'm a little unclear of the before and 
> after you are trying to fix, could you add something to the description to 
> help me understand?
Good point with using TT_CtorInitializerColon instead!

Although I had tested it with different combinations of 
Style.BreakConstructorInitializers settings and didn't find any interference, 
it's probably not an inconsistency in the case of the colon based on your and 
@klimek's comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60374



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

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D60374#1457693 , @MyDeveloperDay 
wrote:

> LGTM , if you also think the test will help show the use case then please add 
> it, otherwise this revision notes might be information enough


Thanks! I just got the alternative patch ready. Should I discard it and just 
commit this one?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60374



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

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 194072.
This revision is now accepted and ready to land.

Repository:
  rC Clang

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

https://reviews.llvm.org/D60374

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12822,6 +12822,24 @@
 format("int i = longFunction(arg);", SixIndent));
 }
 
+TEST_F(FormatTest, WrappedClosingParenthesisIndent) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat(
+  "int Foo::getter(\n"
+  "//\n"
+  ") const {\n"
+  "  return foo;\n"
+  "}",
+  Style);
+  verifyFormat(
+  "void Foo::setter(\n"
+  "//\n"
+  ") {\n"
+  "  foo = 1;\n"
+  "}",
+  Style);
+}
+
 TEST_F(FormatTest, SpacesInAngles) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInAngles = true;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -945,18 +945,24 @@
   return State.Stack[State.Stack.size() - 2].LastSpace;
 return State.FirstIndent;
   }
-  // Indent a closing parenthesis at the previous level if followed by a semi 
or
-  // opening brace. This allows indentations such as:
+  // Indent a closing parenthesis at the previous level if followed by a semi,
+  // const, or opening brace. This allows indentations such as:
   // foo(
   //   a,
   // );
+  // int Foo::getter(
+  // //
+  // ) const {
+  //   return foo;
+  // }
   // function foo(
   //   a,
   // ) {
   //   code(); //
   // }
   if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
-  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace)))
+  (!Current.Next ||
+   Current.Next->isOneOf(tok::semi, tok::kw_const, tok::l_brace)))
 return State.Stack[State.Stack.size() - 2].LastSpace;
   if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
 return State.Stack[State.Stack.size() - 2].LastSpace;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12822,6 +12822,24 @@
 format("int i = longFunction(arg);", SixIndent));
 }
 
+TEST_F(FormatTest, WrappedClosingParenthesisIndent) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat(
+  "int Foo::getter(\n"
+  "//\n"
+  ") const {\n"
+  "  return foo;\n"
+  "}",
+  Style);
+  verifyFormat(
+  "void Foo::setter(\n"
+  "//\n"
+  ") {\n"
+  "  foo = 1;\n"
+  "}",
+  Style);
+}
+
 TEST_F(FormatTest, SpacesInAngles) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInAngles = true;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -945,18 +945,24 @@
   return State.Stack[State.Stack.size() - 2].LastSpace;
 return State.FirstIndent;
   }
-  // Indent a closing parenthesis at the previous level if followed by a semi or
-  // opening brace. This allows indentations such as:
+  // Indent a closing parenthesis at the previous level if followed by a semi,
+  // const, or opening brace. This allows indentations such as:
   // foo(
   //   a,
   // );
+  // int Foo::getter(
+  // //
+  // ) const {
+  //   return foo;
+  // }
   // function foo(
   //   a,
   // ) {
   //   code(); //
   // }
   if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
-  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace)))
+  (!Current.Next ||
+   Current.Next->isOneOf(tok::semi, tok::kw_const, tok::l_brace)))
 return State.Stack[State.Stack.size() - 2].LastSpace;
   if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
 return State.Stack[State.Stack.size() - 2].LastSpace;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

[Owen Pan via Phabricator via cfe-commits]

owenpan requested review of this revision.
owenpan added a comment.

Updated the diff.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60374



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

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357877: [clang-format] Fix bug 
https://bugs.llvm.org/show_bug.cgi?id=41413 (authored by owenpan, committed by 
).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60374?vs=194072&id=194073#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60374

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


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -945,18 +945,24 @@
   return State.Stack[State.Stack.size() - 2].LastSpace;
 return State.FirstIndent;
   }
-  // Indent a closing parenthesis at the previous level if followed by a semi 
or
-  // opening brace. This allows indentations such as:
+  // Indent a closing parenthesis at the previous level if followed by a semi,
+  // const, or opening brace. This allows indentations such as:
   // foo(
   //   a,
   // );
+  // int Foo::getter(
+  // //
+  // ) const {
+  //   return foo;
+  // }
   // function foo(
   //   a,
   // ) {
   //   code(); //
   // }
   if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
-  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace)))
+  (!Current.Next ||
+   Current.Next->isOneOf(tok::semi, tok::kw_const, tok::l_brace)))
 return State.Stack[State.Stack.size() - 2].LastSpace;
   if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
 return State.Stack[State.Stack.size() - 2].LastSpace;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -12822,6 +12822,24 @@
 format("int i = longFunction(arg);", SixIndent));
 }
 
+TEST_F(FormatTest, WrappedClosingParenthesisIndent) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat(
+  "int Foo::getter(\n"
+  "//\n"
+  ") const {\n"
+  "  return foo;\n"
+  "}",
+  Style);
+  verifyFormat(
+  "void Foo::setter(\n"
+  "//\n"
+  ") {\n"
+  "  foo = 1;\n"
+  "}",
+  Style);
+}
+
 TEST_F(FormatTest, SpacesInAngles) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInAngles = true;


Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp
===
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp
@@ -945,18 +945,24 @@
   return State.Stack[State.Stack.size() - 2].LastSpace;
 return State.FirstIndent;
   }
-  // Indent a closing parenthesis at the previous level if followed by a semi or
-  // opening brace. This allows indentations such as:
+  // Indent a closing parenthesis at the previous level if followed by a semi,
+  // const, or opening brace. This allows indentations such as:
   // foo(
   //   a,
   // );
+  // int Foo::getter(
+  // //
+  // ) const {
+  //   return foo;
+  // }
   // function foo(
   //   a,
   // ) {
   //   code(); //
   // }
   if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
-  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace)))
+  (!Current.Next ||
+   Current.Next->isOneOf(tok::semi, tok::kw_const, tok::l_brace)))
 return State.Stack[State.Stack.size() - 2].LastSpace;
   if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
 return State.Stack[State.Stack.size() - 2].LastSpace;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -12822,6 +12822,24 @@
 format("int i = longFunction(arg);", SixIndent));
 }
 
+TEST_F(FormatTest, WrappedClosingParenthesisIndent) {
+  FormatStyle Style = getLLVMStyle();
+  verifyFormat(
+  "int Foo::getter(\n"
+  "//\n"
+  ") const {\n"
+  "  return foo;\n"
+  "}",
+  Style);
+  verifyFormat(
+  "void Foo::setter(\n"
+  "//\n"
+  ") {\n"
+  "  foo = 1;\n"
+  "}",
+  Style);
+}
+
 TEST_F(FormatTest, SpacesInAngles) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInAngles = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 194219.
owenpan added a reviewer: reuk.
owenpan added a comment.

Thank you to all for reviewing this revision! Here is the update that addresses 
all of your comments.

(Also added @reuk to the reviewer list per @MyDeveloperDay 's suggestion.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D52527

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  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
@@ -1117,6 +1117,7 @@
   Style.IndentCaseLabels = true;
   Style.AllowShortBlocksOnASingleLine = false;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = true;
   EXPECT_EQ("switch (n)\n"
 "{\n"
@@ -1138,6 +1139,27 @@
"  }\n"
"}",
Style));
+  Style.BraceWrapping.AfterCaseLabel = false;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0: {\n"
+"return false;\n"
+"  }\n"
+"  default: {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0:\n"
+   "  {\n"
+   "return false;\n"
+   "  }\n"
+   "  default:\n"
+   "  {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -1291,6 +1313,7 @@
Style));
   Style.AllowShortCaseLabelsOnASingleLine = true;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = true;
   EXPECT_EQ("switch (n)\n"
 "{\n"
@@ -11356,6 +11379,7 @@
   CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
   CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
 
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterCaseLabel);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterControlStatement);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterEnum);
@@ -12841,20 +12865,18 @@
 
 TEST_F(FormatTest, WrappedClosingParenthesisIndent) {
   FormatStyle Style = getLLVMStyle();
-  verifyFormat(
-  "int Foo::getter(\n"
-  "//\n"
-  ") const {\n"
-  "  return foo;\n"
-  "}",
-  Style);
-  verifyFormat(
-  "void Foo::setter(\n"
-  "//\n"
-  ") {\n"
-  "  foo = 1;\n"
-  "}",
-  Style);
+  verifyFormat("int Foo::getter(\n"
+   "//\n"
+   ") const {\n"
+   "  return foo;\n"
+   "}",
+   Style);
+  verifyFormat("void Foo::setter(\n"
+   "//\n"
+   ") {\n"
+   "  foo = 1;\n"
+   "}",
+   Style);
 }
 
 TEST_F(FormatTest, SpacesInAngles) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -172,10 +172,16 @@
 public:
   CompoundStatementIndenter(UnwrappedLineParser *Parser,
 const FormatStyle &Style, unsigned &LineLevel)
+  : CompoundStatementIndenter(Parser, LineLevel,
+  Style.BraceWrapping.AfterControlStatement,
+  Style.BraceWrapping.IndentBraces) {
+  }
+  CompoundStatementIndenter(UnwrappedLineParser *Parser, unsigned &LineLevel,
+bool WrapBrace, bool IndentBrace)
   : LineLevel(LineLevel), OldLineLevel(LineLevel) {
-if (Style.BraceWrapping.AfterControlStatement)
+if (WrapBrace)
   Parser->addUnwrappedLine();
-if (Style.BraceWrapping.IndentBraces)
+if (IndentBrace)
   ++LineLevel;
   }
   ~CompoundStatementIndenter() { LineLevel = OldLineLevel; }
@@ -1950,7 +1956,9 @@
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
 --Line->Level;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
-CompoundStatementIndenter Indenter(this, Style, Line->Level);
+CompoundStatementIndenter Indenter(this, Line->Level,
+   Style.BraceWrapping.AfterCaseLabel,
+   Style.BraceWrapping.IndentBraces);
 parseBlock(/*MustBeDeclaration=*/false);
 if (FormatTok->Tok.is(tok::kw_break)) {
   if (Style.BraceWrapping.AfterControlStatement)
Index: clang/lib/Format/Forma

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

@MyDeveloperDay :

> do you happen to know if this script is run by the build or is supposed to be 
> run by the developer after making the change to Format.h

I believe the developer must run it manually.

> If the latter, then I reckon the two are out of sync, perhaps I should submit 
> a change to realign them, but really there should be some sort of make target 
> that lets us determine when they diverge otherwise they'll keep changing

+1


Repository:
  rC Clang

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

https://reviews.llvm.org/D52527



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

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

[Owen Pan via Phabricator via cfe-commits]

owenpan closed this revision.
owenpan added a comment.

llvm-svn: 357957


Repository:
  rC Clang

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

https://reviews.llvm.org/D52527



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

[PATCH] D60363: [clang-format] [PR41170] Break after return type ignored with certain comments positions

[Owen Pan via Phabricator via cfe-commits]

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

Looks good!


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

https://reviews.llvm.org/D60363



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

[PATCH] D60362: [clang-format] [PR39719] clang-format converting object-like macro to function-like macro

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

A more straightforward way, IMO, is to add to the `spaceRequiredBetween` 
function a separate `if` statement that returns false for the sequence of 
tokens: `#`, `define`, tok::identifier, and `(`


Repository:
  rC Clang

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

https://reviews.llvm.org/D60362



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

[PATCH] D60853: clang-format incorrectly inserts a space after a macro function name that is a keyword

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: klimek, djasper, reuk, russellmcc, sammccall, 
MyDeveloperDay.
owenpan added a project: clang.
Herald added a subscriber: cfe-commits.

See https://bugs.llvm.org/show_bug.cgi?id=39719 and discussion: 
https://reviews.llvm.org/D60362


Repository:
  rC Clang

https://reviews.llvm.org/D60853

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
@@ -2468,6 +2468,12 @@
 TEST_F(FormatTest, RespectWhitespaceInMacroDefinitions) {
   EXPECT_EQ("#define A (x)", format("#define A (x)"));
   EXPECT_EQ("#define A(x)", format("#define A(x)"));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Never;
+  verifyFormat("#define true ((foo)1)", Style);
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+  verifyFormat("#define false((foo)0)", Style);
 }
 
 TEST_F(FormatTest, EmptyLinesInMacroDefinitions) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -805,7 +805,7 @@
 void UnwrappedLineParser::parsePPDefine() {
   nextToken();
 
-  if (FormatTok->Tok.getKind() != tok::identifier) {
+  if (!FormatTok->Tok.getIdentifierInfo()) {
 IncludeGuard = IG_Rejected;
 IncludeGuardToken = nullptr;
 parsePPUnknown();


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2468,6 +2468,12 @@
 TEST_F(FormatTest, RespectWhitespaceInMacroDefinitions) {
   EXPECT_EQ("#define A (x)", format("#define A (x)"));
   EXPECT_EQ("#define A(x)", format("#define A(x)"));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Never;
+  verifyFormat("#define true ((foo)1)", Style);
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+  verifyFormat("#define false((foo)0)", Style);
 }
 
 TEST_F(FormatTest, EmptyLinesInMacroDefinitions) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -805,7 +805,7 @@
 void UnwrappedLineParser::parsePPDefine() {
   nextToken();
 
-  if (FormatTok->Tok.getKind() != tok::identifier) {
+  if (!FormatTok->Tok.getIdentifierInfo()) {
 IncludeGuard = IG_Rejected;
 IncludeGuardToken = nullptr;
 parsePPUnknown();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60362: [clang-format] [PR39719] clang-format converting object-like macro to function-like macro

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Actually, there is a neater way: https://reviews.llvm.org/D60853


Repository:
  rC Clang

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

https://reviews.llvm.org/D60362



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

[PATCH] D60853: clang-format converts a keyword macro definition to a macro function

[Owen Pan via Phabricator via cfe-commits]

owenpan marked 2 inline comments as done.
owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:808
 
-  if (FormatTok->Tok.getKind() != tok::identifier) {
+  if (!FormatTok->Tok.getIdentifierInfo()) {
 IncludeGuard = IG_Rejected;

MyDeveloperDay wrote:
> Is this equivalent to saying something that means `FormatTok->isKeyWord()` ? 
> for those case where the #define 
The condition tests if `FormatTok` is neither a keyword (`true`, `false`, etc.) 
nor a non-keyword identifier.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60853



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

[PATCH] D60853: clang-format converts a keyword macro definition to a macro function

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358710: [clang-format] Fix incorrect formatting of keyword 
macro definition (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60853?vs=195675&id=195806#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60853

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


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -805,7 +805,7 @@
 void UnwrappedLineParser::parsePPDefine() {
   nextToken();
 
-  if (FormatTok->Tok.getKind() != tok::identifier) {
+  if (!FormatTok->Tok.getIdentifierInfo()) {
 IncludeGuard = IG_Rejected;
 IncludeGuardToken = nullptr;
 parsePPUnknown();
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -2468,6 +2468,12 @@
 TEST_F(FormatTest, RespectWhitespaceInMacroDefinitions) {
   EXPECT_EQ("#define A (x)", format("#define A (x)"));
   EXPECT_EQ("#define A(x)", format("#define A(x)"));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Never;
+  verifyFormat("#define true ((foo)1)", Style);
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+  verifyFormat("#define false((foo)0)", Style);
 }
 
 TEST_F(FormatTest, EmptyLinesInMacroDefinitions) {


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -805,7 +805,7 @@
 void UnwrappedLineParser::parsePPDefine() {
   nextToken();
 
-  if (FormatTok->Tok.getKind() != tok::identifier) {
+  if (!FormatTok->Tok.getIdentifierInfo()) {
 IncludeGuard = IG_Rejected;
 IncludeGuardToken = nullptr;
 parsePPUnknown();
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -2468,6 +2468,12 @@
 TEST_F(FormatTest, RespectWhitespaceInMacroDefinitions) {
   EXPECT_EQ("#define A (x)", format("#define A (x)"));
   EXPECT_EQ("#define A(x)", format("#define A(x)"));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Never;
+  verifyFormat("#define true ((foo)1)", Style);
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+  verifyFormat("#define false((foo)0)", Style);
 }
 
 TEST_F(FormatTest, EmptyLinesInMacroDefinitions) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60996: [clang-format] Fix bug in reflow of block comments containing CR/LF

[Owen Pan via Phabricator via cfe-commits]

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

This patch fixes PR 36119 .


Repository:
  rC Clang

https://reviews.llvm.org/D60996

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,18 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   " \r\n"
+   "*/"));
+  EXPECT_EQ("/*\r\n"
+"\r\r\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -40,6 +40,8 @@
 bool UseCRLF)
   : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {}
 
+  bool useCRLF() const { return UseCRLF; }
+
   /// Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   ///
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1810,7 +1810,7 @@
 }
 return llvm::make_unique(
 Current, StartColumn, Current.OriginalColumn, !Current.Previous,
-State.Line->InPPDirective, Encoding, Style);
+State.Line->InPPDirective, Encoding, Style, Whitespaces.useCRLF());
   } else if (Current.is(TT_LineComment) &&
  (Current.Previous == nullptr ||
   Current.Previous->isNot(TT_ImplicitStringLiteral))) {
Index: clang/lib/Format/BreakableToken.h
===
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -359,7 +359,7 @@
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine,
 bool InPPDirective, encoding::Encoding Encoding,
-const FormatStyle &Style);
+const FormatStyle &Style, bool UseCRLF);
 
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -329,7 +329,7 @@
 BreakableBlockComment::BreakableBlockComment(
 const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
-encoding::Encoding Encoding, const FormatStyle &Style)
+encoding::Encoding Encoding, const FormatStyle &Style, bool UseCRLF)
 : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style),
   DelimitersOnNewline(false),
   UnbreakableTailLength(Token.UnbreakableTailLength) {
@@ -338,7 +338,8 @@
 
   StringRef TokenText(Tok.TokenText);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
-  TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
+  TokenText.substr(2, TokenText.size() - 4).split(Lines,
+  UseCRLF ? "\r\n" : "\n");
 
   int IndentDelta = StartColumn - OriginalStartColumn;
   Content.resize(Lines.size());


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,18 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   " \r\n"
+   "*/"));
+  EXPECT_EQ("/*\r\n"
+"\r\r\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/W

[PATCH] D60996: [clang-format] Fix bug in reflow of block comments containing CR/LF

[Owen Pan via Phabricator via cfe-commits]

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

Removes a no longer needed hack that worked around this bug when computing 
`StartOfLine` in the `BreakableBlockComment::adjustWhitespace` function.

Also updates the test case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60996

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,18 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   " \r\n"
+   "*/"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -40,6 +40,8 @@
 bool UseCRLF)
   : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {}
 
+  bool useCRLF() const { return UseCRLF; }
+
   /// Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   ///
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1810,7 +1810,7 @@
 }
 return llvm::make_unique(
 Current, StartColumn, Current.OriginalColumn, !Current.Previous,
-State.Line->InPPDirective, Encoding, Style);
+State.Line->InPPDirective, Encoding, Style, Whitespaces.useCRLF());
   } else if (Current.is(TT_LineComment) &&
  (Current.Previous == nullptr ||
   Current.Previous->isNot(TT_ImplicitStringLiteral))) {
Index: clang/lib/Format/BreakableToken.h
===
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -359,7 +359,7 @@
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine,
 bool InPPDirective, encoding::Encoding Encoding,
-const FormatStyle &Style);
+const FormatStyle &Style, bool UseCRLF);
 
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -329,7 +329,7 @@
 BreakableBlockComment::BreakableBlockComment(
 const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
-encoding::Encoding Encoding, const FormatStyle &Style)
+encoding::Encoding Encoding, const FormatStyle &Style, bool UseCRLF)
 : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style),
   DelimitersOnNewline(false),
   UnbreakableTailLength(Token.UnbreakableTailLength) {
@@ -338,7 +338,8 @@
 
   StringRef TokenText(Tok.TokenText);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
-  TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
+  TokenText.substr(2, TokenText.size() - 4).split(Lines,
+  UseCRLF ? "\r\n" : "\n");
 
   int IndentDelta = StartColumn - OriginalStartColumn;
   Content.resize(Lines.size());
@@ -472,7 +473,7 @@
   // Calculate the start of the non-whitespace text in the current line.
   size_t StartOfLine = Lines[LineIndex].find_first_not_of(Blanks);
   if (StartOfLine == StringRef::npos)
-StartOfLine = Lines[LineIndex].rtrim("\r\n").size();
+StartOfLine = Lines[LineIndex].size();
 
   StringRef Whitespace = Lines[LineIndex].substr(0, StartOfLine);
   // Adjust Lines to only contain relevant text.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,18 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+

[PATCH] D60996: [clang-format] Fix bug in reflow of block comments containing CR/LF

[Owen Pan via Phabricator via cfe-commits]

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

Removes a redundant test case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60996

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,12 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -40,6 +40,8 @@
 bool UseCRLF)
   : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {}
 
+  bool useCRLF() const { return UseCRLF; }
+
   /// Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   ///
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1810,7 +1810,7 @@
 }
 return llvm::make_unique(
 Current, StartColumn, Current.OriginalColumn, !Current.Previous,
-State.Line->InPPDirective, Encoding, Style);
+State.Line->InPPDirective, Encoding, Style, Whitespaces.useCRLF());
   } else if (Current.is(TT_LineComment) &&
  (Current.Previous == nullptr ||
   Current.Previous->isNot(TT_ImplicitStringLiteral))) {
Index: clang/lib/Format/BreakableToken.h
===
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -359,7 +359,7 @@
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine,
 bool InPPDirective, encoding::Encoding Encoding,
-const FormatStyle &Style);
+const FormatStyle &Style, bool UseCRLF);
 
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -329,7 +329,7 @@
 BreakableBlockComment::BreakableBlockComment(
 const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
-encoding::Encoding Encoding, const FormatStyle &Style)
+encoding::Encoding Encoding, const FormatStyle &Style, bool UseCRLF)
 : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style),
   DelimitersOnNewline(false),
   UnbreakableTailLength(Token.UnbreakableTailLength) {
@@ -338,7 +338,8 @@
 
   StringRef TokenText(Tok.TokenText);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
-  TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
+  TokenText.substr(2, TokenText.size() - 4).split(Lines,
+  UseCRLF ? "\r\n" : "\n");
 
   int IndentDelta = StartColumn - OriginalStartColumn;
   Content.resize(Lines.size());
@@ -472,7 +473,7 @@
   // Calculate the start of the non-whitespace text in the current line.
   size_t StartOfLine = Lines[LineIndex].find_first_not_of(Blanks);
   if (StartOfLine == StringRef::npos)
-StartOfLine = Lines[LineIndex].rtrim("\r\n").size();
+StartOfLine = Lines[LineIndex].size();
 
   StringRef Whitespace = Lines[LineIndex].substr(0, StartOfLine);
   // Adjust Lines to only contain relevant text.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,12 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {
Index: clang/lib/Format/WhitespaceManager.h
===
--- clang/lib/Format/WhitespaceManag

[PATCH] D60996: [clang-format] Fix bug in reflow of block comments containing CR/LF

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC359029: [clang-format] Fix bug in reflow of block comments 
containing CR/LF (authored by owenpan, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60996?vs=196261&id=196314#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60996

Files:
  lib/Format/BreakableToken.cpp
  lib/Format/BreakableToken.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp


Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -40,6 +40,8 @@
 bool UseCRLF)
   : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {}
 
+  bool useCRLF() const { return UseCRLF; }
+
   /// Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   ///
Index: lib/Format/BreakableToken.h
===
--- lib/Format/BreakableToken.h
+++ lib/Format/BreakableToken.h
@@ -359,7 +359,7 @@
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine,
 bool InPPDirective, encoding::Encoding Encoding,
-const FormatStyle &Style);
+const FormatStyle &Style, bool UseCRLF);
 
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
Index: lib/Format/BreakableToken.cpp
===
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -329,7 +329,7 @@
 BreakableBlockComment::BreakableBlockComment(
 const FormatToken &Token, unsigned StartColumn,
 unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
-encoding::Encoding Encoding, const FormatStyle &Style)
+encoding::Encoding Encoding, const FormatStyle &Style, bool UseCRLF)
 : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style),
   DelimitersOnNewline(false),
   UnbreakableTailLength(Token.UnbreakableTailLength) {
@@ -338,7 +338,8 @@
 
   StringRef TokenText(Tok.TokenText);
   assert(TokenText.startswith("/*") && TokenText.endswith("*/"));
-  TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
+  TokenText.substr(2, TokenText.size() - 4).split(Lines,
+  UseCRLF ? "\r\n" : "\n");
 
   int IndentDelta = StartColumn - OriginalStartColumn;
   Content.resize(Lines.size());
@@ -472,7 +473,7 @@
   // Calculate the start of the non-whitespace text in the current line.
   size_t StartOfLine = Lines[LineIndex].find_first_not_of(Blanks);
   if (StartOfLine == StringRef::npos)
-StartOfLine = Lines[LineIndex].rtrim("\r\n").size();
+StartOfLine = Lines[LineIndex].size();
 
   StringRef Whitespace = Lines[LineIndex].substr(0, StartOfLine);
   // Adjust Lines to only contain relevant text.
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1810,7 +1810,7 @@
 }
 return llvm::make_unique(
 Current, StartColumn, Current.OriginalColumn, !Current.Previous,
-State.Line->InPPDirective, Encoding, Style);
+State.Line->InPPDirective, Encoding, Style, Whitespaces.useCRLF());
   } else if (Current.is(TT_LineComment) &&
  (Current.Previous == nullptr ||
   Current.Previous->isNot(TT_ImplicitStringLiteral))) {
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -12857,6 +12857,12 @@
"should not introduce\r\n"
"an extra carriage return\r\n"
"*/\r\n"));
+  EXPECT_EQ("/*\r\n"
+"\r\n"
+"*/",
+format("/*\r\n"
+   "\r\r\r\n"
+   "*/"));
 }
 
 TEST_F(FormatTest, MunchSemicolonAfterBlocks) {


Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -40,6 +40,8 @@
 bool UseCRLF)
   : SourceMgr(SourceMgr), Style(Style), UseCRLF(UseCRLF) {}
 
+  bool useCRLF() const { return UseCRLF; }
+
   /// Replaces the whitespace in front of \p Tok. Only call once for
   /// each \c AnnotatedToken.
   ///
Index: lib/Format/BreakableToken.h
===
--- lib/Format/BreakableToken.h
+++ lib/Format/BreakableToken.h
@@ -359,7 +359,7 @@
  

[PATCH] D61174: [clang-format] Fix documentation for FixNamespaceComments

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes PR40409 .


Repository:
  rC Clang

https://reviews.llvm.org/D61174

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1139,7 +1139,7 @@
   ///true:  false:
   ///namespace a {  vs. namespace a {
   ///foo(); foo();
-  ///} // namespace a;  }
+  ///} // namespace a   }
   /// \endcode
   bool FixNamespaceComments;
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1346,7 +1346,7 @@
  true:  false:
  namespace a {  vs. namespace a {
  foo(); foo();
- } // namespace a;  }
+ } // namespace a   }
 
 **ForEachMacros** (``std::vector``)
   A vector of macros that should be interpreted as foreach loops


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1139,7 +1139,7 @@
   ///true:  false:
   ///namespace a {  vs. namespace a {
   ///foo(); foo();
-  ///} // namespace a;  }
+  ///} // namespace a   }
   /// \endcode
   bool FixNamespaceComments;
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1346,7 +1346,7 @@
  true:  false:
  namespace a {  vs. namespace a {
  foo(); foo();
- } // namespace a;  }
+ } // namespace a   }
 
 **ForEachMacros** (``std::vector``)
   A vector of macros that should be interpreted as foreach loops
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61174: [clang-format] Fix documentation for FixNamespaceComments

[Owen Pan via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359280: [clang-format] Fix documentation for 
FixNamespaceComments (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61174?vs=196800&id=196803#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61174

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h


Index: cfe/trunk/docs/ClangFormatStyleOptions.rst
===
--- cfe/trunk/docs/ClangFormatStyleOptions.rst
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst
@@ -1346,7 +1346,7 @@
  true:  false:
  namespace a {  vs. namespace a {
  foo(); foo();
- } // namespace a;  }
+ } // namespace a   }
 
 **ForEachMacros** (``std::vector``)
   A vector of macros that should be interpreted as foreach loops
Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1139,7 +1139,7 @@
   ///true:  false:
   ///namespace a {  vs. namespace a {
   ///foo(); foo();
-  ///} // namespace a;  }
+  ///} // namespace a   }
   /// \endcode
   bool FixNamespaceComments;
 


Index: cfe/trunk/docs/ClangFormatStyleOptions.rst
===
--- cfe/trunk/docs/ClangFormatStyleOptions.rst
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst
@@ -1346,7 +1346,7 @@
  true:  false:
  namespace a {  vs. namespace a {
  foo(); foo();
- } // namespace a;  }
+ } // namespace a   }
 
 **ForEachMacros** (``std::vector``)
   A vector of macros that should be interpreted as foreach loops
Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1139,7 +1139,7 @@
   ///true:  false:
   ///namespace a {  vs. namespace a {
   ///foo(); foo();
-  ///} // namespace a;  }
+  ///} // namespace a   }
   /// \endcode
   bool FixNamespaceComments;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61222: [clang-format] Fix bug in determineTokenType() for TT_StartOfName

[Owen Pan via Phabricator via cfe-commits]

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

This patch fixes PR37175 .


Repository:
  rC Clang

https://reviews.llvm.org/D61222

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10575,6 +10575,13 @@
"  unsigned c;\n"
"}",
Alignment);
+
+  // See PR37175
+  FormatStyle Style = getMozillaStyle();
+  Style.AlignConsecutiveDeclarations = true;
+  EXPECT_EQ("DECOR1 DECOR2 uint32_t\n"
+"f1(int arg1, int arg2);",
+format("DECOR1 DECOR2 uint32_t f1 (int arg1, int arg2);", Style));
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1350,8 +1350,14 @@
   Current.Type = TT_BinaryOperator;
 } else if (isStartOfName(Current) &&
(!Line.MightBeFunctionDecl || Current.NestingLevel != 0)) {
-  Contexts.back().FirstStartOfName = &Current;
-  Current.Type = TT_StartOfName;
+  const FormatToken *Next = Current.Next;
+  bool NonMacroIdentifier =
+  Next && Next->Tok.getIdentifierInfo() &&
+  Next->TokenText != Next->TokenText.upper();
+  if (!NonMacroIdentifier) {
+Contexts.back().FirstStartOfName = &Current;
+Current.Type = TT_StartOfName;
+  }
 } else if (Current.is(tok::semi)) {
   // Reset FirstStartOfName after finding a semicolon so that a for loop
   // with multiple increment statements is not confused with a for loop


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10575,6 +10575,13 @@
"  unsigned c;\n"
"}",
Alignment);
+
+  // See PR37175
+  FormatStyle Style = getMozillaStyle();
+  Style.AlignConsecutiveDeclarations = true;
+  EXPECT_EQ("DECOR1 DECOR2 uint32_t\n"
+"f1(int arg1, int arg2);",
+format("DECOR1 DECOR2 uint32_t f1 (int arg1, int arg2);", Style));
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1350,8 +1350,14 @@
   Current.Type = TT_BinaryOperator;
 } else if (isStartOfName(Current) &&
(!Line.MightBeFunctionDecl || Current.NestingLevel != 0)) {
-  Contexts.back().FirstStartOfName = &Current;
-  Current.Type = TT_StartOfName;
+  const FormatToken *Next = Current.Next;
+  bool NonMacroIdentifier =
+  Next && Next->Tok.getIdentifierInfo() &&
+  Next->TokenText != Next->TokenText.upper();
+  if (!NonMacroIdentifier) {
+Contexts.back().FirstStartOfName = &Current;
+Current.Type = TT_StartOfName;
+  }
 } else if (Current.is(tok::semi)) {
   // Reset FirstStartOfName after finding a semicolon so that a for loop
   // with multiple increment statements is not confused with a for loop
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61222: [clang-format] Fix bug in determineTokenType() for TT_StartOfName

[Owen Pan via Phabricator via cfe-commits]

owenpan planned changes to this revision.
owenpan added a comment.

Tested it a bit more and found another problem. The code

  DECOR1 uint32_t DECOR2 function1 (int arg1, int arg2) { return 1U; }
  DECOR1 unsigned DECOR2 function2 (int arg1, int arg2) { return 1U; }

would still be misformatted to:

  DECOR1 uint32_t DECOR2
 function1(int arg1, int arg2) { return 1U; }
  DECOR1 unsigned DECOR2
  function2(int arg1, int arg2) { return 1U; }


Repository:
  rC Clang

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

https://reviews.llvm.org/D61222



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

[PATCH] D61222: [clang-format] Fix a bug in AlignConsecutiveDeclarations

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 196994.
owenpan retitled this revision from "[clang-format] Fix bug in 
determineTokenType() for TT_StartOfName" to "[clang-format] Fix a bug in 
AlignConsecutiveDeclarations".
owenpan added a comment.
This revision is now accepted and ready to land.

Fix it in `WhitespaceManager::alignConsecutiveDeclarations()` instead.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61222

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10575,6 +10575,16 @@
"  unsigned c;\n"
"}",
Alignment);
+
+  // See PR37175
+  FormatStyle Style = getMozillaStyle();
+  Style.AlignConsecutiveDeclarations = true;
+  EXPECT_EQ("DECOR1 /**/ DECOR2 /**/ int8_t /**/\n"
+"foo(int a);",
+format("DECOR1 /**/ DECOR2 /**/ int8_t /**/ foo (int a);", Style));
+  EXPECT_EQ("DECOR1 /**/ int8_t /**/ DECOR2 /**/\n"
+"bar(int a);",
+format("DECOR1 /**/ int8_t /**/ DECOR2 /**/ bar (int a);", Style));
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -463,9 +463,21 @@
   [](Change const &C) {
 // tok::kw_operator is necessary for aligning operator overload
 // definitions.
-return C.Tok->is(TT_StartOfName) ||
-   C.Tok->is(TT_FunctionDeclarationName) ||
-   C.Tok->is(tok::kw_operator);
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, tok::kw_operator))
+  return true;
+if (C.Tok->isNot(TT_StartOfName))
+  return false;
+// Check if there is a subsequent name that starts the same 
declaration.
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
+  if (Next->is(tok::comment))
+continue;
+  if (!Next->Tok.getIdentifierInfo())
+break;
+  if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+tok::kw_operator))
+return false;
+}
+return true;
   },
   Changes, /*StartAt=*/0);
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10575,6 +10575,16 @@
"  unsigned c;\n"
"}",
Alignment);
+
+  // See PR37175
+  FormatStyle Style = getMozillaStyle();
+  Style.AlignConsecutiveDeclarations = true;
+  EXPECT_EQ("DECOR1 /**/ DECOR2 /**/ int8_t /**/\n"
+"foo(int a);",
+format("DECOR1 /**/ DECOR2 /**/ int8_t /**/ foo (int a);", Style));
+  EXPECT_EQ("DECOR1 /**/ int8_t /**/ DECOR2 /**/\n"
+"bar(int a);",
+format("DECOR1 /**/ int8_t /**/ DECOR2 /**/ bar (int a);", Style));
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -463,9 +463,21 @@
   [](Change const &C) {
 // tok::kw_operator is necessary for aligning operator overload
 // definitions.
-return C.Tok->is(TT_StartOfName) ||
-   C.Tok->is(TT_FunctionDeclarationName) ||
-   C.Tok->is(tok::kw_operator);
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, tok::kw_operator))
+  return true;
+if (C.Tok->isNot(TT_StartOfName))
+  return false;
+// Check if there is a subsequent name that starts the same declaration.
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
+  if (Next->is(tok::comment))
+continue;
+  if (!Next->Tok.getIdentifierInfo())
+break;
+  if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+tok::kw_operator))
+return false;
+}
+return true;
   },
   Changes, /*StartAt=*/0);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

[Owen Pan via Phabricator via cfe-commits]

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

This patch fixes PR41213 .


Repository:
  rC Clang

https://reviews.llvm.org/D61276

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11137,6 +11137,16 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" * 456789012345678\n"
+" * /x\n"
+" */",
+format("/*\n"
+   " *\t456789012345678 /x\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   // comment", Style);
   EXPECT_EQ("int a; /* first line\n"
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -380,6 +380,8 @@
   break;
 if (!Content[i].empty() && i + 1 != e && Decoration.startswith(Content[i]))
   continue;
+if (Content[i].startswith("*\t"))
+  continue;
 while (!Content[i].startswith(Decoration))
   Decoration = Decoration.substr(0, Decoration.size() - 1);
   }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11137,6 +11137,16 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" * 456789012345678\n"
+" * /x\n"
+" */",
+format("/*\n"
+   " *\t456789012345678 /x\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   // comment", Style);
   EXPECT_EQ("int a; /* first line\n"
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -380,6 +380,8 @@
   break;
 if (!Content[i].empty() && i + 1 != e && Decoration.startswith(Content[i]))
   continue;
+if (Content[i].startswith("*\t"))
+  continue;
 while (!Content[i].startswith(Decoration))
   Decoration = Decoration.substr(0, Decoration.size() - 1);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61297: [clang-format] Fix bug that misses some function-like macro usages

[Owen Pan via Phabricator via cfe-commits]

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

This patch fixes PR41483 .


Repository:
  rC Clang

https://reviews.llvm.org/D61297

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
@@ -2584,6 +2584,12 @@
   verifyFormat("VISIT_GL_CALL(GenBuffers, void, (GLsizei n, GLuint* buffers), "
"(n, buffers))\n",
getChromiumStyle(FormatStyle::LK_Cpp));
+
+  // See PR41483
+  EXPECT_EQ("/**/ FOO(a)\n"
+"FOO(b)",
+format("/**/ FOO(a)\n"
+   "FOO(b)"));
 }
 
 TEST_F(FormatTest, MacroCallsWithoutTrailingSemicolon) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1335,10 +1335,15 @@
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
-  if (Line->Tokens.size() == 1 &&
-  // JS doesn't have macros, and within classes colons indicate fields,
-  // not labels.
-  Style.Language != FormatStyle::LK_JavaScript) {
+
+  // JS doesn't have macros, and within classes colons indicate fields, not
+  // labels.
+  if (Style.Language == FormatStyle::LK_JavaScript)
+break;
+
+  TokenCount = Line->Tokens.size();
+  if (TokenCount == 1 ||
+  (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
   parseLabel();


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2584,6 +2584,12 @@
   verifyFormat("VISIT_GL_CALL(GenBuffers, void, (GLsizei n, GLuint* buffers), "
"(n, buffers))\n",
getChromiumStyle(FormatStyle::LK_Cpp));
+
+  // See PR41483
+  EXPECT_EQ("/**/ FOO(a)\n"
+"FOO(b)",
+format("/**/ FOO(a)\n"
+   "FOO(b)"));
 }
 
 TEST_F(FormatTest, MacroCallsWithoutTrailingSemicolon) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1335,10 +1335,15 @@
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
-  if (Line->Tokens.size() == 1 &&
-  // JS doesn't have macros, and within classes colons indicate fields,
-  // not labels.
-  Style.Language != FormatStyle::LK_JavaScript) {
+
+  // JS doesn't have macros, and within classes colons indicate fields, not
+  // labels.
+  if (Style.Language == FormatStyle::LK_JavaScript)
+break;
+
+  TokenCount = Line->Tokens.size();
+  if (TokenCount == 1 ||
+  (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
   parseLabel();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61297: [clang-format] Fix bug that misses some function-like macro usages

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359687: [clang-format] Fix bug that misses some 
function-like macro usages (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61297?vs=197275&id=197553#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61297

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


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1335,10 +1335,15 @@
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
-  if (Line->Tokens.size() == 1 &&
-  // JS doesn't have macros, and within classes colons indicate fields,
-  // not labels.
-  Style.Language != FormatStyle::LK_JavaScript) {
+
+  // JS doesn't have macros, and within classes colons indicate fields, not
+  // labels.
+  if (Style.Language == FormatStyle::LK_JavaScript)
+break;
+
+  TokenCount = Line->Tokens.size();
+  if (TokenCount == 1 ||
+  (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
   parseLabel();
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -2584,6 +2584,12 @@
   verifyFormat("VISIT_GL_CALL(GenBuffers, void, (GLsizei n, GLuint* buffers), "
"(n, buffers))\n",
getChromiumStyle(FormatStyle::LK_Cpp));
+
+  // See PR41483
+  EXPECT_EQ("/**/ FOO(a)\n"
+"FOO(b)",
+format("/**/ FOO(a)\n"
+   "FOO(b)"));
 }
 
 TEST_F(FormatTest, MacroCallsWithoutTrailingSemicolon) {


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1335,10 +1335,15 @@
   // See if the following token should start a new unwrapped line.
   StringRef Text = FormatTok->TokenText;
   nextToken();
-  if (Line->Tokens.size() == 1 &&
-  // JS doesn't have macros, and within classes colons indicate fields,
-  // not labels.
-  Style.Language != FormatStyle::LK_JavaScript) {
+
+  // JS doesn't have macros, and within classes colons indicate fields, not
+  // labels.
+  if (Style.Language == FormatStyle::LK_JavaScript)
+break;
+
+  TokenCount = Line->Tokens.size();
+  if (TokenCount == 1 ||
+  (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
   parseLabel();
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -2584,6 +2584,12 @@
   verifyFormat("VISIT_GL_CALL(GenBuffers, void, (GLsizei n, GLuint* buffers), "
"(n, buffers))\n",
getChromiumStyle(FormatStyle::LK_Cpp));
+
+  // See PR41483
+  EXPECT_EQ("/**/ FOO(a)\n"
+"FOO(b)",
+format("/**/ FOO(a)\n"
+   "FOO(b)"));
 }
 
 TEST_F(FormatTest, MacroCallsWithoutTrailingSemicolon) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

@MyDeveloperDay In theory, any whitespace character other than a blank might 
trigger the bug, but in practice, we only need to handle the tab, IMO.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61276



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

[PATCH] D61222: [clang-format] Fix a bug in AlignConsecutiveDeclarations

[Owen Pan via Phabricator via cfe-commits]

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

Removed a redundant test case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61222

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10575,6 +10575,13 @@
"  unsigned c;\n"
"}",
Alignment);
+
+  // See PR37175
+  FormatStyle Style = getMozillaStyle();
+  Style.AlignConsecutiveDeclarations = true;
+  EXPECT_EQ("DECOR1 /**/ int8_t /**/ DECOR2 /**/\n"
+"foo(int a);",
+format("DECOR1 /**/ int8_t /**/ DECOR2 /**/ foo (int a);", Style));
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -463,9 +463,21 @@
   [](Change const &C) {
 // tok::kw_operator is necessary for aligning operator overload
 // definitions.
-return C.Tok->is(TT_StartOfName) ||
-   C.Tok->is(TT_FunctionDeclarationName) ||
-   C.Tok->is(tok::kw_operator);
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, tok::kw_operator))
+  return true;
+if (C.Tok->isNot(TT_StartOfName))
+  return false;
+// Check if there is a subsequent name that starts the same 
declaration.
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
+  if (Next->is(tok::comment))
+continue;
+  if (!Next->Tok.getIdentifierInfo())
+break;
+  if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+tok::kw_operator))
+return false;
+}
+return true;
   },
   Changes, /*StartAt=*/0);
 }


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10575,6 +10575,13 @@
"  unsigned c;\n"
"}",
Alignment);
+
+  // See PR37175
+  FormatStyle Style = getMozillaStyle();
+  Style.AlignConsecutiveDeclarations = true;
+  EXPECT_EQ("DECOR1 /**/ int8_t /**/ DECOR2 /**/\n"
+"foo(int a);",
+format("DECOR1 /**/ int8_t /**/ DECOR2 /**/ foo (int a);", Style));
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -463,9 +463,21 @@
   [](Change const &C) {
 // tok::kw_operator is necessary for aligning operator overload
 // definitions.
-return C.Tok->is(TT_StartOfName) ||
-   C.Tok->is(TT_FunctionDeclarationName) ||
-   C.Tok->is(tok::kw_operator);
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, tok::kw_operator))
+  return true;
+if (C.Tok->isNot(TT_StartOfName))
+  return false;
+// Check if there is a subsequent name that starts the same declaration.
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
+  if (Next->is(tok::comment))
+continue;
+  if (!Next->Tok.getIdentifierInfo())
+break;
+  if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+tok::kw_operator))
+return false;
+}
+return true;
   },
   Changes, /*StartAt=*/0);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61222: [clang-format] Fix a bug in AlignConsecutiveDeclarations

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359711: [clang-format] Fix a bug in 
AlignConsecutiveDeclarations. (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61222?vs=197566&id=197600#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61222

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


Index: cfe/trunk/lib/Format/WhitespaceManager.cpp
===
--- cfe/trunk/lib/Format/WhitespaceManager.cpp
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp
@@ -463,9 +463,21 @@
   [](Change const &C) {
 // tok::kw_operator is necessary for aligning operator overload
 // definitions.
-return C.Tok->is(TT_StartOfName) ||
-   C.Tok->is(TT_FunctionDeclarationName) ||
-   C.Tok->is(tok::kw_operator);
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, tok::kw_operator))
+  return true;
+if (C.Tok->isNot(TT_StartOfName))
+  return false;
+// Check if there is a subsequent name that starts the same 
declaration.
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
+  if (Next->is(tok::comment))
+continue;
+  if (!Next->Tok.getIdentifierInfo())
+break;
+  if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+tok::kw_operator))
+return false;
+}
+return true;
   },
   Changes, /*StartAt=*/0);
 }
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -10581,6 +10581,13 @@
"  unsigned c;\n"
"}",
Alignment);
+
+  // See PR37175
+  FormatStyle Style = getMozillaStyle();
+  Style.AlignConsecutiveDeclarations = true;
+  EXPECT_EQ("DECOR1 /**/ int8_t /**/ DECOR2 /**/\n"
+"foo(int a);",
+format("DECOR1 /**/ int8_t /**/ DECOR2 /**/ foo (int a);", Style));
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {


Index: cfe/trunk/lib/Format/WhitespaceManager.cpp
===
--- cfe/trunk/lib/Format/WhitespaceManager.cpp
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp
@@ -463,9 +463,21 @@
   [](Change const &C) {
 // tok::kw_operator is necessary for aligning operator overload
 // definitions.
-return C.Tok->is(TT_StartOfName) ||
-   C.Tok->is(TT_FunctionDeclarationName) ||
-   C.Tok->is(tok::kw_operator);
+if (C.Tok->isOneOf(TT_FunctionDeclarationName, tok::kw_operator))
+  return true;
+if (C.Tok->isNot(TT_StartOfName))
+  return false;
+// Check if there is a subsequent name that starts the same declaration.
+for (FormatToken *Next = C.Tok->Next; Next; Next = Next->Next) {
+  if (Next->is(tok::comment))
+continue;
+  if (!Next->Tok.getIdentifierInfo())
+break;
+  if (Next->isOneOf(TT_StartOfName, TT_FunctionDeclarationName,
+tok::kw_operator))
+return false;
+}
+return true;
   },
   Changes, /*StartAt=*/0);
 }
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -10581,6 +10581,13 @@
"  unsigned c;\n"
"}",
Alignment);
+
+  // See PR37175
+  FormatStyle Style = getMozillaStyle();
+  Style.AlignConsecutiveDeclarations = true;
+  EXPECT_EQ("DECOR1 /**/ int8_t /**/ DECOR2 /**/\n"
+"foo(int a);",
+format("DECOR1 /**/ int8_t /**/ DECOR2 /**/ foo (int a);", Style));
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

[Owen Pan via Phabricator via cfe-commits]

owenpan planned changes to this revision.
owenpan added a comment.

There are some corner cases this patch doesn't cover.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61276



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

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

[Owen Pan via Phabricator via cfe-commits]

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

Updated the patch to cover all scenarios in which the bug might be triggered.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61276

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11137,6 +11137,24 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" *\t456789012345678\n"
+" * /x\n"
+" */",
+format("/*\n"
+   " *\t456789012345678 /x\n"
+   " */",
+   Style));
+  EXPECT_EQ("/*\n"
+" *3456789012345678\n"
+" * /x\n"
+" */",
+format("/*\n"
+   " *3456789012345678 /x\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   // comment", Style);
   EXPECT_EQ("int a; /* first line\n"
Index: clang/lib/Format/BreakableToken.h
===
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -361,6 +361,9 @@
 bool InPPDirective, encoding::Encoding Encoding,
 const FormatStyle &Style, bool UseCRLF);
 
+  Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+ unsigned ContentStartColumn,
+ llvm::Regex &CommentPragmasRegex) const override;
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
   unsigned StartColumn) const override;
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -65,7 +65,8 @@
 static BreakableToken::Split
 getCommentSplit(StringRef Text, unsigned ContentStartColumn,
 unsigned ColumnLimit, unsigned TabWidth,
-encoding::Encoding Encoding, const FormatStyle &Style) {
+encoding::Encoding Encoding, const FormatStyle &Style,
+bool DecorationEndsWithStar = false) {
   LLVM_DEBUG(llvm::dbgs() << "Comment split: \"" << Text
   << "\", Column limit: " << ColumnLimit
   << ", Content start: " << ContentStartColumn << 
"\n");
@@ -123,7 +124,10 @@
 if (SpaceOffset == 1 && Text[SpaceOffset - 1] == '*')
   return BreakableToken::Split(StringRef::npos, 0);
 StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim(Blanks);
-StringRef AfterCut = Text.substr(SpaceOffset).ltrim(Blanks);
+StringRef AfterCut = Text.substr(SpaceOffset);
+// Don't trim the leading blanks if it would create a */ after the break.
+if (!DecorationEndsWithStar || AfterCut.size() <= 1 || AfterCut[1] != '/')
+  AfterCut = AfterCut.ltrim(Blanks);
 return BreakableToken::Split(BeforeCut.size(),
  AfterCut.begin() - BeforeCut.end());
   }
@@ -452,6 +456,18 @@
   });
 }
 
+BreakableToken::Split
+BreakableBlockComment::getSplit(unsigned LineIndex, unsigned TailOffset,
+   unsigned ColumnLimit, unsigned ContentStartColumn,
+   llvm::Regex &CommentPragmasRegex) const {
+  // Don't break lines matching the comment pragmas regex.
+  if (CommentPragmasRegex.match(Content[LineIndex]))
+return Split(StringRef::npos, 0);
+  return getCommentSplit(Content[LineIndex].substr(TailOffset),
+ ContentStartColumn, ColumnLimit, Style.TabWidth,
+ Encoding, Style, Decoration.endswith("*"));
+}
+
 void BreakableBlockComment::adjustWhitespace(unsigned LineIndex,
  int IndentDelta) {
   // When in a preprocessor directive, the trailing backslash in a block 
comment


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11137,6 +11137,24 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" *\t456789012345678\n"
+" * /x\n"
+" */",
+format("/*\n"
+   " *\t456789012345678 /x\n"
+   " */",
+   Style));
+  EXPECT_EQ("/*\n"
+" *3456789012345678\n"
+" * /x\n"
+" */",
+format("/*\n"
+   " *3456789012345678

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

[Owen Pan via Phabricator via cfe-commits]

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

Updated the test cases to make them precise and more varied.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61276

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/lib/Format/BreakableToken.h
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11137,6 +11137,24 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" *\t9012345\n"
+" * /8901\n"
+" */",
+format("/*\n"
+   " *\t9012345 /8901\n"
+   " */",
+   Style));
+  EXPECT_EQ("/*\n"
+" *345678\n"
+" *\t/8901\n"
+" */",
+format("/*\n"
+   " *345678\t/8901\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   // comment", Style);
   EXPECT_EQ("int a; /* first line\n"
Index: clang/lib/Format/BreakableToken.h
===
--- clang/lib/Format/BreakableToken.h
+++ clang/lib/Format/BreakableToken.h
@@ -361,6 +361,9 @@
 bool InPPDirective, encoding::Encoding Encoding,
 const FormatStyle &Style, bool UseCRLF);
 
+  Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+ unsigned ContentStartColumn,
+ llvm::Regex &CommentPragmasRegex) const override;
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
   unsigned StartColumn) const override;
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -65,7 +65,8 @@
 static BreakableToken::Split
 getCommentSplit(StringRef Text, unsigned ContentStartColumn,
 unsigned ColumnLimit, unsigned TabWidth,
-encoding::Encoding Encoding, const FormatStyle &Style) {
+encoding::Encoding Encoding, const FormatStyle &Style,
+bool DecorationEndsWithStar = false) {
   LLVM_DEBUG(llvm::dbgs() << "Comment split: \"" << Text
   << "\", Column limit: " << ColumnLimit
   << ", Content start: " << ContentStartColumn << 
"\n");
@@ -123,7 +124,10 @@
 if (SpaceOffset == 1 && Text[SpaceOffset - 1] == '*')
   return BreakableToken::Split(StringRef::npos, 0);
 StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim(Blanks);
-StringRef AfterCut = Text.substr(SpaceOffset).ltrim(Blanks);
+StringRef AfterCut = Text.substr(SpaceOffset);
+// Don't trim the leading blanks if it would create a */ after the break.
+if (!DecorationEndsWithStar || AfterCut.size() <= 1 || AfterCut[1] != '/')
+  AfterCut = AfterCut.ltrim(Blanks);
 return BreakableToken::Split(BeforeCut.size(),
  AfterCut.begin() - BeforeCut.end());
   }
@@ -452,6 +456,18 @@
   });
 }
 
+BreakableToken::Split
+BreakableBlockComment::getSplit(unsigned LineIndex, unsigned TailOffset,
+   unsigned ColumnLimit, unsigned ContentStartColumn,
+   llvm::Regex &CommentPragmasRegex) const {
+  // Don't break lines matching the comment pragmas regex.
+  if (CommentPragmasRegex.match(Content[LineIndex]))
+return Split(StringRef::npos, 0);
+  return getCommentSplit(Content[LineIndex].substr(TailOffset),
+ ContentStartColumn, ColumnLimit, Style.TabWidth,
+ Encoding, Style, Decoration.endswith("*"));
+}
+
 void BreakableBlockComment::adjustWhitespace(unsigned LineIndex,
  int IndentDelta) {
   // When in a preprocessor directive, the trailing backslash in a block 
comment


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11137,6 +11137,24 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" *\t9012345\n"
+" * /8901\n"
+" */",
+format("/*\n"
+   " *\t9012345 /8901\n"
+   " */",
+   Style));
+  EXPECT_EQ("/*\n"
+" *345678\n"
+" *\t/8901\n"
+" */",
+format("/*\n"
+   " *345678\t/8901\n"
+   " */",
+   Style))

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359943: [clang-format] Fix bug in block comment reflow that 
joins * and / (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61276?vs=198057&id=198107#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61276

Files:
  cfe/trunk/lib/Format/BreakableToken.cpp
  cfe/trunk/lib/Format/BreakableToken.h
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/BreakableToken.h
===
--- cfe/trunk/lib/Format/BreakableToken.h
+++ cfe/trunk/lib/Format/BreakableToken.h
@@ -361,6 +361,9 @@
 bool InPPDirective, encoding::Encoding Encoding,
 const FormatStyle &Style, bool UseCRLF);
 
+  Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+ unsigned ContentStartColumn,
+ llvm::Regex &CommentPragmasRegex) const override;
   unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
   StringRef::size_type Length,
   unsigned StartColumn) const override;
Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -65,7 +65,8 @@
 static BreakableToken::Split
 getCommentSplit(StringRef Text, unsigned ContentStartColumn,
 unsigned ColumnLimit, unsigned TabWidth,
-encoding::Encoding Encoding, const FormatStyle &Style) {
+encoding::Encoding Encoding, const FormatStyle &Style,
+bool DecorationEndsWithStar = false) {
   LLVM_DEBUG(llvm::dbgs() << "Comment split: \"" << Text
   << "\", Column limit: " << ColumnLimit
   << ", Content start: " << ContentStartColumn << 
"\n");
@@ -123,7 +124,10 @@
 if (SpaceOffset == 1 && Text[SpaceOffset - 1] == '*')
   return BreakableToken::Split(StringRef::npos, 0);
 StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim(Blanks);
-StringRef AfterCut = Text.substr(SpaceOffset).ltrim(Blanks);
+StringRef AfterCut = Text.substr(SpaceOffset);
+// Don't trim the leading blanks if it would create a */ after the break.
+if (!DecorationEndsWithStar || AfterCut.size() <= 1 || AfterCut[1] != '/')
+  AfterCut = AfterCut.ltrim(Blanks);
 return BreakableToken::Split(BeforeCut.size(),
  AfterCut.begin() - BeforeCut.end());
   }
@@ -452,6 +456,18 @@
   });
 }
 
+BreakableToken::Split
+BreakableBlockComment::getSplit(unsigned LineIndex, unsigned TailOffset,
+   unsigned ColumnLimit, unsigned ContentStartColumn,
+   llvm::Regex &CommentPragmasRegex) const {
+  // Don't break lines matching the comment pragmas regex.
+  if (CommentPragmasRegex.match(Content[LineIndex]))
+return Split(StringRef::npos, 0);
+  return getCommentSplit(Content[LineIndex].substr(TailOffset),
+ ContentStartColumn, ColumnLimit, Style.TabWidth,
+ Encoding, Style, Decoration.endswith("*"));
+}
+
 void BreakableBlockComment::adjustWhitespace(unsigned LineIndex,
  int IndentDelta) {
   // When in a preprocessor directive, the trailing backslash in a block 
comment
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11150,6 +11150,24 @@
   FormatStyle Style = getLLVMStyle();
   Style.ColumnLimit = 20;
 
+  // See PR41213
+  EXPECT_EQ("/*\n"
+" *\t9012345\n"
+" * /8901\n"
+" */",
+format("/*\n"
+   " *\t9012345 /8901\n"
+   " */",
+   Style));
+  EXPECT_EQ("/*\n"
+" *345678\n"
+" *\t/8901\n"
+" */",
+format("/*\n"
+   " *345678\t/8901\n"
+   " */",
+   Style));
+
   verifyFormat("int a; // the\n"
"   // comment", Style);
   EXPECT_EQ("int a; /* first line\n"


Index: cfe/trunk/lib/Format/BreakableToken.h
===
--- cfe/trunk/lib/Format/BreakableToken.h
+++ cfe/trunk/lib/Format/BreakableToken.h
@@ -361,6 +361,9 @@
 bool InPPDirective, encoding::Encoding Encoding,
 const FormatStyle &Style, bool UseCRLF);
 
+  Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+ unsigned C

[PATCH] D61559: Fix the crash when formatting unsupported encodings

[Owen Pan via Phabricator via cfe-commits]

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

See PR33946 .


Repository:
  rC Clang

https://reviews.llvm.org/D61559

Files:
  clang/tools/clang-format/ClangFormat.cpp


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -257,6 +257,35 @@
   std::unique_ptr Code = std::move(CodeOrErr.get());
   if (Code->getBufferSize() == 0)
 return false; // Empty files are formatted correctly.
+
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // http://en.wikipedia.org/wiki/Byte_order_mark for more information.
+  StringRef BufStr = Code->getBuffer();
+  const char *InvalidBOM = llvm::StringSwitch(BufStr)
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
+.StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+.StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+.StartsWith("\x2B\x2F\x76", "UTF-7")
+.StartsWith("\xF7\x64\x4C", "UTF-1")
+.StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+.StartsWith("\x0E\xFE\xFF", "SDSU")
+.StartsWith("\xFB\xEE\x28", "BOCU-1")
+.StartsWith("\x84\x31\x95\x33", "GB-18030")
+.Default(nullptr);
+
+  if (InvalidBOM) {
+errs() << "error: encoding with unsupported byte order mark \""
+   << InvalidBOM << "\" detected";
+if (FileName != "-")
+  errs() << " in file '" << FileName << "'";
+errs() << ".\n";
+return true;
+  }
+
   std::vector Ranges;
   if (fillRanges(Code.get(), Ranges))
 return true;


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -257,6 +257,35 @@
   std::unique_ptr Code = std::move(CodeOrErr.get());
   if (Code->getBufferSize() == 0)
 return false; // Empty files are formatted correctly.
+
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // http://en.wikipedia.org/wiki/Byte_order_mark for more information.
+  StringRef BufStr = Code->getBuffer();
+  const char *InvalidBOM = llvm::StringSwitch(BufStr)
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
+.StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+.StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+.StartsWith("\x2B\x2F\x76", "UTF-7")
+.StartsWith("\xF7\x64\x4C", "UTF-1")
+.StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+.StartsWith("\x0E\xFE\xFF", "SDSU")
+.StartsWith("\xFB\xEE\x28", "BOCU-1")
+.StartsWith("\x84\x31\x95\x33", "GB-18030")
+.Default(nullptr);
+
+  if (InvalidBOM) {
+errs() << "error: encoding with unsupported byte order mark \""
+   << InvalidBOM << "\" detected";
+if (FileName != "-")
+  errs() << " in file '" << FileName << "'";
+errs() << ".\n";
+return true;
+  }
+
   std::vector Ranges;
   if (fillRanges(Code.get(), Ranges))
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61559: Fix the crash when formatting unsupported encodings

[Owen Pan via Phabricator via cfe-commits]

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

Moved "UTF-32 (LE)" to before "UTF-16 (LE)" in `llvm::StringSwitch` so that the 
former BOM wouldn't be misnamed as the latter.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61559

Files:
  clang/tools/clang-format/ClangFormat.cpp


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -257,6 +257,36 @@
   std::unique_ptr Code = std::move(CodeOrErr.get());
   if (Code->getBufferSize() == 0)
 return false; // Empty files are formatted correctly.
+
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  StringRef BufStr = Code->getBuffer();
+  const char *InvalidBOM = llvm::StringSwitch(BufStr)
+.StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+.StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
+.StartsWith("\x2B\x2F\x76", "UTF-7")
+.StartsWith("\xF7\x64\x4C", "UTF-1")
+.StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+.StartsWith("\x0E\xFE\xFF", "SDSU")
+.StartsWith("\xFB\xEE\x28", "BOCU-1")
+.StartsWith("\x84\x31\x95\x33", "GB-18030")
+.Default(nullptr);
+
+  if (InvalidBOM) {
+errs() << "error: encoding with unsupported byte order mark \""
+   << InvalidBOM << "\" detected";
+if (FileName != "-")
+  errs() << " in file '" << FileName << "'";
+errs() << ".\n";
+return true;
+  }
+
   std::vector Ranges;
   if (fillRanges(Code.get(), Ranges))
 return true;


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -257,6 +257,36 @@
   std::unique_ptr Code = std::move(CodeOrErr.get());
   if (Code->getBufferSize() == 0)
 return false; // Empty files are formatted correctly.
+
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  StringRef BufStr = Code->getBuffer();
+  const char *InvalidBOM = llvm::StringSwitch(BufStr)
+.StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+.StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
+.StartsWith("\x2B\x2F\x76", "UTF-7")
+.StartsWith("\xF7\x64\x4C", "UTF-1")
+.StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+.StartsWith("\x0E\xFE\xFF", "SDSU")
+.StartsWith("\xFB\xEE\x28", "BOCU-1")
+.StartsWith("\x84\x31\x95\x33", "GB-18030")
+.Default(nullptr);
+
+  if (InvalidBOM) {
+errs() << "error: encoding with unsupported byte order mark \""
+   << InvalidBOM << "\" detected";
+if (FileName != "-")
+  errs() << " in file '" << FileName << "'";
+errs() << ".\n";
+return true;
+  }
+
   std::vector Ranges;
   if (fillRanges(Code.get(), Ranges))
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61628: Fix a bug that reports UTF16 (LE) files as UTF32 (LE) ones

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added a reviewer: sammccall.
owenpan added a project: clang.
Herald added a subscriber: cfe-commits.

Also fix a typo for the SCSU byte order mark.


Repository:
  rC Clang

https://reviews.llvm.org/D61628

Files:
  clang/lib/Basic/SourceManager.cpp


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -167,16 +167,16 @@
   // http://en.wikipedia.org/wiki/Byte_order_mark for more information.
   StringRef BufStr = Buffer.getPointer()->getBuffer();
   const char *InvalidBOM = llvm::StringSwitch(BufStr)
-.StartsWith("\xFE\xFF", "UTF-16 (BE)")
-.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
   "UTF-32 (BE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
   "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith("\x2B\x2F\x76", "UTF-7")
 .StartsWith("\xF7\x64\x4C", "UTF-1")
 .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
-.StartsWith("\x0E\xFE\xFF", "SDSU")
+.StartsWith("\x0E\xFE\xFF", "SCSU")
 .StartsWith("\xFB\xEE\x28", "BOCU-1")
 .StartsWith("\x84\x31\x95\x33", "GB-18030")
 .Default(nullptr);


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -167,16 +167,16 @@
   // http://en.wikipedia.org/wiki/Byte_order_mark for more information.
   StringRef BufStr = Buffer.getPointer()->getBuffer();
   const char *InvalidBOM = llvm::StringSwitch(BufStr)
-.StartsWith("\xFE\xFF", "UTF-16 (BE)")
-.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
   "UTF-32 (BE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
   "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith("\x2B\x2F\x76", "UTF-7")
 .StartsWith("\xF7\x64\x4C", "UTF-1")
 .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
-.StartsWith("\x0E\xFE\xFF", "SDSU")
+.StartsWith("\x0E\xFE\xFF", "SCSU")
 .StartsWith("\xFB\xEE\x28", "BOCU-1")
 .StartsWith("\x84\x31\x95\x33", "GB-18030")
 .Default(nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61628: Fix a bug that reports UTF16 (LE) files as UTF32 (LE) ones

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

This patch fixes `clang`. I copied the code from here to 
`clang/tools/clang-format/ClangFormat.cpp` in D61559 
 to fix `clang-format`. I'm not sure if the 
code here should be moved into a function, though.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61628



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

[PATCH] D61559: Fix the crash when formatting unsupported encodings

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

I copied the code from `clang/lib/Basic/SourceManager.cpp`. See D61628 
.

I will update this patch to correct the typo in SCSU or remove it along with 
the other rare BOMs. How do you add test cases for this kind of fixes that emit 
error messages?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61559



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

[PATCH] D61628: Fix a bug that reports UTF16 (LE) files as UTF32 (LE) ones

[Owen Pan via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL360256: [clang] Fix a bug that reports UTF32 (LE) files as 
UTF16 (LE) ones (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61628?vs=198404&id=198641#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61628

Files:
  cfe/trunk/lib/Basic/SourceManager.cpp


Index: cfe/trunk/lib/Basic/SourceManager.cpp
===
--- cfe/trunk/lib/Basic/SourceManager.cpp
+++ cfe/trunk/lib/Basic/SourceManager.cpp
@@ -167,16 +167,16 @@
   // http://en.wikipedia.org/wiki/Byte_order_mark for more information.
   StringRef BufStr = Buffer.getPointer()->getBuffer();
   const char *InvalidBOM = llvm::StringSwitch(BufStr)
-.StartsWith("\xFE\xFF", "UTF-16 (BE)")
-.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
   "UTF-32 (BE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
   "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith("\x2B\x2F\x76", "UTF-7")
 .StartsWith("\xF7\x64\x4C", "UTF-1")
 .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
-.StartsWith("\x0E\xFE\xFF", "SDSU")
+.StartsWith("\x0E\xFE\xFF", "SCSU")
 .StartsWith("\xFB\xEE\x28", "BOCU-1")
 .StartsWith("\x84\x31\x95\x33", "GB-18030")
 .Default(nullptr);


Index: cfe/trunk/lib/Basic/SourceManager.cpp
===
--- cfe/trunk/lib/Basic/SourceManager.cpp
+++ cfe/trunk/lib/Basic/SourceManager.cpp
@@ -167,16 +167,16 @@
   // http://en.wikipedia.org/wiki/Byte_order_mark for more information.
   StringRef BufStr = Buffer.getPointer()->getBuffer();
   const char *InvalidBOM = llvm::StringSwitch(BufStr)
-.StartsWith("\xFE\xFF", "UTF-16 (BE)")
-.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
   "UTF-32 (BE)")
 .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
   "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
 .StartsWith("\x2B\x2F\x76", "UTF-7")
 .StartsWith("\xF7\x64\x4C", "UTF-1")
 .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
-.StartsWith("\x0E\xFE\xFF", "SDSU")
+.StartsWith("\x0E\xFE\xFF", "SCSU")
 .StartsWith("\xFB\xEE\x28", "BOCU-1")
 .StartsWith("\x84\x31\x95\x33", "GB-18030")
 .Default(nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61559: Fix the crash when formatting unsupported encodings

[Owen Pan via Phabricator via cfe-commits]

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

Fixed the typo for SCSU.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61559

Files:
  clang/tools/clang-format/ClangFormat.cpp


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -257,6 +257,36 @@
   std::unique_ptr Code = std::move(CodeOrErr.get());
   if (Code->getBufferSize() == 0)
 return false; // Empty files are formatted correctly.
+
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  StringRef BufStr = Code->getBuffer();
+  const char *InvalidBOM = llvm::StringSwitch(BufStr)
+.StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+.StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
+.StartsWith("\x2B\x2F\x76", "UTF-7")
+.StartsWith("\xF7\x64\x4C", "UTF-1")
+.StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+.StartsWith("\x0E\xFE\xFF", "SCSU")
+.StartsWith("\xFB\xEE\x28", "BOCU-1")
+.StartsWith("\x84\x31\x95\x33", "GB-18030")
+.Default(nullptr);
+
+  if (InvalidBOM) {
+errs() << "error: encoding with unsupported byte order mark \""
+   << InvalidBOM << "\" detected";
+if (FileName != "-")
+  errs() << " in file '" << FileName << "'";
+errs() << ".\n";
+return true;
+  }
+
   std::vector Ranges;
   if (fillRanges(Code.get(), Ranges))
 return true;


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -257,6 +257,36 @@
   std::unique_ptr Code = std::move(CodeOrErr.get());
   if (Code->getBufferSize() == 0)
 return false; // Empty files are formatted correctly.
+
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  StringRef BufStr = Code->getBuffer();
+  const char *InvalidBOM = llvm::StringSwitch(BufStr)
+.StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+.StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
+.StartsWith("\x2B\x2F\x76", "UTF-7")
+.StartsWith("\xF7\x64\x4C", "UTF-1")
+.StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+.StartsWith("\x0E\xFE\xFF", "SCSU")
+.StartsWith("\xFB\xEE\x28", "BOCU-1")
+.StartsWith("\x84\x31\x95\x33", "GB-18030")
+.Default(nullptr);
+
+  if (InvalidBOM) {
+errs() << "error: encoding with unsupported byte order mark \""
+   << InvalidBOM << "\" detected";
+if (FileName != "-")
+  errs() << " in file '" << FileName << "'";
+errs() << ".\n";
+return true;
+  }
+
   std::vector Ranges;
   if (fillRanges(Code.get(), Ranges))
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61559: Fix the crash when formatting unsupported encodings

[Owen Pan via Phabricator via cfe-commits]

owenpan marked an inline comment as done.
owenpan added inline comments.



Comment at: clang/tools/clang-format/ClangFormat.cpp:273
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
+.StartsWith("\x2B\x2F\x76", "UTF-7")
+.StartsWith("\xF7\x64\x4C", "UTF-1")

sammccall wrote:
> Seems unlikely we'll ever see any of these other than UTF{16,32}.
> I'd suggest dropping them, but up to you.
I will keep the rare BOM cases to keep it in sync with D61628.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61559



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

[PATCH] D61559: Fix the crash when formatting unsupported encodings

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL360257: [clang-format] Fix the crash when formatting 
unsupported encodings (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61559?vs=198652&id=198655#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61559

Files:
  cfe/trunk/tools/clang-format/ClangFormat.cpp


Index: cfe/trunk/tools/clang-format/ClangFormat.cpp
===
--- cfe/trunk/tools/clang-format/ClangFormat.cpp
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp
@@ -257,6 +257,36 @@
   std::unique_ptr Code = std::move(CodeOrErr.get());
   if (Code->getBufferSize() == 0)
 return false; // Empty files are formatted correctly.
+
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  StringRef BufStr = Code->getBuffer();
+  const char *InvalidBOM = llvm::StringSwitch(BufStr)
+.StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+.StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
+.StartsWith("\x2B\x2F\x76", "UTF-7")
+.StartsWith("\xF7\x64\x4C", "UTF-1")
+.StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+.StartsWith("\x0E\xFE\xFF", "SCSU")
+.StartsWith("\xFB\xEE\x28", "BOCU-1")
+.StartsWith("\x84\x31\x95\x33", "GB-18030")
+.Default(nullptr);
+
+  if (InvalidBOM) {
+errs() << "error: encoding with unsupported byte order mark \""
+   << InvalidBOM << "\" detected";
+if (FileName != "-")
+  errs() << " in file '" << FileName << "'";
+errs() << ".\n";
+return true;
+  }
+
   std::vector Ranges;
   if (fillRanges(Code.get(), Ranges))
 return true;


Index: cfe/trunk/tools/clang-format/ClangFormat.cpp
===
--- cfe/trunk/tools/clang-format/ClangFormat.cpp
+++ cfe/trunk/tools/clang-format/ClangFormat.cpp
@@ -257,6 +257,36 @@
   std::unique_ptr Code = std::move(CodeOrErr.get());
   if (Code->getBufferSize() == 0)
 return false; // Empty files are formatted correctly.
+
+  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
+  // We only support UTF-8 with and without a BOM right now.  See
+  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
+  // for more information.
+  StringRef BufStr = Code->getBuffer();
+  const char *InvalidBOM = llvm::StringSwitch(BufStr)
+.StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
+  "UTF-32 (BE)")
+.StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
+  "UTF-32 (LE)")
+.StartsWith("\xFE\xFF", "UTF-16 (BE)")
+.StartsWith("\xFF\xFE", "UTF-16 (LE)")
+.StartsWith("\x2B\x2F\x76", "UTF-7")
+.StartsWith("\xF7\x64\x4C", "UTF-1")
+.StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
+.StartsWith("\x0E\xFE\xFF", "SCSU")
+.StartsWith("\xFB\xEE\x28", "BOCU-1")
+.StartsWith("\x84\x31\x95\x33", "GB-18030")
+.Default(nullptr);
+
+  if (InvalidBOM) {
+errs() << "error: encoding with unsupported byte order mark \""
+   << InvalidBOM << "\" detected";
+if (FileName != "-")
+  errs() << " in file '" << FileName << "'";
+errs() << ".\n";
+return true;
+  }
+
   std::vector Ranges;
   if (fillRanges(Code.get(), Ranges))
 return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://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 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.

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 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] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

[Owen Pan via Phabricator via cfe-commits]

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D52527

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1069,6 +1069,7 @@
   Style.IndentCaseLabels = true;
   Style.AllowShortBlocksOnASingleLine = false;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = true;
   EXPECT_EQ("switch (n)\n"
 "{\n"
@@ -1090,6 +1091,27 @@
"  }\n"
"}",
Style));
+  Style.BraceWrapping.AfterCaseLabel = false;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0: {\n"
+"return false;\n"
+"  }\n"
+"  default: {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0:\n"
+   "  {\n"
+   "return false;\n"
+   "  }\n"
+   "  default:\n"
+   "  {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -1243,6 +1265,7 @@
Style));
   Style.AllowShortCaseLabelsOnASingleLine = true;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = true;
   EXPECT_EQ("switch (n)\n"
 "{\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -173,10 +173,16 @@
 public:
   CompoundStatementIndenter(UnwrappedLineParser *Parser,
 const FormatStyle &Style, unsigned &LineLevel)
+  : CompoundStatementIndenter(Parser, LineLevel,
+  Style.BraceWrapping.AfterControlStatement,
+  Style.BraceWrapping.IndentBraces) {
+  }
+  CompoundStatementIndenter(UnwrappedLineParser *Parser, unsigned &LineLevel,
+bool WrapeBrace, bool IndentBrace)
   : LineLevel(LineLevel), OldLineLevel(LineLevel) {
-if (Style.BraceWrapping.AfterControlStatement)
+if (WrapeBrace)
   Parser->addUnwrappedLine();
-if (Style.BraceWrapping.IndentBraces)
+if (IndentBrace)
   ++LineLevel;
   }
   ~CompoundStatementIndenter() { LineLevel = OldLineLevel; }
@@ -1888,7 +1894,9 @@
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
 --Line->Level;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
-CompoundStatementIndenter Indenter(this, Style, Line->Level);
+CompoundStatementIndenter Indenter(this, Line->Level,
+   Style.BraceWrapping.AfterCaseLabel,
+   Style.BraceWrapping.IndentBraces);
 parseBlock(/*MustBeDeclaration=*/false);
 if (FormatTok->Tok.is(tok::kw_break)) {
   if (Style.BraceWrapping.AfterControlStatement)
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -476,6 +476,7 @@
 
 template <> struct MappingTraits {
   static void mapping(IO &IO, FormatStyle::BraceWrappingFlags &Wrapping) {
+IO.mapOptional("AfterCaseLabel", Wrapping.AfterCaseLabel);
 IO.mapOptional("AfterClass", Wrapping.AfterClass);
 IO.mapOptional("AfterControlStatement", Wrapping.AfterControlStatement);
 IO.mapOptional("AfterEnum", Wrapping.AfterEnum);
@@ -568,7 +569,7 @@
   if (Style.BreakBeforeBraces == FormatStyle::BS_Custom)
 return Style;
   FormatStyle Expanded = Style;
-  Expanded.BraceWrapping = {false, false, false, false, false,
+  Expanded.BraceWrapping = {false, false, false, false, false, false,
 false, false, false, false, false,
 false, false, true,  true,  true};
   switch (Style.BreakBeforeBraces) {
@@ -593,6 +594,7 @@
 Expanded.BraceWrapping.BeforeElse = true;
 break;
   case FormatStyle::BS_Allman:
+Expanded.BraceWrapping.AfterCaseLabel = true;
 Expanded.BraceWrapping.AfterClass = true;
 Expanded.BraceWrapping.AfterControlStatement = true;
 Expanded.BraceWrapping.AfterEnum = true;
@@ -606,7 +608,7 @@
 break;
   case FormatStyle::BS_GNU:
 Expanded.BraceWrapping = {true, true, true, true, true, true, true, true,
-   

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

[Owen Pan via Phabricator via cfe-commits]

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

Updated ClangFormatStyleOptions.rst.


Repository:
  rC Clang

https://reviews.llvm.org/D52527

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1069,6 +1069,7 @@
   Style.IndentCaseLabels = true;
   Style.AllowShortBlocksOnASingleLine = false;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = true;
   EXPECT_EQ("switch (n)\n"
 "{\n"
@@ -1090,6 +1091,27 @@
"  }\n"
"}",
Style));
+  Style.BraceWrapping.AfterCaseLabel = false;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0: {\n"
+"return false;\n"
+"  }\n"
+"  default: {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0:\n"
+   "  {\n"
+   "return false;\n"
+   "  }\n"
+   "  default:\n"
+   "  {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, CaseRanges) {
@@ -1243,6 +1265,7 @@
Style));
   Style.AllowShortCaseLabelsOnASingleLine = true;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = true;
   EXPECT_EQ("switch (n)\n"
 "{\n"
@@ -10845,6 +10868,7 @@
   CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
   CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
 
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterCaseLabel);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterControlStatement);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterEnum);
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -173,10 +173,16 @@
 public:
   CompoundStatementIndenter(UnwrappedLineParser *Parser,
 const FormatStyle &Style, unsigned &LineLevel)
+  : CompoundStatementIndenter(Parser, LineLevel,
+  Style.BraceWrapping.AfterControlStatement,
+  Style.BraceWrapping.IndentBraces) {
+  }
+  CompoundStatementIndenter(UnwrappedLineParser *Parser, unsigned &LineLevel,
+bool WrapeBrace, bool IndentBrace)
   : LineLevel(LineLevel), OldLineLevel(LineLevel) {
-if (Style.BraceWrapping.AfterControlStatement)
+if (WrapeBrace)
   Parser->addUnwrappedLine();
-if (Style.BraceWrapping.IndentBraces)
+if (IndentBrace)
   ++LineLevel;
   }
   ~CompoundStatementIndenter() { LineLevel = OldLineLevel; }
@@ -1888,7 +1894,9 @@
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
 --Line->Level;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
-CompoundStatementIndenter Indenter(this, Style, Line->Level);
+CompoundStatementIndenter Indenter(this, Line->Level,
+   Style.BraceWrapping.AfterCaseLabel,
+   Style.BraceWrapping.IndentBraces);
 parseBlock(/*MustBeDeclaration=*/false);
 if (FormatTok->Tok.is(tok::kw_break)) {
   if (Style.BraceWrapping.AfterControlStatement)
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -476,6 +476,7 @@
 
 template <> struct MappingTraits {
   static void mapping(IO &IO, FormatStyle::BraceWrappingFlags &Wrapping) {
+IO.mapOptional("AfterCaseLabel", Wrapping.AfterCaseLabel);
 IO.mapOptional("AfterClass", Wrapping.AfterClass);
 IO.mapOptional("AfterControlStatement", Wrapping.AfterControlStatement);
 IO.mapOptional("AfterEnum", Wrapping.AfterEnum);
@@ -568,7 +569,7 @@
   if (Style.BreakBeforeBraces == FormatStyle::BS_Custom)
 return Style;
   FormatStyle Expanded = Style;
-  Expanded.BraceWrapping = {false, false, false, false, false,
+  Expanded.BraceWrapping = {false, false, false, false, false, false,
 false, false, false, false, false,
 false, false, true,  true,  true};
   switch (Style.BreakBeforeBraces) {
@@ -593,6 +594,7 @@
 Expanded.BraceWrapping.BeforeElse = true;
 break;
   case FormatStyle::BS_Allman:
+Expanded.BraceWrap

[PATCH] D52591: Fix Bug 39067: The keyword "try" in C++ function-try-blocks doesn't obey BraceWrapping settings

[Owen Pan via Phabricator via cfe-commits]

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D52591

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2628,6 +2628,16 @@
"  A(X x)\n"
"  try : t(0) {} catch (...) {}\n"
"};"));
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterFunction = true;
+  EXPECT_EQ("void f()\n"
+"try\n"
+"{\n"
+"}",
+format("void f() try {\n"
+   "}", Style));
   EXPECT_EQ("class SomeClass {\n"
 "public:\n"
 "  SomeClass() EXCLUSIVE_LOCK_FUNCTION(mu_);\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1266,6 +1266,8 @@
   break;
 case tok::kw_try:
   // We arrive here when parsing function-try blocks.
+  if (Style.BraceWrapping.AfterFunction)
+addUnwrappedLine();
   parseTryCatch();
   return;
 case tok::identifier: {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2628,6 +2628,16 @@
"  A(X x)\n"
"  try : t(0) {} catch (...) {}\n"
"};"));
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterFunction = true;
+  EXPECT_EQ("void f()\n"
+"try\n"
+"{\n"
+"}",
+format("void f() try {\n"
+   "}", Style));
   EXPECT_EQ("class SomeClass {\n"
 "public:\n"
 "  SomeClass() EXCLUSIVE_LOCK_FUNCTION(mu_);\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1266,6 +1266,8 @@
   break;
 case tok::kw_try:
   // We arrive here when parsing function-try blocks.
+  if (Style.BraceWrapping.AfterFunction)
+addUnwrappedLine();
   parseTryCatch();
   return;
 case tok::identifier: {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52591: Fix Bug 39067: The keyword "try" in C++ function-try-blocks doesn't obey BraceWrapping settings

[Owen Pan via Phabricator via cfe-commits]

owenpan abandoned this revision.
owenpan added a comment.

Closed by r343305


Repository:
  rC Clang

https://reviews.llvm.org/D52591



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

[PATCH] D50403: [clang-format]AlignConsecutiveAssignments

[Owen Pan via Phabricator via cfe-commits]

owenpan requested changes to this revision.
owenpan added inline comments.
This revision now requires changes to proceed.



Comment at: include/clang/Format/Format.h:91-93
+  ///   int ddd += 12;
+  ///   int ee  += 22;
+  ///   int f   += 23;

These are invalid C++ examples.



Comment at: lib/Format/WhitespaceManager.cpp:435-451
+  std::vector assignment_tokens =
+{tok::equal, tok::pipeequal, tok::caretequal, tok::percentequal,
+ tok::ampequal, tok::plusequal, tok::minusequal, tok::starequal,
+ tok::slashequal, tok::lesslessequal, tok::greatergreaterequal};
+  for (auto assignment_token : assignment_tokens)
+  {
+AlignTokens(Style,

It would be simpler to just use `isOneOf(tok::equal, tok::pipeequal, ...)` here.


Repository:
  rC Clang

https://reviews.llvm.org/D50403



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

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

I'd greatly appreciate it if someone could review this before I commit it next 
week.


Repository:
  rC Clang

https://reviews.llvm.org/D52527



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

[PATCH] D61729: [docs] Fix example for Allman brace breaking style

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

This file is generated from the `clang/include/clang/Format/Format.h` header 
file by the `clang/docs/tools/dump_format_style.py` script. Please update the 
header file and rerun the script.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61729



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

[PATCH] D61729: [docs] Fix example for Allman brace breaking style

[Owen Pan via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D61729



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

[PATCH] D65631: [clang-format] Fix a bug that doesn't break braces before unions for Allman

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes PR42155


Repository:
  rC Clang

https://reviews.llvm.org/D65631

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11086,6 +11086,9 @@
"{\n"
"  int x;\n"
"};\n"
+   "union C\n"
+   "{\n"
+   "};\n"
"} // namespace a",
AllmanBraceStyle);
 
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -640,6 +640,7 @@
 Expanded.BraceWrapping.AfterNamespace = true;
 Expanded.BraceWrapping.AfterObjCDeclaration = true;
 Expanded.BraceWrapping.AfterStruct = true;
+Expanded.BraceWrapping.AfterUnion = true;
 Expanded.BraceWrapping.AfterExternBlock = true;
 Expanded.BraceWrapping.BeforeCatch = true;
 Expanded.BraceWrapping.BeforeElse = true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11086,6 +11086,9 @@
"{\n"
"  int x;\n"
"};\n"
+   "union C\n"
+   "{\n"
+   "};\n"
"} // namespace a",
AllmanBraceStyle);
 
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -640,6 +640,7 @@
 Expanded.BraceWrapping.AfterNamespace = true;
 Expanded.BraceWrapping.AfterObjCDeclaration = true;
 Expanded.BraceWrapping.AfterStruct = true;
+Expanded.BraceWrapping.AfterUnion = true;
 Expanded.BraceWrapping.AfterExternBlock = true;
 Expanded.BraceWrapping.BeforeCatch = true;
 Expanded.BraceWrapping.BeforeElse = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65631: [clang-format] Fix a bug that doesn't break braces before unions for Allman

[Owen Pan via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367648: [clang-format] Fix a bug that doesn't break 
braces before unions for Allman (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65631?vs=212964&id=212966#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65631

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


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -640,6 +640,7 @@
 Expanded.BraceWrapping.AfterNamespace = true;
 Expanded.BraceWrapping.AfterObjCDeclaration = true;
 Expanded.BraceWrapping.AfterStruct = true;
+Expanded.BraceWrapping.AfterUnion = true;
 Expanded.BraceWrapping.AfterExternBlock = true;
 Expanded.BraceWrapping.BeforeCatch = true;
 Expanded.BraceWrapping.BeforeElse = true;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11086,6 +11086,9 @@
"{\n"
"  int x;\n"
"};\n"
+   "union C\n"
+   "{\n"
+   "};\n"
"} // namespace a",
AllmanBraceStyle);
 


Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -640,6 +640,7 @@
 Expanded.BraceWrapping.AfterNamespace = true;
 Expanded.BraceWrapping.AfterObjCDeclaration = true;
 Expanded.BraceWrapping.AfterStruct = true;
+Expanded.BraceWrapping.AfterUnion = true;
 Expanded.BraceWrapping.AfterExternBlock = true;
 Expanded.BraceWrapping.BeforeCatch = true;
 Expanded.BraceWrapping.BeforeElse = true;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11086,6 +11086,9 @@
"{\n"
"  int x;\n"
"};\n"
+   "union C\n"
+   "{\n"
+   "};\n"
"} // namespace a",
AllmanBraceStyle);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65925: Add SpaceInEmptyBlock option for WebKit

[Owen Pan via Phabricator via cfe-commits]

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

See PR40840


Repository:
  rC Clang

https://reviews.llvm.org/D65925

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  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
@@ -3683,6 +3683,11 @@
   EXPECT_EQ("{}", format("{}"));
   verifyFormat("enum E {};");
   verifyFormat("enum E {}");
+  EXPECT_EQ("void f() { }", format("void f() {}", getWebKitStyle()));
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.SpaceInEmptyBlock = true;
+  EXPECT_EQ("while (true) { }", format("while (true) {}", Style));
 }
 
 TEST_F(FormatTest, FormatBeginBlockEndMacros) {
@@ -11756,6 +11761,7 @@
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInAngles);
+  CHECK_PARSE_BOOL(SpaceInEmptyBlock);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -551,7 +551,7 @@
   (Tok->getNextNonComment() == nullptr ||
Tok->getNextNonComment()->is(tok::semi))) {
 // We merge empty blocks even if the line exceeds the column limit.
-Tok->SpacesRequiredBefore = 0;
+Tok->SpacesRequiredBefore = Style.SpaceInEmptyBlock ? 1 : 0;
 Tok->CanBreakBefore = true;
 return 1;
   } else if (Limit != 0 && !Line.startsWithNamespace() &&
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -494,6 +494,7 @@
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Style.SpaceBeforeRangeBasedForLoopColon);
+IO.mapOptional("SpaceInEmptyBlock", Style.SpaceInEmptyBlock);
 IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses);
 IO.mapOptional("SpacesBeforeTrailingComments",
Style.SpacesBeforeTrailingComments);
@@ -734,6 +735,7 @@
   LLVMStyle.ReflowComments = true;
   LLVMStyle.SpacesInParentheses = false;
   LLVMStyle.SpacesInSquareBrackets = false;
+  LLVMStyle.SpaceInEmptyBlock = false;
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
@@ -979,6 +981,7 @@
   Style.ObjCSpaceAfterProperty = true;
   Style.PointerAlignment = FormatStyle::PAS_Left;
   Style.SpaceBeforeCpp11BracedList = true;
+  Style.SpaceInEmptyBlock = true;
   return Style;
 }
 
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1799,6 +1799,14 @@
   /// \endcode
   bool SpaceBeforeRangeBasedForLoopColon;
 
+  /// If ``true``, spaces may be inserted into ``{}``.
+  /// \code
+  ///true:false:
+  ///void f() { }   vs.   void f() {}
+  ///while (true) { } while (true) {}
+  /// \endcode
+  bool SpaceInEmptyBlock;
+
   /// If ``true``, spaces may be inserted into ``()``.
   /// \code
   ///true:false:
@@ -1995,6 +2003,7 @@
SpaceBeforeParens == R.SpaceBeforeParens &&
SpaceBeforeRangeBasedForLoopColon ==
R.SpaceBeforeRangeBasedForLoopColon &&
+   SpaceInEmptyBlock == R.SpaceInEmptyBlock &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&
SpacesInAngles == R.SpacesInAngles &&
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -192,20 +192,6 @@
 
 
 
-**AlignConsecutiveMacros** (``bool``)
-  If ``true``, aligns consecutive C/C++ preprocessor macros.
-
-  This will align the C/C++ preprocessor macros of consecutive lines. This
-  will result in formattings like
-
-  .. code-block:: c++
-
-#define SHORT_NAME   42
-#define LONGER_NAME  0x007f
-#define EVEN_LONGER_NAME (2)
-#define foo(x)   (x * x)
-#define bar(y, z)(y + z)
-
 *

[PATCH] D65925: [clang-format] Add SpaceInEmptyBlock option for WebKit

[Owen Pan via Phabricator via cfe-commits]

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

Changed "may" to "will" and updated the diff. (Should we do the same for the 
SpaceInEmptyParentheses option?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D65925

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  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
@@ -3683,6 +3683,11 @@
   EXPECT_EQ("{}", format("{}"));
   verifyFormat("enum E {};");
   verifyFormat("enum E {}");
+  EXPECT_EQ("void f() { }", format("void f() {}", getWebKitStyle()));
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.SpaceInEmptyBlock = true;
+  EXPECT_EQ("while (true) { }", format("while (true) {}", Style));
 }
 
 TEST_F(FormatTest, FormatBeginBlockEndMacros) {
@@ -11756,6 +11761,7 @@
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInAngles);
+  CHECK_PARSE_BOOL(SpaceInEmptyBlock);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -551,7 +551,7 @@
   (Tok->getNextNonComment() == nullptr ||
Tok->getNextNonComment()->is(tok::semi))) {
 // We merge empty blocks even if the line exceeds the column limit.
-Tok->SpacesRequiredBefore = 0;
+Tok->SpacesRequiredBefore = Style.SpaceInEmptyBlock ? 1 : 0;
 Tok->CanBreakBefore = true;
 return 1;
   } else if (Limit != 0 && !Line.startsWithNamespace() &&
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -494,6 +494,7 @@
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Style.SpaceBeforeRangeBasedForLoopColon);
+IO.mapOptional("SpaceInEmptyBlock", Style.SpaceInEmptyBlock);
 IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses);
 IO.mapOptional("SpacesBeforeTrailingComments",
Style.SpacesBeforeTrailingComments);
@@ -734,6 +735,7 @@
   LLVMStyle.ReflowComments = true;
   LLVMStyle.SpacesInParentheses = false;
   LLVMStyle.SpacesInSquareBrackets = false;
+  LLVMStyle.SpaceInEmptyBlock = false;
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
@@ -979,6 +981,7 @@
   Style.ObjCSpaceAfterProperty = true;
   Style.PointerAlignment = FormatStyle::PAS_Left;
   Style.SpaceBeforeCpp11BracedList = true;
+  Style.SpaceInEmptyBlock = true;
   return Style;
 }
 
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1799,6 +1799,14 @@
   /// \endcode
   bool SpaceBeforeRangeBasedForLoopColon;
 
+  /// If ``true``, spaces will be inserted into ``{}``.
+  /// \code
+  ///true:false:
+  ///void f() { }   vs.   void f() {}
+  ///while (true) { } while (true) {}
+  /// \endcode
+  bool SpaceInEmptyBlock;
+
   /// If ``true``, spaces may be inserted into ``()``.
   /// \code
   ///true:false:
@@ -1995,6 +2003,7 @@
SpaceBeforeParens == R.SpaceBeforeParens &&
SpaceBeforeRangeBasedForLoopColon ==
R.SpaceBeforeRangeBasedForLoopColon &&
+   SpaceInEmptyBlock == R.SpaceInEmptyBlock &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&
SpacesInAngles == R.SpacesInAngles &&
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -192,20 +192,6 @@
 
 
 
-**AlignConsecutiveMacros** (``bool``)
-  If ``true``, aligns consecutive C/C++ preprocessor macros.
-
-  This will align the C/C++ preprocessor macros of consecutive lines. This
-  will result in formattings like
-
-  .. code-block:: c++
-
-#define SHORT_NAME   42
-#define LONGER_NAME  0x007f
-#define EVEN_LONGER_NAME (2)
-#define f

[PATCH] D65925: [clang-format] Add SpaceInEmptyBlock option for WebKit

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D65925#1623965 , @thakis wrote:

> https://webkit.org/code-style-guidelines/ has one instance of `{ }` with a 
> space and one of `{}` without a space, and as far as I can tell no text that 
> says which instance is correct. Are you sure this is the right thing for 
> webkit style?


The one without a space is an example of "Wrong" WebKit style there.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65925



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

[PATCH] D65925: [clang-format] Add SpaceInEmptyBlock option for WebKit

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368507: [clang-format] Add SpaceInEmptyBlock option for 
WebKit (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65925?vs=214509&id=214511#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65925

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  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
@@ -551,7 +551,7 @@
   (Tok->getNextNonComment() == nullptr ||
Tok->getNextNonComment()->is(tok::semi))) {
 // We merge empty blocks even if the line exceeds the column limit.
-Tok->SpacesRequiredBefore = 0;
+Tok->SpacesRequiredBefore = Style.SpaceInEmptyBlock ? 1 : 0;
 Tok->CanBreakBefore = true;
 return 1;
   } else if (Limit != 0 && !Line.startsWithNamespace() &&
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -494,6 +494,7 @@
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Style.SpaceBeforeRangeBasedForLoopColon);
+IO.mapOptional("SpaceInEmptyBlock", Style.SpaceInEmptyBlock);
 IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses);
 IO.mapOptional("SpacesBeforeTrailingComments",
Style.SpacesBeforeTrailingComments);
@@ -734,6 +735,7 @@
   LLVMStyle.ReflowComments = true;
   LLVMStyle.SpacesInParentheses = false;
   LLVMStyle.SpacesInSquareBrackets = false;
+  LLVMStyle.SpaceInEmptyBlock = false;
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
@@ -979,6 +981,7 @@
   Style.ObjCSpaceAfterProperty = true;
   Style.PointerAlignment = FormatStyle::PAS_Left;
   Style.SpaceBeforeCpp11BracedList = true;
+  Style.SpaceInEmptyBlock = true;
   return Style;
 }
 
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -3692,6 +3692,11 @@
   EXPECT_EQ("{}", format("{}"));
   verifyFormat("enum E {};");
   verifyFormat("enum E {}");
+  EXPECT_EQ("void f() { }", format("void f() {}", getWebKitStyle()));
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.SpaceInEmptyBlock = true;
+  EXPECT_EQ("while (true) { }", format("while (true) {}", Style));
 }
 
 TEST_F(FormatTest, FormatBeginBlockEndMacros) {
@@ -11765,6 +11770,7 @@
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInAngles);
+  CHECK_PARSE_BOOL(SpaceInEmptyBlock);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
Index: cfe/trunk/docs/ClangFormatStyleOptions.rst
===
--- cfe/trunk/docs/ClangFormatStyleOptions.rst
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst
@@ -192,20 +192,6 @@
 
 
 
-**AlignConsecutiveMacros** (``bool``)
-  If ``true``, aligns consecutive C/C++ preprocessor macros.
-
-  This will align the C/C++ preprocessor macros of consecutive lines. This
-  will result in formattings like
-
-  .. code-block:: c++
-
-#define SHORT_NAME   42
-#define LONGER_NAME  0x007f
-#define EVEN_LONGER_NAME (2)
-#define foo(x)   (x * x)
-#define bar(y, z)(y + z)
-
 **AlignConsecutiveAssignments** (``bool``)
   If ``true``, aligns consecutive assignments.
 
@@ -230,6 +216,20 @@
 float   b = 23;
 std::string ccc = 23;
 
+**AlignConsecutiveMacros** (``bool``)
+  If ``true``, aligns consecutive C/C++ preprocessor macros.
+
+  This will align C/C++ preprocessor macros of consecutive lines.
+  Will result in formattings like
+
+  .. code-block:: c++
+
+#define SHORT_NAME   42
+#define LONGER_NAME  0x007f
+#define EVEN_LONGER_NAME (2)
+#define foo(x)   (x * x)
+#define bar(y, z)(y + z)
+
 **AlignEscapedNewlines** (``EscapedNewlineAlignmentStyle``)
   Options for aligning backslashes in escaped newlines.
 
@@ -1390,24 +1390,6 @@
 
   For example: BOOST_FOREACH.
 
-**TypenameMacros** (``std::vector``)
-  A vector of macros that 

[PATCH] D66059: [clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

[Owen Pan via Phabricator via cfe-commits]

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

See PR40840


Repository:
  rC Clang

https://reviews.llvm.org/D66059

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  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
@@ -561,7 +561,8 @@
 TEST_F(FormatTest, FormatShortBracedStatements) {
   FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
   AllowSimpleBracedStatements.ColumnLimit = 40;
-  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine =
+  FormatStyle::SBS_Always;
 
   AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
   FormatStyle::SIS_WithoutElse;
@@ -714,7 +715,7 @@
 
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
   FormatStyle Style = getLLVMStyleWithColumns(60);
-  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;
   Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A  \\\n"
@@ -1163,7 +1164,7 @@
 
   FormatStyle Style = getLLVMStyle();
   Style.IndentCaseLabels = true;
-  Style.AllowShortBlocksOnASingleLine = false;
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = true;
@@ -3692,10 +3693,10 @@
   EXPECT_EQ("{}", format("{}"));
   verifyFormat("enum E {};");
   verifyFormat("enum E {}");
-  EXPECT_EQ("void f() { }", format("void f() {}", getWebKitStyle()));
   FormatStyle Style = getLLVMStyle();
-  Style.AllowShortBlocksOnASingleLine = true;
   Style.SpaceInEmptyBlock = true;
+  EXPECT_EQ("void f() { }", format("void f() {}", Style));
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
   EXPECT_EQ("while (true) { }", format("while (true) {}", Style));
 }
 
@@ -11745,7 +11746,6 @@
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllConstructorInitializersOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
-  CHECK_PARSE_BOOL(AllowShortBlocksOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
@@ -11920,6 +11920,19 @@
   CHECK_PARSE("UseTab: false", UseTab, FormatStyle::UT_Never);
   CHECK_PARSE("UseTab: true", UseTab, FormatStyle::UT_Always);
 
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: Never",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Never);
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: Empty",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Empty);
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: Always",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Always);
+  // For backward compatibility:
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: false",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Never);
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: true",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Always);
+
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: None",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
@@ -12527,6 +12540,14 @@
   // Allow functions on a single line.
   verifyFormat("void f() { return; }", Style);
 
+  // Allow empty blocks on a single line and insert a space in empty blocks.
+  EXPECT_EQ("void f() { }", format("void f() {}", Style));
+  EXPECT_EQ("while (true) { }", format("while (true) {}", Style));
+  // However, don't merge non-empty short loops.
+  EXPECT_EQ("while (true) {\n"
+"continue;\n"
+"}", format("while (true) { continue; }", Style));
+
   // Constructor initializers are formatted one per line with the "," on the
   // new line.
   verifyFormat("Constructor()\n"
@@ -13033,7 +13054,7 @@
 
 TEST_F(FormatTest, FormatsBlocks) {
   FormatStyle ShortBlocks = getLLVMStyle();
-  ShortBlocks.AllowShortBlocksOnASingleLine = true;
+  ShortBlocks.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;
   verifyFormat("int (^Block)(int, int);", ShortBlocks);
   verifyFormat("int (^Block1)(int, int) = ^(int i, int j)", ShortBlocks);
   verifyFormat("void (^block)(int) = ^(id test)

[PATCH] D66059: [clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368539: [clang-format] Expand AllowShortBlocksOnASingleLine 
for WebKit (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66059?vs=214547&id=214560#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66059

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -561,7 +561,8 @@
 TEST_F(FormatTest, FormatShortBracedStatements) {
   FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
   AllowSimpleBracedStatements.ColumnLimit = 40;
-  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine =
+  FormatStyle::SBS_Always;
 
   AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
   FormatStyle::SIS_WithoutElse;
@@ -714,7 +715,7 @@
 
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
   FormatStyle Style = getLLVMStyleWithColumns(60);
-  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;
   Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A  \\\n"
@@ -1163,7 +1164,7 @@
 
   FormatStyle Style = getLLVMStyle();
   Style.IndentCaseLabels = true;
-  Style.AllowShortBlocksOnASingleLine = false;
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = true;
@@ -3692,10 +3693,10 @@
   EXPECT_EQ("{}", format("{}"));
   verifyFormat("enum E {};");
   verifyFormat("enum E {}");
-  EXPECT_EQ("void f() { }", format("void f() {}", getWebKitStyle()));
   FormatStyle Style = getLLVMStyle();
-  Style.AllowShortBlocksOnASingleLine = true;
   Style.SpaceInEmptyBlock = true;
+  EXPECT_EQ("void f() { }", format("void f() {}", Style));
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
   EXPECT_EQ("while (true) { }", format("while (true) {}", Style));
 }
 
@@ -11745,7 +11746,6 @@
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllConstructorInitializersOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
-  CHECK_PARSE_BOOL(AllowShortBlocksOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
@@ -11920,6 +11920,19 @@
   CHECK_PARSE("UseTab: false", UseTab, FormatStyle::UT_Never);
   CHECK_PARSE("UseTab: true", UseTab, FormatStyle::UT_Always);
 
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: Never",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Never);
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: Empty",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Empty);
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: Always",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Always);
+  // For backward compatibility:
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: false",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Never);
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: true",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Always);
+
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: None",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
@@ -12527,6 +12540,14 @@
   // Allow functions on a single line.
   verifyFormat("void f() { return; }", Style);
 
+  // Allow empty blocks on a single line and insert a space in empty blocks.
+  EXPECT_EQ("void f() { }", format("void f() {}", Style));
+  EXPECT_EQ("while (true) { }", format("while (true) {}", Style));
+  // However, don't merge non-empty short loops.
+  EXPECT_EQ("while (true) {\n"
+"continue;\n"
+"}", format("while (true) { continue; }", Style));
+
   // Constructor initializers are formatted one per line with the "," on the
   // new line.
   verifyFormat("Constructor()\n"
@@ -13033,7 +13054,7 @@
 
 TEST_F(FormatTest, FormatsBlocks) {
   FormatStyle ShortBlocks = getLLVMStyle();
-  ShortBlocks.AllowShortBlocks

[PATCH] D66332: [clang-format] Fix the bug that joins template closer and > or >>

[Owen Pan via Phabricator via cfe-commits]

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

Also fixes a buggy test case.

See PR42404


Repository:
  rC Clang

https://reviews.llvm.org/D66332

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6618,7 +6618,10 @@
   EXPECT_EQ("auto x = [] { A>> a; };",
 format("auto x=[]{A >> a;};", getGoogleStyle()));
 
-  verifyFormat("A> a;", getChromiumStyle(FormatStyle::LK_Cpp));
+  verifyFormat("A> a;", getChromiumStyle(FormatStyle::LK_Cpp));
+
+  verifyFormat("int i = a<1> >> 1;");
+  verifyFormat("bool b = a<1> > 1;");
 
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -919,6 +919,8 @@
 case tok::greater:
   if (Style.Language != FormatStyle::LK_TextProto)
 Tok->Type = TT_BinaryOperator;
+  if (Tok->Previous && Tok->Previous->is(TT_TemplateCloser))
+Tok->SpacesRequiredBefore = 1;
   break;
 case tok::kw_operator:
   if (Style.Language == FormatStyle::LK_TextProto ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6618,7 +6618,10 @@
   EXPECT_EQ("auto x = [] { A>> a; };",
 format("auto x=[]{A >> a;};", getGoogleStyle()));
 
-  verifyFormat("A> a;", getChromiumStyle(FormatStyle::LK_Cpp));
+  verifyFormat("A> a;", getChromiumStyle(FormatStyle::LK_Cpp));
+
+  verifyFormat("int i = a<1> >> 1;");
+  verifyFormat("bool b = a<1> > 1;");
 
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -919,6 +919,8 @@
 case tok::greater:
   if (Style.Language != FormatStyle::LK_TextProto)
 Tok->Type = TT_BinaryOperator;
+  if (Tok->Previous && Tok->Previous->is(TT_TemplateCloser))
+Tok->SpacesRequiredBefore = 1;
   break;
 case tok::kw_operator:
   if (Style.Language == FormatStyle::LK_TextProto ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66332: [clang-format] Fix the bug that joins template closer and > or >>

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D66332#1633448 , @Quuxplusone wrote:

> Drive-by observation: My experiments with 
> https://zed0.co.uk/clang-format-configurator/ show that there is a similar 
> bug where clang-format accidentally produces `>=` via reformatting of 
> `template` `=0>`, `pi` `=3;`, and so on. Is it 
> possible to fix that bug as part of this patch, and/or would you be 
> interested in patching that bug as a follow-up to this one?


Do you have `SpaceBeforeAssignmentOperators` off? I am more than happy to fix 
it as a follow-up, but the current behavior of `SpaceBeforeAssignmentOperators` 
is:
`true`:

  int a = 5;
  a += 42;

`false`:

  int a= 5;
  a+= 42;

It's weird that there is a space after the assignment operators regardless.

The documentation 

 should be fixed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66332



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

[PATCH] D66332: [clang-format] Fix the bug that joins template closer and > or >>

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGac67414618df: [clang-format] Fix the bug that joins template 
closer and > or >> (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66332

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6618,7 +6618,10 @@
   EXPECT_EQ("auto x = [] { A>> a; };",
 format("auto x=[]{A >> a;};", getGoogleStyle()));
 
-  verifyFormat("A> a;", getChromiumStyle(FormatStyle::LK_Cpp));
+  verifyFormat("A> a;", getChromiumStyle(FormatStyle::LK_Cpp));
+
+  verifyFormat("int i = a<1> >> 1;");
+  verifyFormat("bool b = a<1> > 1;");
 
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -919,6 +919,8 @@
 case tok::greater:
   if (Style.Language != FormatStyle::LK_TextProto)
 Tok->Type = TT_BinaryOperator;
+  if (Tok->Previous && Tok->Previous->is(TT_TemplateCloser))
+Tok->SpacesRequiredBefore = 1;
   break;
 case tok::kw_operator:
   if (Style.Language == FormatStyle::LK_TextProto ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6618,7 +6618,10 @@
   EXPECT_EQ("auto x = [] { A>> a; };",
 format("auto x=[]{A >> a;};", getGoogleStyle()));
 
-  verifyFormat("A> a;", getChromiumStyle(FormatStyle::LK_Cpp));
+  verifyFormat("A> a;", getChromiumStyle(FormatStyle::LK_Cpp));
+
+  verifyFormat("int i = a<1> >> 1;");
+  verifyFormat("bool b = a<1> > 1;");
 
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -919,6 +919,8 @@
 case tok::greater:
   if (Style.Language != FormatStyle::LK_TextProto)
 Tok->Type = TT_BinaryOperator;
+  if (Tok->Previous && Tok->Previous->is(TT_TemplateCloser))
+Tok->SpacesRequiredBefore = 1;
   break;
 case tok::kw_operator:
   if (Style.Language == FormatStyle::LK_TextProto ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66332: [clang-format] Fix the bug that joins template closer and > or >>

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

> Are there any other clang-format options that might lead to a lack-of-space 
> before `>`, `>=`, `==`, or `=`? I brought up `enable_if_t=0` 
> because that specifically is a construction I've run into in my own code, 
> even though I've never tried to clang-format it, let alone have it formatted 
> incorrectly.

Any token that starts with `>`, e.g., `>`, `>>`, `>=`, and `>>=`, are already 
taken care of by this patch. For tokens starting with `=`, only the assignment 
operator `=` has a problem and it only occurs when 
`SpaceBeforeAssignmentOperators` is set to true.

I can insert a space between a template closer `>` and an assignment operator 
`=` despite the value of `SpaceBeforeAssignmentOperators`, but the formatted 
code may be inconsistent...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66332



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

[PATCH] D66384: [clang-format] Fix a bug that joins template closer and =

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: Quuxplusone, MyDeveloperDay, sammccall, klimek, 
djasper.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Also fixes the documentation for `SpaceBeforeAssignmentOperators`.

See the discussion here .


Repository:
  rC Clang

https://reviews.llvm.org/D66384

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6620,8 +6620,15 @@
 
   verifyFormat("A> a;", getChromiumStyle(FormatStyle::LK_Cpp));
 
-  verifyFormat("int i = a<1> >> 1;");
+  // template closer followed by a token that starts with > or =
   verifyFormat("bool b = a<1> > 1;");
+  verifyFormat("bool b = a<1> >= 1;");
+  verifyFormat("int i = a<1> >> 1;");
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeAssignmentOperators = false;
+  verifyFormat("bool b= a<1> == 1;", Style);
+  verifyFormat("a = 1;", Style);
+  verifyFormat("a >>= 1;", Style);
 
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2870,7 +2870,7 @@
   Left.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar) ||
   (Right.is(tok::period) && Right.isNot(TT_DesignatedInitializerPeriod)))
 return false;
-  if (!Style.SpaceBeforeAssignmentOperators &&
+  if (!Style.SpaceBeforeAssignmentOperators && Left.isNot(TT_TemplateCloser) &&
   Right.getPrecedence() == prec::Assignment)
 return false;
   if (Style.Language == FormatStyle::LK_Java && Right.is(tok::coloncolon) &&
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1738,8 +1738,8 @@
   /// If ``false``, spaces will be removed before assignment operators.
   /// \code
   ///true:  false:
-  ///int a = 5; vs. int a=5;
-  ///a += 42a+=42;
+  ///int a = 5; vs. int a= 5;
+  ///a += 42;   a+= 42;
   /// \endcode
   bool SpaceBeforeAssignmentOperators;
 
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2050,8 +2050,8 @@
   .. code-block:: c++
 
  true:  false:
- int a = 5; vs. int a=5;
- a += 42a+=42;
+ int a = 5; vs. int a= 5;
+ a += 42;   a+= 42;
 
 **SpaceBeforeCpp11BracedList** (``bool``)
   If ``true``, a space will be inserted before a C++11 braced list


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6620,8 +6620,15 @@
 
   verifyFormat("A> a;", getChromiumStyle(FormatStyle::LK_Cpp));
 
-  verifyFormat("int i = a<1> >> 1;");
+  // template closer followed by a token that starts with > or =
   verifyFormat("bool b = a<1> > 1;");
+  verifyFormat("bool b = a<1> >= 1;");
+  verifyFormat("int i = a<1> >> 1;");
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeAssignmentOperators = false;
+  verifyFormat("bool b= a<1> == 1;", Style);
+  verifyFormat("a = 1;", Style);
+  verifyFormat("a >>= 1;", Style);
 
   verifyFormat("test >> a >> b;");
   verifyFormat("test << a >> b;");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2870,7 +2870,7 @@
   Left.isOneOf(tok::arrow, tok::period, tok::arrowstar, tok::periodstar) ||
   (Right.is(tok::period) && Right.isNot(TT_DesignatedInitializerPeriod)))
 return false;
-  if (!Style.SpaceBeforeAssignmentOperators &&
+  if (!Style.SpaceBeforeAssignmentOperators && Left.isNot(TT_TemplateCloser) &&
   Right.getPrecedence() == prec::Assignment)
 return false;
   if (Style.Language == FormatStyle::LK_Java && Right.is(tok::coloncolon) &&
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1738,8 +1738,8 @@
   /// If ``false``, spaces will be removed before assignment

[PATCH] D66384: [clang-format] Fix a bug that joins template closer and =

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D66384#1634405 , @Quuxplusone wrote:

> although I would still question whether 
> `Style.SpaceBeforeAssignmentOperators` is providing anyone any benefit at all.


See here  
for the patch. I don't know why the option was added but think it's probably 
outdated and should be deprecated.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66384



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