Bug 22680 – @safe hole with destructors

C c; class C { ~this() @safe { c = this; } } This is accepted, but is unsafe. I propose that destructors always be marked as 'scope'.

There is nothing unsafe in assigning a class reference to another. I agree that

(In reply to RazvanN from comment #1) > There is nothing unsafe in assigning a class reference to another. Unless you're assigning garbage, which is happening here. A more elaborate demonstration of the unsafety: ---- import std.stdio: writeln; import core.memory: GC; C c; class C { immutable int* ip; this(int x) @safe { this.ip = new int(x); } ~this() @safe { c = this; } } void main() @safe { () { new C(42); } (); () { ubyte[1000] clear_stack; } (); () @trusted { GC.collect(); } (); immutable int* ip = c.ip; writeln(*ip); /* Prints "42". */ new int(13); int should_still_be_42 = *ip; writeln(should_still_be_42); /* Prints "13" - immutable data has changed. */ } ----

(In reply to ag0aep6g from comment #2) > (In reply to RazvanN from comment #1) > > There is nothing unsafe in assigning a class reference to another. > > Unless you're assigning garbage, which is happening here. A more elaborate > demonstration of the unsafety: > > ---- > import std.stdio: writeln; > import core.memory: GC; > C c; > class C > { > immutable int* ip; > this(int x) @safe { this.ip = new int(x); } > ~this() @safe { c = this; } > } > void main() @safe > { > () { new C(42); } (); > () { ubyte[1000] clear_stack; } (); > () @trusted { GC.collect(); } (); > immutable int* ip = c.ip; > writeln(*ip); /* Prints "42". */ > new int(13); > int should_still_be_42 = *ip; > writeln(should_still_be_42); /* Prints "13" - immutable data has > changed. */ > } > ---- My comment was unfinished. What I wanted to propose is to mark destructors that are @safe with scope. I don't think there would be any un-wanted side effects. I added some labels and I pushed "Save Changes", but I forgot that I had a comment started.

(In reply to RazvanN from comment #3) > My comment was unfinished. What I wanted to propose is to mark destructors > that are @safe with scope. I don't think there would be any un-wanted side > effects. > There would be unwanted side effects. On a struct, for example, marking a destructor scope would prevent you from returning instances of such struct. I think that a more prudent measure is to mark escaping destructors @system (i.e. making original code a compile-time error).

(In reply to Stanislav Blinov from comment #4) > There would be unwanted side effects. On a struct, for example, marking a > destructor scope would prevent you from returning instances of such struct. No, it only adds restrictions to the destructor's function body, not the destructor's caller.

(In reply to Dennis from comment #5) > (In reply to Stanislav Blinov from comment #4) > > There would be unwanted side effects. On a struct, for example, marking a > > destructor scope would prevent you from returning instances of such struct. > > No, it only adds restrictions to the destructor's function body, not the > destructor's caller. Compile this with -dip1000: @safe: struct S { ~this() scope {} void* p; } S test() { S s; return s; // Error: scope variable `s` may not be returned } void main() { }

(In reply to Stanislav Blinov from comment #6) > Compile this with -dip1000: That looks like a bug / remnant from the old dip1000 design.

(In reply to Dennis from comment #7) > That looks like a bug / remnant from the old dip1000 design. Introduced by https://github.com/dlang/dmd/pull/7284

(In reply to Dennis from comment #7) > (In reply to Stanislav Blinov from comment #6) > > Compile this with -dip1000: > > That looks like a bug / remnant from the old dip1000 design. Oh, OK, that's good to know, thanks!

(In reply to Stanislav Blinov from comment #6) > S test() > { > S s; > return s; // Error: scope variable `s` may not be returned > } This has been fixed.

And so the way should be clear now for marking a destructor as `scope`.

@WalterBright created dlang/dmd pull request #14402 "fix Issue 22680 - @safe hole with destructors" fixing this issue: - fix Issue 22680 - @safe hole with destructors https://github.com/dlang/dmd/pull/14402

dlang/dmd pull request #14402 "fix Issue 22680 - @safe hole with destructors" was merged into master: - 4780baecbbccb3e1d5c8fb85ef6e34de4030c71c by Walter Bright: fix Issue 22680 - @safe hole with destructors https://github.com/dlang/dmd/pull/14402