On Mon, 02 Jun 2014 19:43:58 -0400, Rene Zwanenburg
<renezwanenb...@gmail.com> wrote:
On Monday, 2 June 2014 at 20:09:12 UTC, Edwin van Leeuwen wrote:
I'm probably missing something basic, but I am confused by what is
going on in the following code.
unittest {
size_t delegate()[size_t] events;
foreach( i; 1..4 ) {
events[i] = { return i; };
}
writeln( events[1]() ); // This outputs 3
assert( events[1]() == 1 );
}
I thought that i should be copied from the local scope and therefore
when I call events[1]() the return value should be 1, but in this case
it is 3 (it always seems to be the last value of i in the loop).
Cheers, Edwin
Yeah this is a known problem. At the moment the foreach loop is
internally rewritten to something like
int i = 1;
while(i < 4)
{
// foreach loop body
++i;
}
While it usually would be preferrable if it were rewritten as
int __compilergeneratedname = 1;
while(__compilergeneratedname < 4)
{
auto i = __compilergeneratedname++;
// foreach loop body
}
With the current approach every delegate refers to the same variable.
Another problem is that it's possible to alter the iteration index from
inside the foreach body.
As you may have guessed, a workaround is to copy the iteration variable
yourself:
unittest {
size_t delegate()[size_t] events;
foreach(_i; 1..4 ) {
auto i = _i;
events[i] = { return i; };
}
assert( events[1]() == 1 );
}
This should work though it's less than ideal. There is an open bug
report:
https://issues.dlang.org/show_bug.cgi?id=8621
There is a school of thought (to which I subscribe) that says you
shouldn't allocate a separate closure for each loop iteration.
Basically, a closure will only use the stack frame of the calling
function, not any loop iteration.
This way, you can clearly delineate what scope the closure has, and avoid
unnecessary allocations.
-Steve
