Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-09-16 Thread strager via cfe-commits
strager marked 4 inline comments as done.


Comment at: lib/Format/UnwrappedLineParser.cpp:1057-1058
@@ +1056,4 @@
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside.
+if (FormatTok->is(tok::l_paren)) {

strager wrote:
> djasper wrote:
> > I very much doubt that we'll have an Expression parser here anytime soon. 
> > So, I don't think that this FIXME makes much sense. Instead, please provide 
> > a comment on what this is actually doing and in which cases it might fail.
> I copied the comment from elsewhere in the file.
I expanded the comment, including a reference to the other reference to 
`parseAssigmentExpression`.


Comment at: lib/Format/UnwrappedLineParser.cpp:1061
@@ +1060,3 @@
+  parseParens();
+} else if (FormatTok->isOneOf(tok::comma, tok::r_square)) {
+  break;

strager wrote:
> djasper wrote:
> > I think this list should be extended to figure out certain cases where we 
> > know something is fishy. In particular:
> > * If you find an l_square or less, call into parseSquare and parseAngle 
> > respectively.
> > * If you find an r_brace or semi, something is wrong, break.
> > 
> Will do.
Handling r_brace and semi is a bit weird, since we end up aborting mid-stream 
and what's left becomes unparsable/incomplete by clang-format.

parseAngle doesn't exist, and even if it did, the less-than operator wouldn't 
be handled properly.

I added l_square and l_brace support.


http://reviews.llvm.org/D11693



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


Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-09-16 Thread strager via cfe-commits
strager updated this revision to Diff 34946.
strager marked 2 inline comments as done.
strager added a comment.

Address @djasper's comments.


http://reviews.llvm.org/D11693

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -10057,6 +10057,18 @@
   // More complex introducers.
   verifyFormat("return [i, args...] {};");
 
+  // Lambdas with generalized captures.
+  verifyFormat("auto f = [b = d]() {};\n");
+  verifyFormat("auto f = [b = std::move(d)]() {};\n");
+  verifyFormat("auto f = [b = c, d = e, g]() {};\n");
+  verifyFormat("auto f = [b = a[3]]() {};\n");
+  verifyFormat("auto f = [InRange = a < b && b < c]() {};\n");
+  verifyFormat("auto f = [b = std::vector{1, 2, 3}]() {};\n");
+  verifyFormat("auto f = [b = std::vector{\n"
+   "  1, 2, 3,\n"
+   "}]() {};\n");
+  verifyFormat("return [b = std::vector()] {}();\n");
+
   // Not lambdas.
   verifyFormat("constexpr char hello[]{\"hello\"};");
   verifyFormat("double &operator[](int i) { return 0; }\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1054,6 +1054,27 @@
 nextToken();
 if (FormatTok->is(tok::ellipsis))
   nextToken();
+if (FormatTok->is(tok::equal)) {
+  // Generalized lambda capture.
+  nextToken();
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside. See also
+// parseBracedList. For now, parsing matching braces ([], (), {}) is
+// good enough.
+if (FormatTok->is(tok::l_paren)) {
+  parseParens();
+} else if (FormatTok->is(tok::l_square)) {
+  parseSquare();
+} else if (FormatTok->is(tok::l_brace)) {
+  parseBracedList(false);
+} else if (FormatTok->isOneOf(tok::comma, tok::r_square)) {
+  break;
+} else {
+  nextToken();
+}
+  }
+}
 if (FormatTok->is(tok::comma)) {
   nextToken();
 } else if (FormatTok->is(tok::r_square)) {
@@ -1110,7 +1131,8 @@
   nextToken();
 
   // FIXME: Once we have an expression parser in the UnwrappedLineParser,
-  // replace this by using parseAssigmentExpression() inside.
+  // replace this by using parseAssigmentExpression() inside. See also
+  // tryToParseLambdaIntroducer.
   do {
 if (Style.Language == FormatStyle::LK_JavaScript) {
   if (FormatTok->is(Keywords.kw_function)) {


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -10057,6 +10057,18 @@
   // More complex introducers.
   verifyFormat("return [i, args...] {};");
 
+  // Lambdas with generalized captures.
+  verifyFormat("auto f = [b = d]() {};\n");
+  verifyFormat("auto f = [b = std::move(d)]() {};\n");
+  verifyFormat("auto f = [b = c, d = e, g]() {};\n");
+  verifyFormat("auto f = [b = a[3]]() {};\n");
+  verifyFormat("auto f = [InRange = a < b && b < c]() {};\n");
+  verifyFormat("auto f = [b = std::vector{1, 2, 3}]() {};\n");
+  verifyFormat("auto f = [b = std::vector{\n"
+   "  1, 2, 3,\n"
+   "}]() {};\n");
+  verifyFormat("return [b = std::vector()] {}();\n");
+
   // Not lambdas.
   verifyFormat("constexpr char hello[]{\"hello\"};");
   verifyFormat("double &operator[](int i) { return 0; }\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1054,6 +1054,27 @@
 nextToken();
 if (FormatTok->is(tok::ellipsis))
   nextToken();
+if (FormatTok->is(tok::equal)) {
+  // Generalized lambda capture.
+  nextToken();
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside. See also
+// parseBracedList. For now, parsing matching braces ([], (), {}) is
+// good enough.
+if (FormatTok->is(tok::l_paren)) {
+  parseParens();
+} else if (FormatTok->is(tok::l_square)) {
+  parseSquare();
+} else if (FormatTok->is(tok::l_brace)) {
+  parseBracedList(false);
+} else if (FormatTok->isOneOf(tok::comma, tok::r_square)) {
+  break;
+} else {
+  nextToken();
+}
+  }
+}
 if (FormatTok->is(tok::comma)) {
   nextToken();
 } else if (FormatTok->is(tok::r_square)) {
@@ -1110,7 +1131,8 @@
   nextToken();
 
   // FIXME: Once we have an expressi

Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-09-17 Thread Daniel Jasper via cfe-commits
djasper added inline comments.


Comment at: lib/Format/UnwrappedLineParser.cpp:1060-1061
@@ +1059,4 @@
+  nextToken();
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside. See also

Again, please remove the FIXME. We aren't going to have an expression parser 
here (anytime soon) and shouldn't add (more) comments that make people think 
otherwise.


Comment at: lib/Format/UnwrappedLineParser.cpp:1064
@@ +1063,3 @@
+// parseBracedList. For now, parsing matching braces ([], (), {}) is
+// good enough.
+if (FormatTok->is(tok::l_paren)) {

Ah, parseAngle doesn't exist here. I was thinking about the TokenAnnotator.

I don't understand your comment about mid-stream. This is precisely about the 
case where the input is corrupt so that clang-format can recover and doesn't 
just parse the reset of the file input the lambda introducer.


http://reviews.llvm.org/D11693



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


Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-09-17 Thread strager via cfe-commits
strager marked 2 inline comments as done.


Comment at: lib/Format/UnwrappedLineParser.cpp:1060-1061
@@ +1059,4 @@
+  nextToken();
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside. See also

djasper wrote:
> Again, please remove the FIXME. We aren't going to have an expression parser 
> here (anytime soon) and shouldn't add (more) comments that make people think 
> otherwise.
> We aren't going to have an expression parser here (anytime soon) and 
> shouldn't add (more) comments that make people think otherwise.

If there is enough need for the function, perhaps it will be written.

I don't think the comment implies some code will be written soon.


Comment at: lib/Format/UnwrappedLineParser.cpp:1064
@@ +1063,3 @@
+// parseBracedList. For now, parsing matching braces ([], (), {}) is
+// good enough.
+if (FormatTok->is(tok::l_paren)) {

djasper wrote:
> Ah, parseAngle doesn't exist here. I was thinking about the TokenAnnotator.
> 
> I don't understand your comment about mid-stream. This is precisely about the 
> case where the input is corrupt so that clang-format can recover and doesn't 
> just parse the reset of the file input the lambda introducer.
> This is precisely about the case where the input is corrupt so that 
> clang-format can recover and doesn't just parse the reset of the file input 
> the lambda introducer.

If I write this test:

```
verifyFormat("return [}] {};\n");
```

I get this output:

```
/Users/strager/Projects/llvm/tools/clang/unittests/Format/FormatTest.cpp:42: 
Failure
Value of: IncompleteFormat
  Actual: true
Expected: ExpectedIncompleteFormat
Which is: false
return [}] {};



/Users/strager/Projects/llvm/tools/clang/unittests/Format/FormatTest.cpp:65: 
Failure
Value of: format(test::messUp(Code), Style)
  Actual: "return [\n}] {};\n"
Expected: Code.str()
Which is: "return [}] {};\n"
```

How can I fix this?


http://reviews.llvm.org/D11693



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


Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-09-22 Thread Daniel Jasper via cfe-commits
djasper added inline comments.


Comment at: lib/Format/UnwrappedLineParser.cpp:1064
@@ +1063,3 @@
+// parseBracedList. For now, parsing matching braces ([], (), {}) is
+// good enough.
+if (FormatTok->is(tok::l_paren)) {

strager wrote:
> djasper wrote:
> > Ah, parseAngle doesn't exist here. I was thinking about the TokenAnnotator.
> > 
> > I don't understand your comment about mid-stream. This is precisely about 
> > the case where the input is corrupt so that clang-format can recover and 
> > doesn't just parse the reset of the file input the lambda introducer.
> > This is precisely about the case where the input is corrupt so that 
> > clang-format can recover and doesn't just parse the reset of the file input 
> > the lambda introducer.
> 
> If I write this test:
> 
> ```
> verifyFormat("return [}] {};\n");
> ```
> 
> I get this output:
> 
> ```
> /Users/strager/Projects/llvm/tools/clang/unittests/Format/FormatTest.cpp:42: 
> Failure
> Value of: IncompleteFormat
>   Actual: true
> Expected: ExpectedIncompleteFormat
> Which is: false
> return [}] {};
> 
> 
> 
> /Users/strager/Projects/llvm/tools/clang/unittests/Format/FormatTest.cpp:65: 
> Failure
> Value of: format(test::messUp(Code), Style)
>   Actual: "return [\n}] {};\n"
> Expected: Code.str()
> Which is: "return [}] {};\n"
> ```
> 
> How can I fix this?
So, there are two errors in the test.

1. clang-format detects that there is a syntax error and reports this. Use 
verifyIncompleteFormat instead of verifyFormat.

2. clang-format adds a '\n' after the opening square bracket. Add that in your 
test string.


http://reviews.llvm.org/D11693



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


Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-08-07 Thread strager via cfe-commits
strager added a comment.

ping


http://reviews.llvm.org/D11693



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


Re: [PATCH] D11693: clang-format: Support generalized lambda captures.

2015-08-11 Thread strager via cfe-commits
strager planned changes to this revision.


Comment at: lib/Format/UnwrappedLineParser.cpp:1057-1058
@@ +1056,4 @@
+  while (!eof()) {
+// FIXME: Once we have an expression parser in the UnwrappedLineParser,
+// replace this by using parseAssigmentExpression() inside.
+if (FormatTok->is(tok::l_paren)) {

djasper wrote:
> I very much doubt that we'll have an Expression parser here anytime soon. So, 
> I don't think that this FIXME makes much sense. Instead, please provide a 
> comment on what this is actually doing and in which cases it might fail.
I copied the comment from elsewhere in the file.


Comment at: lib/Format/UnwrappedLineParser.cpp:1061
@@ +1060,3 @@
+  parseParens();
+} else if (FormatTok->isOneOf(tok::comma, tok::r_square)) {
+  break;

djasper wrote:
> I think this list should be extended to figure out certain cases where we 
> know something is fishy. In particular:
> * If you find an l_square or less, call into parseSquare and parseAngle 
> respectively.
> * If you find an r_brace or semi, something is wrong, break.
> 
Will do.


http://reviews.llvm.org/D11693



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