Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for capturing lambdas in GENERATE() #1566

Closed
wants to merge 2 commits into from
Closed

Add support for capturing lambdas in GENERATE() #1566

wants to merge 2 commits into from

Conversation

alibabashack
Copy link

Description

It is already allowed to use non-capturing lambdas in the argument table of a GENERATE(). This simple fix allows to use capturing lambdas as-well, now.

Since the GENERATE() macro-magic contains a (non-capturing) lambda, we end up in a lambda-in-lambda situation when lambdas are supplied as arguments in the table. Since the inner lambdas can only capture if the outer lambda has a default capture, a capture by reference is added here. This still allows the inner lambdas to capture by value if necessary or to capture not at all.

An example test scenario is added as well, which hopefully shows that capturing lambdas can be quite useful in conjunction with generators.

GitHub Issues

Also see #850

@alibabashack
Copy link
Author

Hi, could somebody please take a look at the output of the test comparison? I think the delta looks good and is only due to the added test, but I am new to the process. Thanks!

@horenmar
Copy link
Member

If the delta looks right, you should approve it yourself before pushing the PR, see contributing.md.

However, this current PR has a different problem as well, specifically that the plain GENERATE intentionally does not allow variable capture for user's safety, as stated in the generator documentation.

I think this PR actually shows that it was a good idea, because the test you've added actually invokes UB because of the capture. The problem is that after the first run-through the test case, the object in the instance variable is destroyed. Then, for the second run, it is created again. At this point, the captured reference inside the lambdas is no longer valid (the refered-to object has been destroyed), so when we use it, the code invokes undefined behaviour and everything has gone pear-shaped.

@alibabashack
Copy link
Author

@horenmar
I did not see that problem coming. Thanks a lot for your crystal clear explanation. That's of course enough to cancel this PR.

For the record:
There is an easy workaround to solve the idea shown in d385b46 without capturing. Simply add an argument to your lambda as shown below:

auto[seqId, inputSequence, expectedProperty] =
        GENERATE(table<std::string, std::function<void(SomeStateMachine&)>, bool>({
            {
                "AA",
                [](SomeStateMachine& sm) {
                    sm.doA();
                    sm.doA();
                },
                false
            },
            {
                "AB",
                [](SomeStateMachine& sm) {
                    sm.doA();
                    sm.doB();
                },
                true
            },
        }));
inputSequence(instance);

@horenmar
Copy link
Member

I've actually been thinking about providing a separate macro that does capture variables, but it would still be an expert-only feature. In the meantime, if someone ends up really needing variables inside generators, we provide an example for that.

@ozars
Copy link
Contributor

ozars commented Mar 21, 2019

In the meanwhile, is it possible to permit capture-by-value [=] in current GENERATE definition?

I got a case where I wanted to do something like range(0, some_vector.size()). This would at least permit user to pass copy assignable constructible variables.

@horenmar horenmar removed the Revisit label Apr 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants