www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - How to avoid variable capturing in `foreach` loop with lambdas?

reply thebluepandabear <therealbluepandabear protonmail.com> writes:
I am using CSFML D bindings and I have created my own sort of UI 
library for drawing elements onto the screen.

One of the classes I've created is a `Button` class, which 
contains a delegate called `onButtonClick` which is called when 
the button is clicked on by the user.

Up until now, everything has worked fine.

I want to add a couple of buttons side by side to represent an 
element of a list and assign them each a unique lambda expression 
for that particular element in the list, this is the current code 
I have:

```D
foreach (BoardSize boardSize; arr) {
     Button button = new Button();
     button.text = format("%sx%s", boardSize[0], boardSize[1]);
     button.onButtonClick = {
         
eventHandler.settingsWindow_onBoardSizeButtonClick(boardSize);
     };
     button.onButtonClick();
     _boardSizeRow.addChild(button);
}
```

Running this code, I had expected that everything would work 
fine. Unfortunately upon running the code, tapping each of the 
buttons returned only the largest `boardSize` value, the one 
which is gets iterated last.

Note that I am still not totally sure why this is the case. At 
first, I was confused, but then I suspected that after the 
variable gets sent as a parameter to the 
`settingsWindow_onBoardSizeButtonClick` function, it gets changed 
again in the next iteration creating a sort of chain effect -- I 
may be wrong, but this is my suspicion.

Some of the things I tried was creating a new object each time, 
although it didn't work. I might have not done this properly as I 
am a beginner to D language. I saw someone else ask a similar 

so it wasn't that much of a use to me.

Help would be appreciated!
Jan 05 2023
next sibling parent reply thebluepandabear <therealbluepandabear protonmail.com> writes:
Update some time later: the only way (oof!) around this seems to 
be using a `static foreach` with arrays:

```D
Button[3] b;

static foreach (indx, BoardSize boardSize; arr) {
     b[indx] = new Button();
     b[indx].text = format("%sx%s", boardSize[0], boardSize[1]);
     b[indx].onButtonClick = {
         
eventHandler.settingsWindow_onBoardSizeButtonClick(boardSize);
     };
     _boardSizeRow.addChild(b[indx]);
}
```

Any other ways of fixing this annoying issue?
Jan 05 2023
parent tsbockman <thomas.bockman gmail.com> writes:
On Thursday, 5 January 2023 at 13:05:46 UTC, thebluepandabear 
wrote:

 Update some time later: the only way (oof!) around this seems 
 to be using a `static foreach` with arrays:

 ```D
 Button[3] b;

 static foreach (indx, BoardSize boardSize; arr) {
     b[indx] = new Button();
     b[indx].text = format("%sx%s", boardSize[0], boardSize[1]);
     b[indx].onButtonClick = {
         
 eventHandler.settingsWindow_onBoardSizeButtonClick(boardSize);
     };
     _boardSizeRow.addChild(b[indx]);
 }
 ```


This is semantically equivalent to copying and pasting the loop 
body `arr.length` number of times, substituting the expression 
`arr[indx]` for `boardSize`, and the literal value of `indx` for 
`indx`.

Unlike with a runtime (non-`static`) loop, the event handler 
closure does not capture a reference to `boardSize` or `indx`, 
because they don't actually exist at runtime and so cannot be 
referenced. Instead, `arr.length` different event handler 
functions are created at compile time, with each having the 
appropriate `indx` literal substituted.

So, while it does work as long as `arr.length` is known at 
compile time, it will cause a ton of needless code bloat unless 
`arr.length` is very small.


 Any other ways of fixing this annoying issue?


Yes, see [my earlier 
reply](https://forum.dlang.org/post/fwnccjunvnffmtaholqc forum.dlang.org) for
an explanation of how to get your original run time loop working as intended.
Jan 05 2023
prev sibling next sibling parent reply tsbockman <thomas.bockman gmail.com> writes:
On Thursday, 5 January 2023 at 11:55:33 UTC, thebluepandabear 
wrote:

 ```D
 foreach (BoardSize boardSize; arr) {
     Button button = new Button();
     button.text = format("%sx%s", boardSize[0], boardSize[1]);
     button.onButtonClick = {
         
 eventHandler.settingsWindow_onBoardSizeButtonClick(boardSize);
     };
     button.onButtonClick();
     _boardSizeRow.addChild(button);
 }
 ```

 Running this code, I had expected that everything would work 
 fine. Unfortunately upon running the code, tapping each of the 
 buttons returned only the largest `boardSize` value, the one 
 which is gets iterated last.


The problem is twofold:

1. Closures in D capture their environment by reference.
2. D (incorrectly, in my opinion) considers loop-local variables 
to have the same identity across each iteration of the loop 
within a single function call.

So, `boardSize` in your event handler is a reference to a single 
variable whose value is overwritten on each iteration of the 
loop. As the event handlers are (I presume) never called until 
after the loop has terminated, the only value they will ever see 
is whichever was set by the final iteration of the loop in that 
function call.

There are at least two possible solutions:

1. Use a `struct` to explicitly capture `boardSize` by value, 
instead of by reference:
```D
static struct ClickHandler {
     // If eventHandler is not a global variable of some sort, add 
another field for it:
     BoardSize iteration_boardSize;
     this(BoardSize iteration_boardSize) {
         this.iteration_boardSize = iteration_boardSize;
     }

     void opCall() {
         
eventHandler.settingsWindow_onBoardSizeButtonClick(iteration_boardSize);
     }
}

foreach (BoardSize loop_boardSize; arr) {
     Button button = new Button();
     button.text = format("%sx%s", loop_boardSize[0], 
loop_boardSize[1]);
     button.onButtonClick = &(new 
ClickHandler(loop_boardSize)).opCall;
     button.onButtonClick();
     _boardSizeRow.addChild(button);
}
```

2. Use a nested function call with a `boardSize` parameter to 
create a copy of `boardSize`  with a unique identity on each 
iteration of the loop:
```D
foreach (BoardSize loop_boardSize; arr) {
     Button button = new Button();
     button.text = format("%sx%s", loop_boardSize[0], 
loop_boardSize[1]);
     button.onButtonClick = (BoardSize iteration_boardSize) {
         return {
             
eventHandler.settingsWindow_onBoardSizeButtonClick(iteration_boardSize);
         };
     }(loop_boardSize);
     button.onButtonClick();
     _boardSizeRow.addChild(button);
}
```

These two solutions should compile to approximately the same 
runtime code, with optimizations enabled. So, it's really down to 
personal preference; the former is more explicit about what the 
computer is to do, while the latter is more concise.
Jan 05 2023
parent thebluepandabear <therealbluepandabear protonmail.com> writes:
 ```

 These two solutions should compile to approximately the same 
 runtime code, with optimizations enabled. So, it's really down 
 to personal preference; the former is more explicit about what 
 the computer is to do, while the latter is more concise.


Thanks! Works great.
Jan 05 2023
prev sibling next sibling parent reply Tejas <notrealemail gmail.com> writes:
On Thursday, 5 January 2023 at 11:55:33 UTC, thebluepandabear 
wrote:

 I am using CSFML D bindings and I have created my own sort of 
 UI library for drawing elements onto the screen.

 One of the classes I've created is a `Button` class, which 
 contains a delegate called `onButtonClick` which is called when 
 the button is clicked on by the user.

 Up until now, everything has worked fine.

 I want to add a couple of buttons side by side to represent an 
 element of a list and assign them each a unique lambda 
 expression for that particular element in the list, this is the 
 current code I have:

 ```D
 foreach (BoardSize boardSize; arr) {
     Button button = new Button();
     button.text = format("%sx%s", boardSize[0], boardSize[1]);
     button.onButtonClick = {
         
 eventHandler.settingsWindow_onBoardSizeButtonClick(boardSize);
     };
     button.onButtonClick();
     _boardSizeRow.addChild(button);
 }
 ```

 Running this code, I had expected that everything would work 
 fine. Unfortunately upon running the code, tapping each of the 
 buttons returned only the largest `boardSize` value, the one 
 which is gets iterated last.

 Note that I am still not totally sure why this is the case. At 
 first, I was confused, but then I suspected that after the 
 variable gets sent as a parameter to the 
 `settingsWindow_onBoardSizeButtonClick` function, it gets 
 changed again in the next iteration creating a sort of chain 
 effect -- I may be wrong, but this is my suspicion.

 Some of the things I tried was creating a new object each time, 
 although it didn't work. I might have not done this properly as 
 I am a beginner to D language. I saw someone else ask a similar 

 D, so it wasn't that much of a use to me.

 Help would be appreciated!


Ah, I think you ran into that delegate bug



Try this code in it's place:

```d
  foreach (BoardSize boardSize; arr) (){ // notice the brackets
      Button button = new Button();
      button.text = format("%sx%s", boardSize[0], boardSize[1]);
      button.onButtonClick = {

  eventHandler.settingsWindow_onBoardSizeButtonClick(boardSize);
      };
      button.onButtonClick();
      _boardSizeRow.addChild(button);
  }() // notice the brackets
```
This is not your fault, it's an "optimization" that's being 
performed by dmd so that "the user doesn't get surprised" when 
they realize that capturing every single variable emitted in a 
`foreach` will allocate a new heap closure

Have fun reading this :

https://issues.dlang.org/show_bug.cgi?id=21929
Jan 05 2023
parent reply thebluepandabear <therealbluepandabear protonmail.com> writes:
 Have fun reading this :

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


Thanks for the code suggestion although it still doesn't fix the 
bug. I am curious as to what those brackets do as well.
Jan 05 2023
parent reply Tejas <notrealemail gmail.com> writes:
On Thursday, 5 January 2023 at 22:49:01 UTC, thebluepandabear 
wrote:

 Have fun reading this :

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


 Thanks for the code suggestion although it still doesn't fix 
 the bug. I am curious as to what those brackets do as well.


Okay, my bad for writing the wrong code, the correct one is:
```d
  foreach (BoardSize boardSizeIter; arr) (Boardsize boardSize){ // 
notice the brackets and I changed the variable names a little
      Button button = new Button();
      button.text = format("%sx%s", boardSize[0], boardSize[1]);
      button.onButtonClick = {

  eventHandler.settingsWindow_onBoardSizeButtonClick(boardSize);
      };
      button.onButtonClick();
      _boardSizeRow.addChild(button);
  }(boardSizeIter) // notice the brackets and that I changed the 
name of you first foreach argument
```

This will do the same thing as HS Teoh and Tsbockman's code did : 
create a copy of the value that is currently present in one 
particular iteration of the `foreach` by creating a function 
literal that takes your `struct` as a paramter. This works 
because structs are value types, so passing it to the function 
creates a copy.
Jan 05 2023
parent thebluepandabear <therealbluepandabear protonmail.com> writes:
 : create a copy of the value that is currently present in one 
 particular iteration of the `foreach` by creating a function 
 literal that takes your `struct` as a paramter. This works 
 because structs are value types, so passing it to the function 
 creates a copy.


Thanks, your solution seems to be the most elegant one yet, I 
recently came across the similar problem and I realized that the 
other answer was too long and verbose.
Jan 08 2023
prev sibling parent reply "H. S. Teoh" <hsteoh qfbox.info> writes:
On Thu, Jan 05, 2023 at 11:55:33AM +0000, thebluepandabear via
Digitalmars-d-learn wrote:
[...]

 ```D
 foreach (BoardSize boardSize; arr) {
     Button button = new Button();
     button.text = format("%sx%s", boardSize[0], boardSize[1]);
     button.onButtonClick = {
 eventHandler.settingsWindow_onBoardSizeButtonClick(boardSize);
     };
     button.onButtonClick();
     _boardSizeRow.addChild(button);
 }
 ```


This is a classic D trap: the loop variable is only allocated once, and
the closure captures the single location where the loop variable
resides, thus every delegate from every loop iteration will see the same
value when they are run later, i.e., the last value of the loop index.
Do this instead:

```D
foreach (BoardSize boardSize; arr) {
    Button button = new Button();
    button.text = format("%sx%s", boardSize[0], boardSize[1]);
    BoardSize size = boardSize; // force separate capture
    button.onButtonClick = {
        eventHandler.settingsWindow_onBoardSizeButtonClick(size);
    };
    button.onButtonClick();
    _boardSizeRow.addChild(button);
}
```

This is arguably a language bug (I can't imagine any sane use case where
somebody would actually want the current semantics).  But we have not be
successful in convincing Walter about this...


T

-- 
Don't throw out the baby with the bathwater. Use your hands...
Jan 05 2023
parent reply thebluepandabear <therealbluepandabear protonmail.com> writes:
On Thursday, 5 January 2023 at 17:36:55 UTC, H. S. Teoh wrote:

 On Thu, Jan 05, 2023 at 11:55:33AM +0000, thebluepandabear via 
 Digitalmars-d-learn wrote: [...]

 ```D
 foreach (BoardSize boardSize; arr) {
     Button button = new Button();
     button.text = format("%sx%s", boardSize[0], boardSize[1]);
     button.onButtonClick = {
 eventHandler.settingsWindow_onBoardSizeButtonClick(boardSize);
     };
     button.onButtonClick();
     _boardSizeRow.addChild(button);
 }
 ```


 This is a classic D trap: the loop variable is only allocated 
 once, and the closure captures the single location where the 
 loop variable resides, thus every delegate from every loop 
 iteration will see the same value when they are run later, 
 i.e., the last value of the loop index. Do this instead:

 ```D
 foreach (BoardSize boardSize; arr) {
     Button button = new Button();
     button.text = format("%sx%s", boardSize[0], boardSize[1]);
     BoardSize size = boardSize; // force separate capture
     button.onButtonClick = {
         
 eventHandler.settingsWindow_onBoardSizeButtonClick(size);
     };
     button.onButtonClick();
     _boardSizeRow.addChild(button);
 }
 ```

 This is arguably a language bug (I can't imagine any sane use 
 case where somebody would actually want the current semantics). 
  But we have not be successful in convincing Walter about 
 this...


 T


Your code with the variable capture doesn't seem to work.
Jan 05 2023
parent reply "H. S. Teoh" <hsteoh qfbox.info> writes:
On Thu, Jan 05, 2023 at 10:46:07PM +0000, thebluepandabear via
Digitalmars-d-learn wrote:

 On Thursday, 5 January 2023 at 17:36:55 UTC, H. S. Teoh wrote:


[...]

 ```D
 foreach (BoardSize boardSize; arr) {
     Button button = new Button();
     button.text = format("%sx%s", boardSize[0], boardSize[1]);
     BoardSize size = boardSize; // force separate capture
     button.onButtonClick = {
 eventHandler.settingsWindow_onBoardSizeButtonClick(size);
     };
     button.onButtonClick();
     _boardSizeRow.addChild(button);
 }
 ```




[...]

 Your code with the variable capture doesn't seem to work.


Argh, apparently locals inside the loop body are subject to the same
quirky behaviour. >:-(  Here's a workaround that works:

	foreach (BoardSize boardSize; arr) {
	    Button button = new Button();
	    button.text = format("%sx%s", boardSize[0], boardSize[1]);
	    void wowThisIsDumb(BoardSize size) {
		    button.onButtonClick = {
			eventHandler.settingsWindow_onBoardSizeButtonClick(size);
		    };
	    }
	    wowThisIsDumb(boardSize);
	    button.onButtonClick();
	    _boardSizeRow.addChild(button);
	}

A nested function (or perhaps an inline lambda) is needed to force the
allocation of a dynamic context for the capture.

This is an embarrassment. Why hasn't this been fixed yet? :-(


T

-- 
Being able to learn is a great learning; being able to unlearn is a greater
learning.
Jan 05 2023
parent reply thebluepandabear <therealbluepandabear protonmail.com> writes:
 A nested function (or perhaps an inline lambda) is needed to 
 force the allocation of a dynamic context for the capture.

 This is an embarrassment. Why hasn't this been fixed yet? :-(


 T


I agree that this needs to get fixed immediately, it seems to be 
bugging me another time as well.

I believe I saw a similar StackOverflow post about this issue for 

this will improve the quality of the language for newcomers such 
as myself greatly.
Jan 08 2023
parent reply thebluepandabear <therealbluepandabear protonmail.com> writes:
 Fixing this will improve the quality of the language for 
 newcomers such as myself greatly.


This not only confuses newcomers but it gave a false illusion of 
a bug within the code :/
Jan 08 2023
parent bauss <jacobbauss gmail.com> writes:
On Monday, 9 January 2023 at 00:19:39 UTC, thebluepandabear wrote:

 Fixing this will improve the quality of the language for 
 newcomers such as myself greatly.


 This not only confuses newcomers but it gave a false illusion 
 of a bug within the code :/


It doesn't confuse newcomers only, also people who have used D 
for over a decade like me :) Mostly because it's not a thing in 
most (if not all) other languages.

I think D is unique in that aspect.

It should be fixed however, I don't think the current behavior is 
correct or expected by anyone.
Jan 09 2023