Re: [PATCH] D11693: clang-format: Support generalized lambda captures.
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.
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.
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.
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.
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.
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.
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