[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 Walter Bright changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |MOVED --- Comment #44 from Walter Bright --- Closed in favor of https://issues.dlang.org/show_bug.cgi?id=23136 which has an accurate example. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 Walter Bright changed: What|Removed |Added See Also||https://issues.dlang.org/sh ||ow_bug.cgi?id=23136 --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 Tim changed: What|Removed |Added See Also||https://issues.dlang.org/sh ||ow_bug.cgi?id=21929 --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 Adam D. Ruppe changed: What|Removed |Added CC||destructiona...@gmail.com --- Comment #43 from Adam D. Ruppe --- > test.d(13): Error: reference to local variable `b` assigned to non-scope > parameter `_param_0` This error is on the *writeln* line, not the actual capture line. It has absolutely nothing to do with the actual bug. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #42 from Artem Borisovskiy --- (In reply to Walter Bright from comment #40) > > dmd test.c -preview=dip1000 Is DIP1000 going to become the standard behaviour anytime soon? I left D, my former favourite language, for good 4 years ago because of this ancient bug (more like it has become the last straw that broke the camel's back). It turned out easier to figure out how Rust lifetimes work, than to not shoot myself in the foot in D, a GC-based language, that's supposed to be even safer w.r.t. memory. What's even the current state of DIP1000? --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 timon.g...@gmx.ch changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|WORKSFORME |--- --- Comment #41 from timon.g...@gmx.ch --- The correct behavior for a delegate literal that outlives a referenced variable is to allocate a closure. This case is not an exception. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043
Walter Bright changed:
What|Removed |Added
Status|NEW |RESOLVED
Resolution|--- |WORKSFORME
--- Comment #40 from Walter Bright ---
Note that dgList exceeds the lifetime of b, and should fail to compile for that
reason.
Let's try it (adding import and @safe and updating the code to D2):
import std.stdio;
@safe:
void delegate()[] dgList;
void test() {
foreach(int i; [1, 2, 3]) {
int b = 2;
dgList ~= { writeln(b); };
writeln(&b); // b will be the *same* on each iteration
}
}
> dmd test.c -preview=dip1000
test.d(13): Error: reference to local variable `b` assigned to non-scope
parameter `_param_0`
This is correct behavior. There is no way D should allow references to
variables that went out of scope.
--
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 Bolpat changed: What|Removed |Added CC||qs.il.paperi...@gmail.com --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #39 from Artem Borisovskiy --- Sorry for off-topic, but this discussion brought back memories of the classic song by Frank C Mantra: It's a feature, not a bug, No one really gives a *cough*, So if you want it to be fixed, Go on and fork it, badam badam pap-pa, How hard can it be, badam badam pap-pa, Trust me, it's worth it, badam badam pap-pa, In 10 years you'll se! --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 greenify changed: What|Removed |Added CC||greeen...@gmail.com --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043
--- Comment #38 from timon.g...@gmx.ch ---
(In reply to Walter Bright from comment #37)
> (In reply to timon.gehr from comment #36)
> > (In reply to Walter Bright from comment #35)
> > As I said, just do it if the delegate does not escape.
>
> Yes, I thought this was clear. This only applies if the delegate escapes.
>
> > > 2. Doing a heap alloc/free for every loop iteration is expensive, and
> > > since
> > > the cost is hidden it will come as a nasty surprise.
> > > ...
> >
> > I don't get how this is different from other implicit heap closure
> > allocations. A function that does such an implicit allocation not within a
> > loop in its own body might well be called in a loop and cause a performance
> > bottleneck. Profile.
>
> People have made it clear they don't particularly like hidden allocations in
> innocuous looking code.
Some people. Others just want obvious code to do the obvious thing.
Also, closures escaped from the loop body don't look innocuous.
> Hence the genesis of the @nogc attribute.
Exactly. I'm not proposing anything that would not be catched by @nogc.
> For this
> particular issue, it would be hard to look at a random loop and see if
> allocations are occurring
Not really. Annotate it with @nogc and you will see. This is really not a new
issue. This is about consistently applying an existing rule.
> - i.e. a nasty surprise if a small change suddenly
> made a big hit in performance. Profiling is not the answer, as very, very
> few people profile code.
> ...
A "big hit in performance" is noticeable.
>
> > > 3. Doing a gc allocation will generate an unbounded set of allocations,
> > > also
> > > coming as a nasty surprise.
> > > ...
> > This is true for all implicit heap allocations. They are also often
> > convenient and there are already ways to flag them.
>
> I can hear the complaints about this already :-(
> ...
There will also be complaints about missing closure support.
>
> > > 4. Doing my delegate rewrite will cause a "by value" which changes the
> > > semantics, so that won't work.
> > > ...
> > That's not true.
>
> I believe it is:
>
> int i = 0;
> auto dg = { ++i; }
> dg();
> print(i); // zero or one?
>
> By ref would make it one, with the rewrite it would be 0 because the ref
> would be to a copy of i, not the original.
> ...
No, the rewrite would wrap the loop body and nothing else. foreach loops have a
lowering similar to the following:
foreach(i; 0..n) delegates~=()=>i;
->
for(int __i=0;__ii;
}
Here, the rewrite would be:
for(int __i=0;__ii;
}()
This has the same semantics as before, and every closure gets its own copy of
i, as this is a different variable for each of them (because it is declared in
the loop body).
If you just had the for loop:
for(int i=0;ii;
}
Then the (unnecessary!) rewrite would be:
for(int i=0;ii;
}();
And all closures in the body get the same copy of i, as they all see the same
version. In this case, in the end, every closure would point to the same i and
the value of that i would be n. For this case, no additional implicit
allocations (gc or non-gc) would be necessary.
> > The delegate rewrite is a very hacky way
> > to implement it though, and probably not really the most convenient as you
> > need to thread through control flow and the generated code will not be as
> > good as it could be.
>
> Things get very sticky if you've got closures from functions nested several
> levels deep. It turns out to be pretty hard to get all this stuff right.
> Doing the rewrite means I know it will be correct, as it leverages all that
> complex, debugged, working code.
> ...
You know that that part will be correct. Wrapping the loop body into a function
does not in general result in valid code though. (E.g. break, continue, early
returns, interactions with scope.) Therefore, there need to be additional
hacks.
> ...
> BTW, I belatedly realized that it wasn't just about loop variables.
> Capturing non-loop variables that get destructed when they go out of scope
> also cannot be done.
Yes, this is also an issue, though not related to the current one.
--
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043
--- Comment #37 from Walter Bright ---
(In reply to timon.gehr from comment #36)
> (In reply to Walter Bright from comment #35)
> As I said, just do it if the delegate does not escape.
Yes, I thought this was clear. This only applies if the delegate escapes.
> > 2. Doing a heap alloc/free for every loop iteration is expensive, and since
> > the cost is hidden it will come as a nasty surprise.
> > ...
>
> I don't get how this is different from other implicit heap closure
> allocations. A function that does such an implicit allocation not within a
> loop in its own body might well be called in a loop and cause a performance
> bottleneck. Profile.
People have made it clear they don't particularly like hidden allocations in
innocuous looking code. Hence the genesis of the @nogc attribute. For this
particular issue, it would be hard to look at a random loop and see if
allocations are occurring - i.e. a nasty surprise if a small change suddenly
made a big hit in performance. Profiling is not the answer, as very, very few
people profile code.
> > 3. Doing a gc allocation will generate an unbounded set of allocations, also
> > coming as a nasty surprise.
> > ...
> This is true for all implicit heap allocations. They are also often
> convenient and there are already ways to flag them.
I can hear the complaints about this already :-(
> > 4. Doing my delegate rewrite will cause a "by value" which changes the
> > semantics, so that won't work.
> > ...
> That's not true.
I believe it is:
int i = 0;
auto dg = { ++i; }
dg();
print(i); // zero or one?
By ref would make it one, with the rewrite it would be 0 because the ref would
be to a copy of i, not the original.
> The delegate rewrite is a very hacky way
> to implement it though, and probably not really the most convenient as you
> need to thread through control flow and the generated code will not be as
> good as it could be.
Things get very sticky if you've got closures from functions nested several
levels deep. It turns out to be pretty hard to get all this stuff right. Doing
the rewrite means I know it will be correct, as it leverages all that complex,
debugged, working code.
> > I suspect the pragmatic solution is to disallow the capture of loop
> > variables by reference by escaping delegates. The user then will be notified
> > of the problem, and he can construct an efficient workaround that works for
> > his application.
>
> This fixes the memory corruption issue without much work. Deprecation might
> be necessary even if we fix this the right way in the end, so this is a good
> first step.
At least we agree on that step :-), although I am far less sanguine about
adding hidden allocations inside loops being an acceptable option.
BTW, I belatedly realized that it wasn't just about loop variables. Capturing
non-loop variables that get destructed when they go out of scope also cannot be
done.
--
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #36 from timon.g...@gmx.ch --- (In reply to Walter Bright from comment #35) > 1. alloca() won't work, because that won't survive the end of the function. > ... As I said, just do it if the delegate does not escape. > 2. Doing a heap alloc/free for every loop iteration is expensive, and since > the cost is hidden it will come as a nasty surprise. > ... I don't get how this is different from other implicit heap closure allocations. A function that does such an implicit allocation not within a loop in its own body might well be called in a loop and cause a performance bottleneck. Profile. > 3. Doing a gc allocation will generate an unbounded set of allocations, also > coming as a nasty surprise. > ... This is true for all implicit heap allocations. They are also often convenient and there are already ways to flag them. > 4. Doing my delegate rewrite will cause a "by value" which changes the > semantics, so that won't work. > ... That's not true. It just gets rid of the memory corruption. The capture is still by reference, which you can easily verify by creating more than one closure over the same instance of the variable. This is the standard semantics for this kind of thing. The delegate rewrite is a very hacky way to implement it though, and probably not really the most convenient as you need to thread through control flow and the generated code will not be as good as it could be. > I suspect the pragmatic solution is to disallow the capture of loop > variables by reference by escaping delegates. The user then will be notified > of the problem, and he can construct an efficient workaround that works for > his application. This fixes the memory corruption issue without much work. Deprecation might be necessary even if we fix this the right way in the end, so this is a good first step. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #35 from Walter Bright --- 1. alloca() won't work, because that won't survive the end of the function. 2. Doing a heap alloc/free for every loop iteration is expensive, and since the cost is hidden it will come as a nasty surprise. 3. Doing a gc allocation will generate an unbounded set of allocations, also coming as a nasty surprise. 4. Doing my delegate rewrite will cause a "by value" which changes the semantics, so that won't work. I suspect the pragmatic solution is to disallow the capture of loop variables by reference by escaping delegates. The user then will be notified of the problem, and he can construct an efficient workaround that works for his application. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #34 from timon.g...@gmx.ch --- (In reply to timon.gehr from comment #33) > ... > In D, if the compiler can prove that closures don't escape, alloca might be > an option. (Or simply malloc/free, of course.) --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #33 from timon.g...@gmx.ch --- (In reply to Walter Bright from comment #32) > > Other programming languages, even those that always capture by reference, > > do not have this problem. > > So, for a variable declared in a loop, where do they allocate storage for it? For some, this is an implementation detail, others explicitly specify heap allocation. In D, if the compiler can prove that closures don't escape, alloca might be an option. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #32 from Walter Bright --- > Other programming languages, even those that always capture by reference, do > not have this problem. So, for a variable declared in a loop, where do they allocate storage for it? --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #31 from timon.g...@gmx.ch --- (timon.gehr meant to write in comment #30) > (In reply to Walter Bright from comment #28) > > Added the `safe` keyword because of the immutable variable issue described > > in the comments. > > This is not a one-off issue. It is a symptom of the generally wrong > behavior. DMD is sharing the same memory for different variables with > *overlapping* lifetimes. This is plain memory corruption, whether or not > immutable is involved. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #30 from timon.g...@gmx.ch --- (In reply to Walter Bright from comment #28) > Added the `safe` keyword because of the immutable variable issue described > in the comments. This is not a one-off issue. It is a symptom of the generally wrong behavior. DMD is sharing the same memory for different variables with non-overlapping lifetimes. This is plain memory corruption, whether or not immutable is involved. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #29 from timon.g...@gmx.ch --- (In reply to Walter Bright from comment #27) > D closures are "by reference" rather than "by value". I make use of it being > by reference all the time, as well as it being much more efficient. Changing > it would likely break all sorts of code. So then the question is, what to do > if you want it by value? > ... No, this is not about this distinction. Other programming languages, even those that always capture by reference, do not have this problem. This form of memory corruption cannot reasonably be defined away as a language feature. > Now, the compiler could actually rewrite the delegate into the nested form > above, but as I said, this changes the semantics of existing code, as well as > being pretty inefficient. I doubt it would be acceptable. All bug fixes change behavior of existing code. The current behavior is unacceptable. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 Walter Bright changed: What|Removed |Added Keywords||safe --- Comment #28 from Walter Bright --- Added the `safe` keyword because of the immutable variable issue described in the comments. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043
--- Comment #27 from Walter Bright ---
D closures are "by reference" rather than "by value". I make use of it being by
reference all the time, as well as it being much more efficient. Changing it
would likely break all sorts of code. So then the question is, what to do if
you want it by value?
One solution to creating such a delegate is to create it inside a nested
function:
import core.stdc.stdio;
int main()
{
foreach (i; 0 .. 2)
{
void delegate(int) nested()
{
auto myi = i; // capture current value of i for use by del()
void del(int x) // the real delegate
{
printf("&myi = %p\n", &myi);
}
return &del;
}
auto dg = nested();
dg(1);
}
return 0;
}
Running it:
&myi = 00261014
&myi = 00261024
This works because `del` escapes its scope, and so the compiler allocates a
closure each time nested() is called, and `myi` is captured in that closure.
Now, the compiler could actually rewrite the delegate into the nested form
above, but as I said, this changes the semantics of existing code, as well as
being pretty inefficient. I doubt it would be acceptable.
--
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #26 from Artem Borisovskiy --- Buckle up for celebration of 10 year anniversary and don't forget flowers for Walter. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 ag0ae...@gmail.com changed: What|Removed |Added CC||alexandru.razva...@gmail.co ||m --- Comment #25 from ag0ae...@gmail.com --- *** Issue 18201 has been marked as a duplicate of this issue. *** --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 calex changed: What|Removed |Added CC||calex+bugzilla-mail@aristow ||eb.net --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 ZombineDev changed: What|Removed |Added CC||petar.p.ki...@gmail.com --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 Edwin van Leeuwen changed: What|Removed |Added CC||ed...@tkwsping.nl --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #24 from timon.g...@gmx.ch --- (In reply to Walter Bright from comment #21) > The most practical solution is to simply disallow referencing variables from > a dynamic closure that end sooner than the closing } of the function. > Why? Lexical scoping exists for a reason. > This leaves it up to the user to decide how to handle it, such as passing > the variable explicitly as an argument, declaring the variable at function > scope, or copying the variable to another declared at function scope. It's up to the user anyway. Mandating the workaround may be the easiest to implement, but it certainly isn't the right thing to do. Of course, your suggestion might be implemented (*only for local scopes inside loops, please*) as a temporary fix for type system unsoundness, but don't assume that this will fix this issue. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #23 from Artem Borisovskiy --- (In reply to Walter Bright from comment #21) > The most practical solution is to simply disallow referencing variables from > a dynamic closure that end sooner than the closing } of the function. > > This leaves it up to the user to decide how to handle it, such as passing > the variable explicitly as an argument, declaring the variable at function > scope, or copying the variable to another declared at function scope. Well, if you think it's the most practical solution, then would you kindly provide us with a couple of examples showing the solution from different angles? It isn't clear how it solves the problem. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #22 from Ketmar Dark --- i.e. forbid closures at all. i wonder how should i pass additional parameter to std.alporithm.map predicate though. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 Walter Bright changed: What|Removed |Added Hardware|x86 |All OS|Windows |All --- Comment #21 from Walter Bright --- The most practical solution is to simply disallow referencing variables from a dynamic closure that end sooner than the closing } of the function. This leaves it up to the user to decide how to handle it, such as passing the variable explicitly as an argument, declaring the variable at function scope, or copying the variable to another declared at function scope. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #20 from Gabor Mezo --- I agree. This is a bug definitely and shuld be addressed as soon as possible. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043
--- Comment #19 from Artem Borisovskiy ---
> If the loop index is mutable, then I think
> it's OK to make it shared across loop iterations.
Ridiculous.
--
void delegate()[] dgList;
const count = 10;
// 1
for (int i = 0; i < count; ++i)
dgList ~= { writeln(i); }
// 2
foreach (immutable i; iota(count))
dgList ~= { writeln(i); }
--
Why should 1 and 2 behave differently? Lambdas should capture outer variables
in a consistent way - by copying them to their activation record, just as all
other languages do. If you really want to capture the counter itself, you
should use a pointer, although it doesn't make sense either, because you'll get
an address of a stack-allocated variable.
--
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #18 from hst...@quickfur.ath.cx --- If the loop index is mutable, then I think it's OK to make it shared across loop iterations. Creating a new instance of a loop variable per iteration is only necessary if (1) the loop index is immutable, and (2) it is being closed over. (I also think modifying the loop counter of a foreach loop is a very bad idea -- you should be using a good ole for(...) loop or while loop instead -- but that's beside the point.) --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043
Ketmar Dark changed:
What|Removed |Added
CC||ket...@ketmar.no-ip.org
--- Comment #17 from Ketmar Dark ---
(In reply to entheh from comment #16)
i believe that modifying `i` should be allowed here. but without 'ref' `i` is a
copy of internal counter, so your sample correctly iterates 10 times, updating
`i` to correct value on each iteration. but this works (and it's correct too):
foreach (ref i; 0..10) {
++i; // skip one
writeln(i);
}
this outputs 1, 3, 5, 7, 9.
> If 'i' became a new variable for each iteration, then the modify-assign
> would not have the effect intended.
it works this way now. ;-)
--
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043
--- Comment #16 from ent...@cantab.net ---
Unfortunately I have a feeling there will be people relying on being able to do
this kind of thing (though I'm not 100% sure if it currently works):
foreach (i; 0..10) {
...
if (somethingStrangeHappens) {
//Try again with the same 'i'
i--;
continue;
}
}
If 'i' became a new variable for each iteration, then the modify-assign would
not have the effect intended. Do we need to make 'i' implicitly final to
protect against this?
--
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #15 from hst...@quickfur.ath.cx --- P.S., printing out the addresses of local variables outside the loop as well as the loop counter, reveals that if we comment out the line that initializes the delegates closing over the loop counter, then the loop counter is allocated on the stack, but with the closures, the compiler actually allocates the loop counter on the heap. However, it fails to recognize that multiple iterations of the loop will reuse the same heap variable. So it looks like the compiler is already halfway there; it's already detecting the closure and putting the loop counter on the heap instead of the stack. Now the only thing that remains, is for it to detect that it needs a new instance of the loop counter per iteration, as opposed to just a single instance. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 --- Comment #14 from hst...@quickfur.ath.cx --- Note that the same thing applies if you make the loop counter immutable, which the compiler also readily accepts. But the delegates will all capture the same address which is reused over and over. So, basically, the logical scope of the loop counter i should be restricted to a *single* iteration (otherwise we must reject immutable loop counters!), but currently, its actual scope is the loop body *across* all iterations. The delegates exacerbate this problem by extending this scope even further, outside the loop body. Ideally, the compiler should allocate each new loop counter value on the heap, unless it can prove that there are no escaping references to it at the end of the iteration, in which case it is then safe to put it on the stack (and reuse the same memory for subsequent iterations). --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043
hst...@quickfur.ath.cx changed:
What|Removed |Added
CC||hst...@quickfur.ath.cx
--- Comment #13 from hst...@quickfur.ath.cx ---
Note that this bug breaks immutability:
-
import std.stdio;
void main() {
void delegate()[] a;
foreach (i; 1 .. 10) {
immutable j = i;
writeln(j);
a ~= { writeln(j); };
}
writeln("---");
foreach (f; a) {
f();
}
}
-
Output:
-
1
2
3
4
5
6
7
8
9
---
9
9
9
9
9
9
9
9
9
-
Notice that in each iteration of the loop, we are closing on the immutable
value j, which is set to the loop counter. However, the output shows that the
value of j has changed since the delegates were initialized. In fact, it
changes at every iteration. This violates the guarantee of immutable.
--
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.
https://issues.dlang.org/show_bug.cgi?id=2043 ent...@cantab.net changed: What|Removed |Added CC||ent...@cantab.net Summary|Closure outer variables in |Closure outer variables in |nested blocks are not |nested blocks are not |allocated/instantiated |allocated/instantiated |correctly. |correctly: should have ||multiple instances but only ||have one. --- Comment #12 from ent...@cantab.net --- Expanded summaries a little on this bug and on 8621 to prevent further confusion - they are not duplicates. --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly.
https://issues.dlang.org/show_bug.cgi?id=2043 Kenji Hara changed: What|Removed |Added CC||and...@erdani.com --- Comment #11 from Kenji Hara --- *** Issue 8621 has been marked as a duplicate of this issue. *** --
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly.
https://issues.dlang.org/show_bug.cgi?id=2043
Gabor Mezo changed:
What|Removed |Added
CC||gabor.m...@outlook.com
--- Comment #10 from Gabor Mezo ---
Actually, I find this style of workaround mutch more readable than we seen
before:
foreach (idx; 1 .. data.length)
{
(idx)
{
// Do stuff
}(idx);
}
It not looks too ugly if you have some JS background. :)
Please fix this bug ASAP. I really like this language but this POS always makes
me sad if I take a look at my code.
--
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly.
https://issues.dlang.org/show_bug.cgi?id=2043
Artem Borisovskiy changed:
What|Removed |Added
CC||kolo...@bk.ru
Severity|normal |major
--- Comment #9 from Artem Borisovskiy ---
I write a simple app that reads a binary log file, decrypts the messages in
parrallel and prints them when all the decrypting threads are finished. Here's
a piece of my code:
--
auto workers = new ThreadGroup;
int i;
foreach (group; logEntries.chunks(averageChunkSize)) () {
auto data = group;
auto ii = i;
auto workerThread = new Thread(
{
writefln("[%s] %s", ii, data.length);
});
workers.add(workerThread);
workerThread.start;
++i;
}();
workers.joinAll;
--
As you can see, I tried to use hack from the code above, but it didn't help
until I created "data" and "ii" variables. Just using "i" and "group" gives
wrong results, just as described by Bruno. The same code works perfectly in
Java, and it should in D, because it's the way closures work.
Please, fix this bug, it's real PITA and it's 6 years old after all.
--
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly.
http://d.puremagic.com/issues/show_bug.cgi?id=2043
Russ Lewis changed:
What|Removed |Added
CC||webmaster@villagersonline.c
||om
--- Comment #8 from Russ Lewis 2013-01-05
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
() {
}
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: ---
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly.
http://d.puremagic.com/issues/show_bug.cgi?id=2043
timon.g...@gmx.ch changed:
What|Removed |Added
CC||timon.g...@gmx.ch
--- Comment #7 from timon.g...@gmx.ch 2012-06-19 09:27:41 PDT ---
(In reply to comment #6)
> ...
> 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: ---
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly.
http://d.puremagic.com/issues/show_bug.cgi?id=2043
Witold Baryluk changed:
What|Removed |Added
CC||bary...@smp.if.uj.edu.pl
--- Comment #6 from Witold Baryluk 2011-01-01
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: ---
[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly.
http://d.puremagic.com/issues/show_bug.cgi?id=2043 Bruno Medeiros changed: What|Removed |Added CC||nfx...@gmail.com --- Comment #5 from Bruno Medeiros 2010-11-19 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: ---
