[PATCH] D26163: [clang-format] Fix PR30527: Regression when clang-format insert spaces in [] when in template

2016-10-31 Thread Branko Kokanovic via cfe-commits
branko created this revision.
branko added a reviewer: djasper.
branko added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

Actual regression was introduced in r272668. This revision fixes JS script, but 
also regress Cpp case. It manifests with spaces added when template is followed 
with array. Bug 30527 mentions case of array as a nested template type 
(foo[]>). Fix is to detect such case and to prevent treating it as 
array initialization, but as a subscript case. However, before r272668, this 
case was treated simple because we were detecting it as a StartsObjCMethodExpr. 
Same was true for other similar case - array of templates (foo[]). This 
patch tries to address two problems: 1) fixing regression 2) making sure both 
cases (array as a nested type, array of templates) which were entering 
StartsObjCMethodExpr branch are handled now appropriately.


https://reviews.llvm.org/D26163

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11501,6 +11501,26 @@
   verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style);
 }
 
+TEST_F(FormatTest, ArrayOfTemplates) {
+  EXPECT_EQ("auto a = new unique_ptr[10];",
+format("auto a = new unique_ptr [ 10];"));
+
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpacesInSquareBrackets = true;
+  EXPECT_EQ("auto a = new unique_ptr[ 10 ];",
+format("auto a = new unique_ptr [10];", Spaces));
+}
+
+TEST_F(FormatTest, ArrayAsTemplateType) {
+  EXPECT_EQ("auto a = unique_ptr[10]>;",
+format("auto a = unique_ptr < Foo < Bar>[ 10]> ;"));
+
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpacesInSquareBrackets = true;
+  EXPECT_EQ("auto a = unique_ptr[ 10 ]>;",
+format("auto a = unique_ptr < Foo < Bar>[10]> ;", Spaces));
+}
+
 // Since this test case uses UNIX-style file path. We disable it for MS
 // compiler.
 #if !defined(_MSC_VER) && !defined(__MINGW32__)
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -305,7 +305,17 @@
 FormatToken *Left = CurrentToken->Previous;
 Left->ParentBracket = Contexts.back().ContextKind;
 FormatToken *Parent = Left->getPreviousNonComment();
-bool StartsObjCMethodExpr =
+
+// Cases where '>' is followed by '['.
+// In C++, this can happen either in array of templates (foo[10])
+// or when array is a nested template type (unique_ptr[]>).
+bool CppArrayTemplates =
+Style.Language == FormatStyle::LK_Cpp && Parent &&
+Parent->is(TT_TemplateCloser) &&
+(Contexts.back().CanBeExpression || Contexts.back().IsExpression ||
+ Contexts.back().InTemplateArgument);
+
+bool StartsObjCMethodExpr = !CppArrayTemplates &&
 Style.Language == FormatStyle::LK_Cpp &&
 Contexts.back().CanBeExpression && Left->isNot(TT_LambdaLSquare) &&
 CurrentToken->isNot(tok::l_brace) &&
@@ -326,7 +336,7 @@
  Parent->isOneOf(tok::l_brace, tok::comma)) {
 Left->Type = TT_JsComputedPropertyName;
   } else if (Style.Language == FormatStyle::LK_Proto ||
- (Parent &&
+ (!CppArrayTemplates && Parent &&
   Parent->isOneOf(TT_BinaryOperator, TT_TemplateCloser, 
tok::at,
   tok::comma, tok::l_paren, tok::l_square,
   tok::question, tok::colon, tok::kw_return,


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11501,6 +11501,26 @@
   verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style);
 }
 
+TEST_F(FormatTest, ArrayOfTemplates) {
+  EXPECT_EQ("auto a = new unique_ptr[10];",
+format("auto a = new unique_ptr [ 10];"));
+
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpacesInSquareBrackets = true;
+  EXPECT_EQ("auto a = new unique_ptr[ 10 ];",
+format("auto a = new unique_ptr [10];", Spaces));
+}
+
+TEST_F(FormatTest, ArrayAsTemplateType) {
+  EXPECT_EQ("auto a = unique_ptr[10]>;",
+format("auto a = unique_ptr < Foo < Bar>[ 10]> ;"));
+
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpacesInSquareBrackets = true;
+  EXPECT_EQ("auto a = unique_ptr[ 10 ]>;",
+format("auto a = unique_ptr < Foo < Bar>[10]> ;", Spaces));
+}
+
 // Since this test case uses UNIX-style file path. We disable it for MS
 // compiler.
 #if !defined(_MSC_VER) && !defined(__MINGW32__)
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -305,7 +305,17 @@
 FormatToken *Left = CurrentToken

Re: [PATCH] D26163: [clang-format] Fix PR30527: Regression when clang-format insert spaces in [] when in template

2016-11-08 Thread Branko Kokanovic via cfe-commits
Friendly ping:)

On Mon, Oct 31, 2016 at 10:16 PM, Branko Kokanovic 
wrote:

> branko created this revision.
> branko added a reviewer: djasper.
> branko added a subscriber: cfe-commits.
> Herald added a subscriber: klimek.
>
> Actual regression was introduced in r272668. This revision fixes JS
> script, but also regress Cpp case. It manifests with spaces added when
> template is followed with array. Bug 30527 mentions case of array as a
> nested template type (foo[]>). Fix is to detect such case and to
> prevent treating it as array initialization, but as a subscript case.
> However, before r272668, this case was treated simple because we were
> detecting it as a StartsObjCMethodExpr. Same was true for other similar
> case - array of templates (foo[]). This patch tries to address two
> problems: 1) fixing regression 2) making sure both cases (array as a nested
> type, array of templates) which were entering StartsObjCMethodExpr branch
> are handled now appropriately.
>
>
> https://reviews.llvm.org/D26163
>
> Files:
>   lib/Format/TokenAnnotator.cpp
>   unittests/Format/FormatTest.cpp
>
>
> Index: unittests/Format/FormatTest.cpp
> ===
> --- unittests/Format/FormatTest.cpp
> +++ unittests/Format/FormatTest.cpp
> @@ -11501,6 +11501,26 @@
>verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style);
>  }
>
> +TEST_F(FormatTest, ArrayOfTemplates) {
> +  EXPECT_EQ("auto a = new unique_ptr[10];",
> +format("auto a = new unique_ptr [ 10];"));
> +
> +  FormatStyle Spaces = getLLVMStyle();
> +  Spaces.SpacesInSquareBrackets = true;
> +  EXPECT_EQ("auto a = new unique_ptr[ 10 ];",
> +format("auto a = new unique_ptr [10];", Spaces));
> +}
> +
> +TEST_F(FormatTest, ArrayAsTemplateType) {
> +  EXPECT_EQ("auto a = unique_ptr[10]>;",
> +format("auto a = unique_ptr < Foo < Bar>[ 10]> ;"));
> +
> +  FormatStyle Spaces = getLLVMStyle();
> +  Spaces.SpacesInSquareBrackets = true;
> +  EXPECT_EQ("auto a = unique_ptr[ 10 ]>;",
> +format("auto a = unique_ptr < Foo < Bar>[10]> ;", Spaces));
> +}
> +
>  // Since this test case uses UNIX-style file path. We disable it for MS
>  // compiler.
>  #if !defined(_MSC_VER) && !defined(__MINGW32__)
> Index: lib/Format/TokenAnnotator.cpp
> ===
> --- lib/Format/TokenAnnotator.cpp
> +++ lib/Format/TokenAnnotator.cpp
> @@ -305,7 +305,17 @@
>  FormatToken *Left = CurrentToken->Previous;
>  Left->ParentBracket = Contexts.back().ContextKind;
>  FormatToken *Parent = Left->getPreviousNonComment();
> -bool StartsObjCMethodExpr =
> +
> +// Cases where '>' is followed by '['.
> +// In C++, this can happen either in array of templates (foo[10])
> +// or when array is a nested template type
> (unique_ptr[]>).
> +bool CppArrayTemplates =
> +Style.Language == FormatStyle::LK_Cpp && Parent &&
> +Parent->is(TT_TemplateCloser) &&
> +(Contexts.back().CanBeExpression || Contexts.back().IsExpression
> ||
> + Contexts.back().InTemplateArgument);
> +
> +bool StartsObjCMethodExpr = !CppArrayTemplates &&
>  Style.Language == FormatStyle::LK_Cpp &&
>  Contexts.back().CanBeExpression && Left->isNot(TT_LambdaLSquare)
> &&
>  CurrentToken->isNot(tok::l_brace) &&
> @@ -326,7 +336,7 @@
>   Parent->isOneOf(tok::l_brace, tok::comma)) {
>  Left->Type = TT_JsComputedPropertyName;
>} else if (Style.Language == FormatStyle::LK_Proto ||
> - (Parent &&
> + (!CppArrayTemplates && Parent &&
>Parent->isOneOf(TT_BinaryOperator, TT_TemplateCloser,
> tok::at,
>tok::comma, tok::l_paren, tok::l_square,
>tok::question, tok::colon,
> tok::kw_return,
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D26163: [clang-format] Fix PR30527: Regression when clang-format insert spaces in [] when in template

2016-11-10 Thread Branko Kokanovic via cfe-commits
branko added a comment.

Thanks. @djasper - can you commit this for me, please?


https://reviews.llvm.org/D26163



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