Jump to page: 1 2 3
Thread overview
[Issue 16193] opApply() doesn't heap allocate closure
Jun 22, 2016
Mathias Lang
Jun 23, 2016
Ketmar Dark
Jun 23, 2016
Ketmar Dark
Jun 23, 2016
Marc Schütz
Jun 23, 2016
ZombineDev
Jul 27, 2016
Walter Bright
Sep 15, 2016
Walter Bright
Sep 15, 2016
Walter Bright
Sep 15, 2016
Walter Bright
Sep 15, 2016
Walter Bright
Dec 19, 2016
Dicebot
Dec 27, 2016
Dicebot
Jan 17, 2017
anonymous4
Jan 17, 2017
Dicebot
Dec 17, 2022
Iain Buclaw
Feb 03, 2023
Nick Treleaven
June 22, 2016
https://issues.dlang.org/show_bug.cgi?id=16193

Mathias Lang <mathias.lang@sociomantic.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mathias.lang@sociomantic.co
                   |                            |m

--- Comment #1 from Mathias Lang <mathias.lang@sociomantic.com> ---
Why do you want it to allocate ?

That can either be changed (with a BIG RED warning), or the specs can be
updated to mention the delegate is always `scope`.
Either way, I'm just puzzled what's the use case here.

--
June 23, 2016
https://issues.dlang.org/show_bug.cgi?id=16193

--- Comment #2 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
(In reply to Mathias Lang from comment #1)
> Why do you want it to allocate ?
for consistency.

> Either way, I'm just puzzled what's the use case here.
less special cases to remember.

--
June 23, 2016
https://issues.dlang.org/show_bug.cgi?id=16193

Ketmar Dark <ketmar@ketmar.no-ip.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ketmar@ketmar.no-ip.org

--
June 23, 2016
https://issues.dlang.org/show_bug.cgi?id=16193

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
                 CC|                            |schveiguy@yahoo.com
           Hardware|x86_64                      |All
                 OS|Linux                       |All
           Severity|normal                      |regression

--- Comment #3 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to Marc Schütz from comment #0)
> I'm deliberately using separate compilation here to
> make sure that the compiler has no way to infer that the closure doesn't
> escape.

Separate compilation doesn't imply anything. The compiler can see all the source for opApply, so it could potentially make the decision that it doesn't escape.

To prove the compiler is making this optimizing assumption, actually *do* escape the delegate, and see what happens. When I do this, it actually still does not create a closure:

// yy.d:
module yy;

struct S {
    int delegate(int) escaped;
    int opApply(int delegate(int) dg) {
        escaped = dg;
        foreach(i; 1 .. 10) {
            int result = dg(i);
            if(result) return result;
        }
        return 0;
    }
}

// xx.d:
import yy;
import std.stdio;

int* px, py;
S s;

int foo() {
    int x = 0, y = 0;
    foreach(i; s) {
        px = &x; py = &y;
        y++;
        writeln("y = ", y);
    }
    return y;
}

void main() {
    writeln(foo());
    int z = 0;
    int *b = new int;
    writeln("&x = ", px, " &y = ", py, " &z = ", &z, " b = ", b);
    s.escaped(0);
}

output (purposely not using separate compilation to prove this has nothing to
do with it):
# dmd xx.d yy.d
# ./xx
y = 1
y = 2
y = 3
y = 4
y = 5
y = 6
y = 7
y = 8
y = 9
9
&x = 7FFF5F29E8E8 &y = 7FFF5F29E8EC &z = 7FFF5F29E920 b = 100C70000
y = 32768

Note the difference in address between the stack variables and the obvious heap variable. And note the invalid value for y for the escaped call since foo has gone out of scope.

BTW, tested on a Mac.

This is just a straight-up wrong-code bug. I'm expecting this is also a regression, but I'm too lazy to find the version where this properly worked :) However, I do know that dmd at one point ALWAYS allocated a closure, so this must have worked at some point.

--
June 23, 2016
https://issues.dlang.org/show_bug.cgi?id=16193

--- Comment #4 from Marc Schütz <schuetzm@gmx.net> ---
(In reply to Steven Schveighoffer from comment #3)
> (In reply to Marc Schütz from comment #0)
> > I'm deliberately using separate compilation here to
> > make sure that the compiler has no way to infer that the closure doesn't
> > escape.
> 
> Separate compilation doesn't imply anything. The compiler can see all the source for opApply, so it could potentially make the decision that it doesn't escape.
> 

Because of separate compilation, the compiler IMO _must not_ rely on the implementation details of the called function, it may only use the information available in the function signature. That's my understanding, at least: attribute inference must only happen for template and auto functions. Inlining is probably an exception, but I didn't enable that in this example.

--
June 23, 2016
https://issues.dlang.org/show_bug.cgi?id=16193

--- Comment #5 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to Marc Schütz from comment #4)

> Because of separate compilation, the compiler IMO _must not_ rely on the implementation details of the called function, it may only use the information available in the function signature. That's my understanding, at least: attribute inference must only happen for template and auto functions. Inlining is probably an exception, but I didn't enable that in this example.

This is somewhat OT, but separate compilation doesn't imply that. If the implementation is available, it's available. If you change the implementation between compilations, you're in for a world of hurt.

Consider that templates can be used with separate compilation, how would that fare?

The reason D does not infer attributes for non-template non-auto functions is because you *could* provide a .di file that just contains prototypes, and then the inferred attributes are no longer correct. For a template or auto-return, the function would be invalid, so that is why we can infer there.

In this case, however, we are talking about an optimization, and something decided by the caller, not the callee. So whether to allocate a closure can easily be decided by looking at all the code that will use that particular delegate instance and seeing if it escapes. It need not modify or infer any attributes of functions to achieve this.

--
June 23, 2016
https://issues.dlang.org/show_bug.cgi?id=16193

ZombineDev <petar.p.kirov@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |petar.p.kirov@gmail.com

--
July 27, 2016
https://issues.dlang.org/show_bug.cgi?id=16193

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |safe
                 CC|                            |bugzilla@digitalmars.com

--
September 15, 2016
https://issues.dlang.org/show_bug.cgi?id=16193

--- Comment #6 from Walter Bright <bugzilla@digitalmars.com> ---
Not a fix for this bug, but Phobos should be fixed:

    https://github.com/dlang/phobos/pull/4787

--
September 15, 2016
https://issues.dlang.org/show_bug.cgi?id=16193

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull
           Severity|regression                  |normal

--- Comment #7 from Walter Bright <bugzilla@digitalmars.com> ---
https://github.com/dlang/dmd/pull/6135

Also, this was not a regression.

--
« First   ‹ Prev
1 2 3