www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 2043] New: Closure outer variables in nested blocks are not allocated/instantiated correctly.

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2043

           Summary: Closure outer variables in nested blocks are not
                    allocated/instantiated correctly.
           Product: D
           Version: 2.012
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla digitalmars.com
        ReportedBy: brunodomedeiros+bugz gmail.com


An outer variable declared in a nested (and scoped) block that is captured by a
closure will be allocated on the heap, but only once per function execution,
instead of once per block execution, which is what makes sense.
See code below:

--- ---
void delegate()[] dgList;

void test() {

        foreach(int i; [1, 2, 3]) {
                int b = 2;
                dgList ~= { writefln(b); };
                writefln(&b); // b will be the *same* on each iteration
        }
}
---


-- 
Apr 26 2008
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2043


brunodomedeiros+bugz gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |benoit tionex.de





*** Bug 2228 has been marked as a duplicate of this bug. ***


-- 
Jul 27 2008
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2043






Here's a better illustration of the problem:

void delegate() dg;
void main()
{
    foreach(int i; 0 .. 2)
    {
        invariant int b = i;
        if (i == 0)
            dg = { printf("b = %d\n", b); };
        dg();
    }
    dg();
}

'b' is supposed to be invariant, yet its value (as seen by dg()) changes. One
possible fix for this is to make invariants that are initialized with varying
values be illegal to reference from a delegate.


-- 
Aug 14 2008
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2043







 Here's a better illustration of the problem:
 void delegate() dg;
 void main()
 {
     foreach(int i; 0 .. 2)
     {
         invariant int b = i;
         if (i == 0)
             dg = { printf("b = %d\n", b); };
         dg();
     }
     dg();
 }
 'b' is supposed to be invariant, yet its value (as seen by dg()) changes. One
 possible fix for this is to make invariants that are initialized with varying
 values be illegal to reference from a delegate.
I'm assuming that a variable declared inside a loop conceptually represents not a single variable that gets reinitialized with each iteration, but instead represents several different variable "instances" (so to speak), each "created" in each iteration. I vaguely recall you confirming this notion (which is indeed what makes more sense and is more consistent), but I can't pinpoint the comment. Given that assumption, then your code is *not* a better illustration of the problem. This isn't specific to invariant, and the problem is not exactly that b's invariance is broken, but rather that D fails to create several "instances" of b. In other words, I would expect your example to work the same as this code: ----------- void delegate() dg; void foreachBody(int i) { invariant int b = i; if (i == 0) dg = { printf("b = %d\n", b); }; dg(); } void main(string[] args) { foreach(int i; 0 .. 2) foreachBody(i); dg(); } ----------- I hope the example above clarifies what I mean by saying that b (and any variables inside the loop) should be "different" variables every time the scope is entered. --
Aug 15 2008
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2043






Given this, the Java syntax makes much sense.
There, a variable that is accessible from an anonymous class must be marked as
'final'. This makes it easy, because the variable can be copied to the
anonymous class.

Perhaps D should consider to recreate a new concept for closures.

E.g. the old nested closure which are not allocated on the heap can access
everything. And the new full closure can only access 'final' vars and is
created with a special syntax like the suggestion from Sean "& new {}". Then
not the outside stackframe is heap allocated, instead the closure has an
allocated from with a copy of the accessed final vars.


-- 
Aug 15 2008
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2043


Bruno Medeiros <bdom.pub+deebugz gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nfxjfg gmail.com



08:59:46 PST ---
*** Issue 4966 has been marked as a duplicate of this issue. ***

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 19 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2043


Witold Baryluk <baryluk smp.if.uj.edu.pl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |baryluk smp.if.uj.edu.pl



18:20:46 PST ---
I today hit this bug. It really isn't obvious how to workaround this problem.
It is not possible to easly just allocate variable or struct with variable's
value manually, as it still will just by single identifier, and it will at the
end still point to single entity.

Simplified case, which returns array of delegates each printing next integer.

  import std.stdio;
  alias void delegate() D;
  /* do not work, */
  auto f(int n) {
        auto tab = new D[n];
        for (int i = 0; i < n; i++) {
                // invariant ii = i; // will not help
                // struct S { int i_; }; S x = new S; x.i_ = i;
                // will also not help, as x is on stack
                tab[i] = delegate void() { writefln("i=%d", i); };
                // also using nested function, and taking address do not work
        }
        return tab;
  }

  void main() {
        auto tab = f(10);
        foreach (k, ref x; tab) {
                writef("%d : ", k);
                x();
        }
  }

It currently will print in all cases:

  0 : i=10
  1 : i=10
  2 : i=10
  3 : i=10
  4 : i=10
  5 : i=10
  6 : i=10
  7 : i=10
  8 : i=10
  9 : i=10


My current workaround involves looping using recursion (and be sure that
compiler do not make it tail-recursive probably):

/* recursive loop version will work */
auto f2(int n) {
    auto tab = new D[n];
    void f2r(int i) {
        tab[i] = delegate void() { writefln("i=%d", i); };
        if (i < n-1) {
            f2r(i+1); /* recursion */
        }
    }
    f2r(0); /* enter recursion */
    return tab;
}

Which will make our constructed delegates work as desired:

  0 : i=0
  1 : i=1
  2 : i=2
  3 : i=3
  4 : i=4
  5 : i=5
  6 : i=6
  7 : i=7
  8 : i=8
  9 : i=9

(I tested if this is not just a a coincidence, but overwriting stack by random
values, and it works still correctly. BTW. switch for compiler which will show
what local variables are/will automatically allocated on heap will be usefull).

As of definitive solution, i think "invariant" which is referenced by escaping
delegate should be allocated on heap, and its addresses should be remembered
immediately in constructed delegate. Other possibility is separate syntax, like
"new delegate void() ... ", which will make all variables referenced by
delegate a copies of current values of variables with the same names
(considering scoping rules).


Hope this will be helpful for somebody.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2043


timon.gehr gmx.ch changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |timon.gehr gmx.ch




 ...
 Simplified case, which returns array of delegates each printing next integer.
 
   import std.stdio;
   alias void delegate() D;
   /* do not work, */
   auto f(int n) {
         auto tab = new D[n];
         for (int i = 0; i < n; i++) {
                 // invariant ii = i; // will not help
                 // struct S { int i_; }; S x = new S; x.i_ = i;
                 // will also not help, as x is on stack
                 tab[i] = delegate void() { writefln("i=%d", i); };
                 // also using nested function, and taking address do not work
         }
         return tab;
   }
 
   void main() {
         auto tab = f(10);
         foreach (k, ref x; tab) {
                 writef("%d : ", k);
                 x();
         }
   }
 
 It currently will print in all cases:
 
   0 : i=10
   1 : i=10
   2 : i=10
   3 : i=10
   4 : i=10
   5 : i=10
   6 : i=10
   7 : i=10
   8 : i=10
   9 : i=10
 
 
 My current workaround involves looping using recursion (and be sure that
 compiler do not make it tail-recursive probably):
 [snip.]
There is a much simpler workaround: import std.stdio; alias void delegate() D; auto f(int n) { auto tab = new D[n]; for (int i = 0; i < n; i++) (){ int i = i; tab[i] = delegate void() { writefln("i=%d", i); }; }(); return tab; } void main() { auto tab = f(10); foreach (k, ref x; tab) { writef("%d : ", k); x(); } } -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jun 19 2012
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=2043


Russ Lewis <webmaster villagersonline.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |webmaster villagersonline.c
                   |                            |om



23:52:25 PST ---
Comment 7's workaround is a little brilliant, though terribly hard to read. 
Basically, it's declaring a delegate with the
    () {
       <code>
    }
syntax, and calling it (with a trailing () ), all inline in each pass of the
for() loop.

This works around the problem because, as far as I can tell, the DMD compiler
will make only 1 heap copy of any given variable *per instance of a function
call*, even if the variable is a loop-local variable.  So, if you run 10
iterations of a for() loop, and create a delegate in each one, then all 10
iterations of the loop share the same heap space for their variables, even the
loop-local ones (which ought to have 10 different copies).  But, if you instead
call the same function 10 times in a row (as comment 7 effectively does), then
you get 10 different copies of the variable on the heap.

I can understand why this is a hard problem to solve on the compiler
side...but, IMHO, it's a fairly important one to solve.  This bug causes some
rather hard-to-understand bugs in what would otherwise look like
straightforward code.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 05 2013