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. ;)