[Issue 2043] Closure outer variables in nested blocks are not allocated/instantiated correctly: should have multiple instances but only have one.

d-bugmail at puremagic.com d-bugmail at puremagic.com
Fri Apr 6 00:39:36 UTC 2018


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

--- Comment #37 from Walter Bright <bugzilla at digitalmars.com> ---
(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.

--


More information about the Digitalmars-d-bugs mailing list