Community
Participate
Working Groups
C++14 will include support for generalized lambda captures described: http://isocpp.org/files/papers/N3648.html The following code has been supported since GCC 4.5 and will become standard for C++14: #include <iostream> #include <memory> int main(int argc, char **argv) { std::unique_ptr<int> a(new int(3)); // declare 'a' and 'b' within the lambda // as the deduced type of the initializer [a = std::move(a), b = 9] { std::cout << *a << ' ' << b << std::endl; }(); return 0; } CDT 8.2.1 shows the entire lambda as a Syntax Error when this is done. Given its existing support in GCC and now being on the standards track, it would be nice to take advantage of this as soon as possible,
Is this a duplicate of bug #488265 ? With Neon.2 (4.6.2) I can still see "Syntax Error" for a lambda with captured variables declared.
(In reply to Francois Ee from comment #1) > Is this a duplicate of bug #488265 ? No, that bug is unrelated. That bug concerns using regular C++11 lambdas in a particular context (inside an initializer list). This bug concerns a C++14 extension to lambdas that has not been implemented yet.
Is anyone working on this bug? It's been over a year since the last update here and close to five years since it was created. This is pretty close to the most annoying thing about the CDT to me right now. Can anyone point me where to look at it if nobody is working on it now? How hard of a task is this to someone who can code but is not familiar with eclipse/CDT internals?
(In reply to Patrck McMichael from comment #3) > Is anyone working on this bug? Not that I'm aware of. > Can anyone point me > where to look at it if nobody is working on it now? I would be happy to provide guidance to someone interested in contributing a fix for this issue! > How hard of a task is > this to someone who can code but is not familiar with eclipse/CDT internals? There are two parts to this fix: 1) Syntax support. This will avoid init-captures causing the entire lambda to be a syntax error. 2) Semantic support. This will ensure that uses of the introduced names inside the lambda resolve (otherwise, with only (1) in place, there will be localized semantic errors under those uses). I expect (1) to be fairly straightforward. (2) is likely to be more involved, though probably still tractable for a determined contributor. Let me know if you'd like to give (1) a try, and I'll provide some specific pointers to the relevant code.
Very poor support for (1) is available in [1]. If the name of the captured variable matches a name the surrounding scope with the same type, it happens to work fairly well (e.g. [x = std::move(x)]). I have used this for several years now. [1] https://github.com/avikivity/cdt/commit/a7b52ad7019e4c229f21a44ef8fdae2f283da225
Thanks, Avi, the patch you posted is a great start as it already eliminates the syntax error. The next step would be to actually store the initializer expression in the AST instead of just having the parser throw it away after parsing it (this involves modifying the AST node representing captures (CPPASTCapture) to optionally store an initializer expression, and setting the initializer expression in the parser). This would e.g. allow the initializer expression to be syntax-colored. You are also correct to point out that for the common case of "foo = std::move(foo)", name resolution just works without further changes because the uses inside the lambda just resolve to the variable in the enclosing scope. (Avi, is there anything else you've been sitting on that you could share with us? ;))
I also had a similarly-poor implementation of digit separators, but it was obsoleted by 7d9e0b0ddd7e515966b01c3c7d222ecb71580ff1.
Thanks for the updates and pointers, everyone! Would it be worthwhile to integrate Avi's patch to ease the visually jarring portion of the problem (marking the entire lambda as being syntactically invalid)? Again, I appreciate the feedback and will see if I can carve out some time to look at this at home (no promises!).
I don't think the patch is good enough (no unit tests etc.). In case it is, I'll be happy to sign the CLA. Patrck, in case you follow the dubious practice of installing random software provided by strangers, you can point your Eclipse at http://eclipse.scylladb.com/cdt/ and get a recent cdt.git + this patch. I updated it every few weeks.
(In reply to Avi Kivity from comment #9) > I don't think the patch is good enough (no unit tests etc.). In case it is, > I'll be happy to sign the CLA. I would merge this patch with a comment added that says "TODO: store the initializer expression in the AST", and a single unit test that contains a trivial use of an init-capture and checks that there are no syntax errors. Would you be willing to make those small changes and submit it via Gerrit?
Certainly. Can you point me to the entry point of the CLA process?
(In reply to Avi Kivity from comment #11) > Certainly. Can you point me to the entry point of the CLA process? Go to https://accounts.eclipse.org/, sign in, and click on "Eclipse Contributor Agreement" at the top.
I managed to find it and signed. So I'll submit the patch for review it soon.
Just a heads up, in case people would like to have the syntax support in Photon: our photon RC1 build is tomorrow, and RC2 a week after. I would accept a minimal patch along the lines described in comment 10, before RC2. After that, I would only accept it after Photon branches (in which case the change will not make it into the Photon release).
I'd love to send a patch, but I can't get "mvn test" to run. I get [ERROR] Failed to execute goal on project org.eclipse.cdt.meson: Could not resolve dependencies for project org.eclipse.cdt:org.eclipse.cdt.meson:eclipse-feature:9.5.0-SNAPSHOT: Could not find artifact org.eclipse.cdt:org.eclipse.cdt.meson.docs:jar:1.0.0-SNAPSHOT Help would be appreciated.
I don't know about that particular error, but I don't find the need to run "mvn" commands at all. To test my changes locally, I just do Run As -> Eclipse Application from within the development instance. If I'm happy with the behaviour, I go ahead and push the patch to gerrit, which will trigger automated tests. If I want to run a particular test suite locally, say ParserTestSuite, I do that with Run As -> JUnit Plug-in Test.
Dear Avi Kivity, Pretty please! :-)
(In reply to Nathan Ridge from comment #16) > I don't know about that particular error, but I don't find the need to run > "mvn" commands at all. > > To test my changes locally, I just do Run As -> Eclipse Application from > within the development instance. If I'm happy with the behaviour, I go ahead > and push the patch to gerrit, which will trigger automated tests. If I want > to run a particular test suite locally, say ParserTestSuite, I do that with > Run As -> JUnit Plug-in Test. That helped a lot! Will talk to Gerrit now.
I think GDPR killed Gerrit. When I try to sign in, I get complaints that cookies are needed. I am logged in to www.eclipse.org but not to git.eclispe.org. Meanwhile, a patch with a unit test can be seen in [1]. [1] https://github.com/avikivity/cdt/commit/0686f2c9b0b82f6dd6acae6f0f228663a444b864
I'm such a n00b. There was a slide-up button in the lower left that allows me to accept cookies.
Having trouble fetching the commit-msg hook for Change-Id: tag, the instructions are geared towards people with ssh access (I have only http access).
New Gerrit change created: https://git.eclipse.org/r/123379
New Gerrit change created: https://git.eclipse.org/r/123380
Looks like we got not just one patch but two :) The first patch, by Avi Kivity, takes the minimal approach described in comment 10. The second patch, by Hansruedi Patzen, implements both of the parts (syntax and semantic) described in comment 4. Since the second patch involves API changes, and I don't believe we can make API changes to Photon at this stage, what I'll do is merge the first patch into Photon, and merge the second patch (rebased) after Photon branches.
(In reply to Nathan Ridge from comment #24) > Since the second patch involves API changes, and I don't believe we can make > API changes to Photon at this stage, what I'll do is merge the first patch > into Photon, and merge the second patch (rebased) after Photon branches. Looking at Hansruedi's patch made me realize that thre are actually three syntactic forms of an init-capture: [foo=expr] [foo{expr}] [foo(expr)] Avi's patch gets the parser to accept the first, but not the other two. We could ask Avi to modify his patch to accept the second two forms, but perhaps a better approach is to ask for an exception to the rule about API changes, and just merge Hansruedi's patch for Photon. I've posted about this on the CDT mailing list: https://dev.eclipse.org/mhonarc/lists/cdt-dev/msg32954.html
Gerrit change https://git.eclipse.org/r/123380 was merged to [master]. Commit: http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=957dae8f4ea9069c9fa67e4e1082ff588bd2a5cb
(In reply to Nathan Ridge from comment #25) > perhaps a better approach is to ask for an exception to the rule about API > changes, and just merge Hansruedi's patch for Photon. > > I've posted about this on the CDT mailing list: > https://dev.eclipse.org/mhonarc/lists/cdt-dev/msg32954.html There were no objections to merging Hansruedi's patch into Photon, so I go ahead and did that. It will even make the RC2 build, which has been delayed until tomorrow morning. So, this is officially fixed for Photon :) Thanks very much to Hansruedi and everyone else who helped move this forward!
Wow, great job everyone! Thank you for proving once again why the CDT is the best C++ IDE out there!
