[Issue 21929] delegates capture do not respect scoping

[d-bugmail at puremagic.com]

https://issues.dlang.org/show_bug.cgi?id=21929

Stanislav Blinov <stanislav.blinov at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |stanislav.blinov at gmail.com
           Severity|major                       |critical

--- Comment #9 from Stanislav Blinov <stanislav.blinov at gmail.com> ---
(In reply to deadalnix from comment #8)

> You'll not that C++'s std::function will allocate on heap if you capture.

??? std::function MAY allocate on the heap, and whether it will would depend on
its implementation and size of lambda's state. A decent implementation of
std::function surely would not allocate if all you capture is a single
reference.

> The equivalent code in C++ WILL allocate in a loop too.

Whether std::function would allocate is irrelevant. Equivalent C++ code would,
generally speaking, print unspecified values for all but the string:

#include <functional>
#include <vector>
#include <iostream>

int main(int argc, char** argv) {
    std::vector<std::function<void()>> dgs;

    for (int i = 0; i < 10; i++) {
        // Capture by reference, as that's D's semantics
        dgs.emplace_back([&i] () {
            std::cout << i << "\n";
        });
    }

    dgs.emplace_back([] () {
        std::cout << "With cached variables" << "\n";
    });

    for (int i = 0; i < 10; i++) {
        int index = i;
        // Capture by reference, as that's D's semantics
        dgs.emplace_back([&index] () {
            std::cout << index << "\n";
        });
    }

    for (auto& dg: dgs) {
        dg();
    }

    return 0;
}

Debug build with clang prints 10s followed by 9s. Debug build with gcc prints
garbage values. Optimized builds with either print garbage values. Walter's
rewrite clearly demonstrates why. D's output is at least predictable
non-garbage, if not expected.

Note the comments, as that's the crux of the problem. D's captures are always
by reference, so equivalent C++ code should do the same. If you capture by
copy, then sure, you'll see a printout of [0, 10) after "With cached
variables", but then the code would not be equivalent.

Back to D land, as mentioned both here and in
https://issues.dlang.org/show_bug.cgi?id=2043, a, uh... "workaround" would be
to force a new frame by turning loop body into e.g. an anonymous lambda call:

    for (int i = 0; i < 10; i++) (int index) {
        dgs ~= () {
            import std.stdio;
            writeln(index);
        };
    } (i);

...but then bye-bye go break and continue :\

Ideally, it'd be great to see by-copy and moving captures in D.

I do support the concern that the behavior is not at all obvious at a glance,
however, mimicking the behavior of C# would be too restrictive, and would make
a different problem not obvious at a glance, as it would hide allocations. Not
that you can't detect that, just that detection won't be "at a glance".

...C++ does this right: forbids implicit captures and makes the programmer
specify exactly how a capture should be performed. IMO, D should follow suit. 

But at the very least, as Vladimir suggested, forming a closure that captures
loop variables by reference should be @system. Code from first message should
fail to compile in @safe, reporting "Closure over variable
`<insert_variable_name_here>` defined in loop scope is not allowed in @safe
code". Further language enhancements should follow, allowing for solutions that
don't require silly wrappers. There shouldn't be ad-hoc fixes though. D has
enough special cases as is.

Could someone make an executive decision as to which of the two reports should
stay open? That this issue is nearing second half of its second decade should
also warrant this to become critical.

--