[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 Nick Treleaven changed: What|Removed |Added CC||n...@geany.org --- Comment #17 from Nick Treleaven --- Issue 23136 shows that this behaviour can violate immutable. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 Iain Buclaw changed: What|Removed |Added Priority|P1 |P2 --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 --- Comment #16 from deadalnix --- It's probably suboptimal, but nevertheless consistent with current semantic. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 --- Comment #15 from Walter Bright --- The solution I outlined in https://issues.dlang.org/show_bug.cgi?id=21929#c10 which is making it an error is the most reasonable one for the way D works. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 --- Comment #14 from Walter Bright --- I don't much care for the "allocate every iteration of the loop" because it may not be obvious to the user that this is happening, and may cause an awful lot of memory to be allocated. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 --- Comment #13 from deadalnix --- (In reply to Walter Bright from comment #10) > Replying to comment #1: > > The observed behavior is a direct result of lambdas in D not capturing by > value, they capture by reference. > This is true of the first exemple, for which things behave apropriately. This is not true of the second one, as, even though it capture by reference, it capture a reference to a new variable every time. > Even allocating a closure on the gc heap is not a solution here, because only > one allocation will be made which will be shared by each lambda, exhibiting > the same behavior. What's nice when most options are incorrect, is that it's fairly obvious what the correct ones are. in this cases, there are 3: - All variables have function scope, like in python. - A new closure is allocated for every loop iteration, like in JavaScript or C#. - Don't pretend you are a safe language in any way like C++ and put the burden of getting things right on the user. Considering D as it stands, it is obvious that the second choice is the correct one. But can also chose to do away with RAII and go for the first option. Or we can get rid of @safe . Or we can ignore the issue for one more decade and hope it goes away as everybody moves to rust. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929
--- Comment #12 from deadalnix ---
(In reply to Stanislav Blinov from comment #9)
> (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
> #include
> #include
>
> int main(int argc, char** argv) {
> std::vector> 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;
> }
>
This code is invalid C++, whatever you deduce from it engages only yourself and
does not involve C++ in any way. In fact, this code could format your hard
drive and and would still be within the C++ spec.
--
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 Tim changed: What|Removed |Added CC||tim.dl...@t-online.de See Also||https://issues.dlang.org/sh ||ow_bug.cgi?id=2043 --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 --- Comment #11 from Walter Bright --- Even allocating a closure on the gc heap is not a solution here, because only one allocation will be made which will be shared by each lambda, exhibiting the same behavior. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929
--- Comment #10 from Walter Bright ---
Replying to comment #1:
The observed behavior is a direct result of lambdas in D not capturing by
value, they capture by reference.
It is also a direct result of loop variables, `i` and `index` although
disappearing at the end of the scope, are recreated in the exact same place.
Hence all the lambda references refer to the same location where each of the
references points to.
The bug here is that the lambda's lifetime, in both loops, exceeds the lifetime
of the variables `i` and `index`.
The error should be emitted when the first lambda, which refers to `i`, is
assigned to dgs[], which has a lifetime longer than `i`. The same goes for the
second lambda and `index`.
Hence, the following simplified code should fail to compile:
@safe
void test() {
int delegate() dg;
foreach (i; 0 .. 10) {
dg = () { return i; };
}
}
--
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929
Stanislav Blinov changed:
What|Removed |Added
CC||stanislav.bli...@gmail.com
Severity|major |critical
--- Comment #9 from Stanislav Blinov ---
(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
#include
#include
int main(int argc, char** argv) {
std::vector> 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
`` 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.
--
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 --- Comment #8 from deadalnix --- (In reply to a11e99z from comment #7) > C++ byRef & byValue capturing is more viable. You'll not that C++'s std::function will allocate on heap if you capture. The equivalent code in C++ WILL allocate in a loop too. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 a11e99z changed: What|Removed |Added CC||blac...@bk.ru --- Comment #7 from a11e99z --- agree with bug. man creating new delegate/lambda with new internal state. what is reason to store just one shared state for created multiple delegetes in loop? what is using case for it? if D-team don't want to break current behavior let add new keyword "lambda" that is 100% compatible with "delegate" but it grabs totally new state when created - just changed compile time generated code. C++ byRef & byValue capturing is more viable. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929
--- Comment #6 from deadalnix ---
(In reply to Walter Bright from comment #4)
> Let's rewrite it to something that does not use closures:
>
> int test() @safe {
> int j;
> int*[20] ps;
>
> for (int i = 0; i < 10; i++) {
> ps[j++] = &i;
> }
>
> for (int i = 0; i < 10; i++) {
> int index = i;
> ps[j++] = &index;
> }
>
> int x;
> foreach (p; ps) {
> x += *p;
> }
>
> return x;
> }
>
> This code is equivalent in terms of what is happening with references and
> scopes.
>
> Compiling it with -dip1000 yields:
>
> Error: address of variable i assigned to ps with longer lifetime
> Error: address of variable index assigned to ps with longer lifetime
>
> Which is pragmatically what the behavior of the delegate example would be,
> because the delegate is also storing a pointer to the variable.
Except it is not. In D, closures do allocate on heap.
--
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 --- Comment #5 from Walter Bright --- The general rule for determining "what should happen here" when there are abstractions around pointers (such as arrays, delegates, refs, outs, class references, etc.), is to rewrite it in explicit terms of those pointers. The (sometimes baffling) behavior is then exposed for what it actually is, and the behavior should match. Granted, there is a special kludge in the compiler to sometimes put the variables referenced by the delegate into a closure allocated by the gc, but that doesn't account for variables that go out of scope before the function scope ends. There is no process to make a closure for them, and adding such a capability is likely much more complication than added value, and so should just be an error. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 Walter Bright changed: What|Removed |Added Keywords||safe --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929
Walter Bright changed:
What|Removed |Added
CC||bugzi...@digitalmars.com
--- Comment #4 from Walter Bright ---
Let's rewrite it to something that does not use closures:
int test() @safe {
int j;
int*[20] ps;
for (int i = 0; i < 10; i++) {
ps[j++] = &i;
}
for (int i = 0; i < 10; i++) {
int index = i;
ps[j++] = &index;
}
int x;
foreach (p; ps) {
x += *p;
}
return x;
}
This code is equivalent in terms of what is happening with references and
scopes.
Compiling it with -dip1000 yields:
Error: address of variable i assigned to ps with longer lifetime
Error: address of variable index assigned to ps with longer lifetime
Which is pragmatically what the behavior of the delegate example would be,
because the delegate is also storing a pointer to the variable.
--
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 Ketmar Dark changed: What|Removed |Added CC||ket...@ketmar.no-ip.org --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 --- Comment #3 from deadalnix --- (In reply to Vladimir Panteleev from comment #1) > In JavaScript: > > [...] > > So, I don't think D's behavior is unexpected compared to other languages. > > Note that if you change "var" to "let" in JS then it behaves closer as to > what you may expect. > > At this point this is part of the language and is not a bug, so I don't > think the bugtracker is the proper place to post this. > > The workaround in D is the same as in JavaScript (before "let"), pass the > mutable values to the lambda so that their current values become part of the > lambda's state (or create an outer lambda which does this). This code is erroneous. In JS, var declare a variable at the function scope level. Try again declaring the variable using `let`, which creates a properly scoped variable in JS and see how it goes. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929 Vladimir Panteleev changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|INVALID |--- --- Comment #2 from Vladimir Panteleev --- (In reply to deadalnix from comment #0) > Failure to do allows for very strange things to happen, such as observing > mutation in immutable variables. This however is a good reason to possibly deprecate/disallow doing so at least in @safe code. --
[Issue 21929] delegates capture do not respect scoping
https://issues.dlang.org/show_bug.cgi?id=21929
Vladimir Panteleev changed:
What|Removed |Added
Status|NEW |RESOLVED
CC||dlang-bugzilla@thecybershad
||ow.net
Resolution|--- |INVALID
--- Comment #1 from Vladimir Panteleev ---
In JavaScript:
---
var dgs = [];
for (var i = 0; i < 10; i++) {
dgs.push(function() {
console.log(i);
});
}
dgs.push(function() {
console.log("With cached variables");
});
for (var i = 0; i < 10; i++) {
var index = i;
dgs.push(function() {
console.log(index);
});
}
for (var dg of dgs) {
dg();
}
---
Prints:
---
10
10
10
10
10
10
10
10
10
10
With cached variables
9
9
9
9
9
9
9
9
9
9
---
So, I don't think D's behavior is unexpected compared to other languages.
Note that if you change "var" to "let" in JS then it behaves closer as to what
you may expect.
At this point this is part of the language and is not a bug, so I don't think
the bugtracker is the proper place to post this.
The workaround in D is the same as in JavaScript (before "let"), pass the
mutable values to the lambda so that their current values become part of the
lambda's state (or create an outer lambda which does this).
--
