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

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

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

--

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

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> ---
This should have never worked to begin with!
Slightly modifying your example to expose the bug:

```
import core.stdc.stdio;

void foo()
{
    scope baseKey = Key.getKey();
}

int main(string[] argv)
{
    printf("main init\n");
    foo();
    printf("main exit\n");
    assert(Key.global !is null);
    return 0;
}

class Key
{
private:
    this() { printf("ctor\n"); }
    ~this() { printf("dtor\n"); }

    static Key global;
    static Key getKey() { return (global = new Key()); }
}
```

As you can see, in here the compiler has no way to know it *should* destroy `baseKey` at the end of the `foo` function. And `scope` in this case is misleading, as the `scope` reference is GC-allocated anyway.

--

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

--- Comment #2 from Rainer Schuetze <r.sagitario@gmx.de> ---
scope class references are not supposed to be safe so far. The current spec explicitly says the destructor is called: "This means that the destructor for an object is automatically called when the reference to it goes out of scope. The destructor is called even if the scope is exited via a thrown exception, thus scope is used to guarantee cleanup."

That might not be consistent with scope as it is used by DIP1000, but changing its meaning should at least have some notification in the changelog. IMO -dip1000 should prohibit unsafe usage, though.

See also discussion in https://github.com/ldc-developers/ldc/pull/2252#issuecomment-333360596

--

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

Martin Nowak <code@dawg.eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |code@dawg.eu
         Resolution|---                         |WONTFIX

--- Comment #3 from Martin Nowak <code@dawg.eu> ---
It's was destroyed twice formerly, destruction was never supposed to run for GC
allocated classes.
Scope class always referred to RAII allocated classes on the stack.
The fact that you can assign heap allocated instances to variables with scope
storage was unfortunately a bug, as was the double destruction of scope class
instances assigned to other scope references.

Scope variables not pointing to stack allocated RAII classes have been repurposed to match the semantics of scope pointers.

Sorry for any unforseen troubles this may have caused, but the existing behavior seemed too buggy to preserve.

--

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

--- Comment #4 from Rainer Schuetze <r.sagitario@gmx.de> ---
I'm not convinced:

> It's was destroyed twice formerly, destruction was never supposed to run for GC allocated classes.

Sure it has been unsafe so far, just like writing "scope(exit) delete obj;"
twice.

> Scope class always referred to RAII allocated classes on the stack.

Yes, but the spec does not say this explicitly about scope declarations.

> The fact that you can assign heap allocated instances to variables with scope storage was unfortunately a bug, as was the double destruction of scope class instances assigned to other scope references.

So why is it now silently accepted?

BTW: Even std.datetime.timezone still uses scope with factory functions, relying on GC caused bugs in LDC due to premature collection.

I'm not against changing the meaning of scope for the sake of DIP1000, but I wonder why it's ok in this case to break code without any warning that worked fine before.

--