Re: Delegate, scope and associative array

2014-06-03 Thread Edwin van Leeuwen via Digitalmars-d-learn

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.


Re: Delegate, scope and associative array

2014-06-03 Thread Ali Çehreli via Digitalmars-d-learn

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



Re: Delegate, scope and associative array

2014-06-03 Thread Vlad Levenfeld via Digitalmars-d-learn
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).


Re: Delegate, scope and associative array

2014-06-03 Thread Vlad Levenfeld via Digitalmars-d-learn

Hah, sorry, I didn't read the last post. I did exactly what Ali
just suggested.


Re: Delegate, scope and associative array

2014-06-03 Thread Edwin van Leeuwen via Digitalmars-d-learn

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


Re: Delegate, scope and associative array

2014-06-03 Thread bearophile via Digitalmars-d-learn

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


Re: Delegate, scope and associative array

2014-06-03 Thread Steven Schveighoffer via Digitalmars-d-learn
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


Re: Delegate, scope and associative array

2014-06-03 Thread via Digitalmars-d-learn
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.


Re: Delegate, scope and associative array

2014-06-02 Thread Rene Zwanenburg via Digitalmars-d-learn

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


Re: Delegate, scope and associative array

2014-06-02 Thread Edwin van Leeuwen via Digitalmars-d-learn

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.