Bug 17449 – [DIP1000] crash due to covariant conversion of scope delegate to delegate

From the dmd testsuite, runnable/xtest46.d: module test; struct Event9834 { void delegate() dg; void set(void delegate() h) pure { dg = h; } // AV occurs void call() { dg(); } } void main() { Event9834 ev; auto a = new class { Object o; this() { o = new Object; ev.set((){ o.toString(); }); } }; ev.call(); } This runs fine when compiled with "dmd test", but crashes with an access violation when compiled with "dmd test -dip1000". This is caused by the delegate being inferred as "scope", and implicitely converted to a non-scope delegate (thus not having a closure). This is ok when the delegate is called by the callee (Event9834.set) directly, but not if it is saved for later invocation.

Reduced test: void main() { void delegate() dg; void savedg(void delegate() h) { dg = h; } void foo(int x) { savedg({ assert(x == 42); }); } foo(42); dg(); } Assertion failure with -dip1000, fine without. It is also ok if dg is made global, so maybe escape analysis fails for local functions only.

Adding `@safe` to main() causes the compiler error message: test.d(8): Error: `@safe` function `D main` cannot call `@system` delegate `dg` which looks appropriate. Adding more @safe: @safe void main() { void delegate() @safe dg; void savedg(void delegate() @safe h) { dg = h; } void foo(int x) { savedg({ assert(x == 42); }); } foo(42); dg(); } Makes it compile and run successfully. Checking the generated code, it is generating a closure. So it looks correct. -dip1000 isn't enough, using @safe is also necessary.

Are you saying that it is ok that adding -dip1000 can change code gen so that it crashes? I don't see anything unsafe and invalid in the original code. Could you elaborate? Please note that it it also in the dmd test suite.

(In reply to Rainer Schuetze from comment #3) > Are you saying that it is ok that adding -dip1000 can change code gen so > that it crashes? Yes. This is because 'scope' for delegates now means something with dip1000, it means the delegate must not escape. If it does, and this is not checked for in non-@safe functions, it will likely crash. > I don't see anything unsafe and invalid in the original code. Could you > elaborate? Please note that it it also in the dmd test suite. The problem is the delegate escapes from its stack frame, and no closure was allocated. The scope checking with delegates is a backwards compatibility thing, as people often pass scope delegates to non-scope delegate parameters.

> This is because 'scope' for delegates now means something with dip1000, it means the delegate must not escape. There is no "scope" in the test cases. I suspect it is falsely inferred.

> Adding more @safe Makes it compile and run successfully. Your safe code also fails with -dip1000. (tried on Windows/OMF).

Fixing the code with @safe and @trusted: --- module test; @safe: struct Event9834 { @safe: void delegate() dg; void set(void delegate() @safe h) pure { dg = h; } // AV occurs void call() { dg(); } } void main() { Event9834 ev; auto a = new class { Object o; this() @safe { o = new Object; ev.set(() @trusted { o.toString(); }); } }; ev.call(); } --- and it compiles and runs without error, with or without -dip1000.

> Fixing the code with @safe and @trusted: As with the modified reduced test case with @safe, this is still crashing for me (Win32 and Win64 with git HEAD). Checking the disassembly for compiling with -dip1000 it doesn't generate the closure that it is generating without the switch.

(In reply to Rainer Schuetze from comment #8) > As with the modified reduced test case with @safe, this is still crashing > for me (Win32 and Win64 with git HEAD). Checking the disassembly for > compiling with -dip1000 it doesn't generate the closure that it is > generating without the switch. If it still crashes, did you mean to reopen this issue? (You changed it from WORKSFORME to FIXED.)

I didn't mean to change the status, but reopening is probably appropriate.

BTW: AFAIK -dip1000 is only supposed to affect @safe code, but there is no @safe in the original test cases.

This looks like a variation of https://issues.dlang.org/show_bug.cgi?id=17959, with -dip1000 inferring "scope".

> Checking the disassembly for compiling with -dip1000 it doesn't generate the closure that it is generating without the switch. When I check it, it does generate the closure. Perhaps this is due to other related PRs that exist on my machine but haven't been pulled yet.

Seems to have been fixed in the meantime by https://github.com/dlang/dmd/pull/7999, with or without @safe annotation.