www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Delegate, scope and associative array

reply "Edwin van Leeuwen" <edder tkwsping.nl> writes:
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
Jun 02 2014
parent reply "Rene Zwanenburg" <renezwanenburg gmail.com> writes:
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
Jun 02 2014
next sibling parent reply "Edwin van Leeuwen" <edder tkwsping.nl> writes:
On Monday, 2 June 2014 at 23:44:01 UTC, Rene Zwanenburg wrote:
 On Monday, 2 June 2014 at 20:09:12 UTC, Edwin van Leeuwen wrote:
 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
Thanks for the suggestion and I just tried it, but it does not work :( In the original code were I discovered this problem I generated the id with an outside function and that also didn't to work.
Jun 02 2014
next sibling parent "Edwin van Leeuwen" <edder tkwsping.nl> writes:
On Tuesday, 3 June 2014 at 05:40:44 UTC, Edwin van Leeuwen wrote:
 On Monday, 2 June 2014 at 23:44:01 UTC, Rene Zwanenburg wrote:
 On Monday, 2 June 2014 at 20:09:12 UTC, Edwin van Leeuwen 
 wrote:
 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
Thanks for the suggestion and I just tried it, but it does not work :( In the original code were I discovered this problem I generated the id with an outside function and that also didn't to work.
unittest { size_t delegate()[size_t] events; size_t i = 1; events[i] = { return i; }; i = 2; events[i] = { return i; }; i = 3; events[i] = { return i; }; writeln( events[1]() ); assert( events[1]() == 1 ); } Explicitly removing the loop still causes the same issue. In that case I find it easier to understand, since it might be using the value of i at the end of the scope. In the foreach case (and especially when copying to a local variable) it is more puzzling to me.
Jun 02 2014
prev sibling parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 06/02/2014 10:40 PM, Edwin van Leeuwen wrote:
 On Monday, 2 June 2014 at 23:44:01 UTC, Rene Zwanenburg wrote:
 On Monday, 2 June 2014 at 20:09:12 UTC, Edwin van Leeuwen wrote:
 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
Thanks for the suggestion and I just tried it, but it does not work :( In the original code were I discovered this problem I generated the id with an outside function and that also didn't to work.
Here is a workaround: unittest { size_t delegate()[size_t] events; auto makeClosure(size_t i) { return { return i; }; } foreach( i; 1..4 ) { events[i] = makeClosure(i); } assert( events[1]() == 1 ); } void main() {} Ali
Jun 03 2014
next sibling parent reply "Vlad Levenfeld" <vlevenfeld gmail.com> writes:
I've run across this myself. The workaround I used was to call a 
function from inside the foreach loop, and in that function you 
construct the delegate (with the index variable passed as a 
parameter).
Jun 03 2014
parent "Vlad Levenfeld" <vlevenfeld gmail.com> writes:
Hah, sorry, I didn't read the last post. I did exactly what Ali
just suggested.
Jun 03 2014
prev sibling next sibling parent "Edwin van Leeuwen" <edder tkwsping.nl> writes:
On Tuesday, 3 June 2014 at 07:00:35 UTC, Ali Çehreli wrote:
 Here is a workaround:

 unittest {
     size_t delegate()[size_t] events;

     auto makeClosure(size_t i) {
         return { return i; };
     }

     foreach( i; 1..4 ) {
         events[i] = makeClosure(i);
     }

     assert( events[1]() == 1 );
 }

 void main()
 {}

 Ali
Thank you Ali, that works beautifully and can be easily adapted to the original (more complicated) case. Cheers, Edwin
Jun 03 2014
prev sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
Ali Çehreli:

 Here is a workaround:

 unittest {
     size_t delegate()[size_t] events;

     auto makeClosure(size_t i) {
         return { return i; };
     }

     foreach( i; 1..4 ) {
         events[i] = makeClosure(i);
     }
You can also use two lambdas to do that, without the "makeClosure": http://rosettacode.org/wiki/Closures/Value_capture#Less_Functional_Version Also don't forget do make the foreach argument immutable (I think this is a lost war for me. Even smart programmers like you stick to the defaults, even when they are a design mistake of the language. So immutability by default is very important). Bye, bearophile
Jun 03 2014
prev sibling parent reply "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Mon, 02 Jun 2014 19:43:58 -0400, Rene Zwanenburg  
<renezwanenburg 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
Jun 03 2014
parent "Marc =?UTF-8?B?U2Now7x0eiI=?= <schuetzm gmx.net> writes:
On Tuesday, 3 June 2014 at 13:30:13 UTC, Steven Schveighoffer 
wrote:
 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.
This is already a level too deep - it closes over variables, not the stack frame. And from this point of view, the current behavior is clearly wrong, because there is a different variable in each iteration; it just happens to have the same name.
Jun 03 2014