C++ lambda function access write violation

Question

I'm learning how to use C++ lambda functions along with <functional>'s function class. I am trying to solve this Code Golf as practice (challenge is Curry for Dinner)

I have this function:

// This creates a function that runs y a number of 
// times equal to x's return value.
function<void()> Curry(function<int()> x, function<void()> y)
{
    return [&]() {
        for (int i = 0; i < x(); i++)
        {
            y();
        }
    };
}

To test this I have this code in my main():

auto x = [](){ return 8; };
auto y = [](){ cout << "test "; };
auto g = Curry(x, y);

This throws Access violation reading location 0xCCCCCCCC. in Functional.h.

Yet when I copy-paste the lambda function from inside Curry() to inside my main like this:

auto x = [](){ return 8; };
auto y = [](){ cout << "test "; };
auto g = [&]() {
    for (int i = 0; i < x(); i++)
    {
        y();
    }
};

I get the code running as expected. Why does this happen?

Answer 1

You have a few problems.

Here:

  return [&]() {

you capture by reference. Any variables you capture has to have a lifetime that exceeds your own. It means that running the lambda becomes undefined behavior after the variables you capture&use lifetime ends. As you are returning this lambda, and capturing local state, this seems likely to happen. (Note I said variables -- due to a quirk in the standard, [&] captures variables not the data referred to by variables, so even capturing & function arguments by [&] is not safe. This may change in future revisions of the standard... There are neat optimizations that this particular set of rules allow in lambda implementations (reduce [&] lambdas to having 1 pointer worth of state(!)), but it also introduces the only case in C++ where you have a reference to a reference variable in effect...)

Change it to

  return [=]() {

and capture by-value.

Or even:

  return [x,y]() {

to list your captures explicitly.

When using a lambda which does not outlive the current scope, I use [&]. Otherwise, I capture by value explicitly the stuff I am going to use, as lifetime is important in that case.

Next:

    for (int i = 0; i < x(); i++)

you run x once for every loop iteration. Seems silly!

Instead:

    auto max = x();
    for (auto i = max; i > 0; --i)

which runs max times, and as it happens works if the return value of x was changed to unsigned int or whatever.

Or:

    int max = x();
    for (int i = 0; i < max; ++i)

which both runs x once, and behaves better if x returns -1.

Alternatively you can use the obscure operator -->:

    int count = x();
    while( count --> 0 )

if you want to make your code unreadable. ;)