Bug 22013 – Making RefCounted dtor @safe breaks DIP1000

Status
NEW
Severity
major
Priority
P2
Component
phobos
Product
D
Version
D2
Platform
All
OS
All
Creation time
2021-06-10T15:17:02Z
Last change time
2024-12-01T16:38:59Z
Keywords
accepts-invalid, safe
Assigned to
No Owner
Creator
Atila Neves
Moved to GitHub: phobos#10468 →

Comments

Comment #0 by atila.neves — 2021-06-10T15:17:02Z
If std.conv.RefCounted's dtor is made @safe, the code below compiles with -preview=dip1000 but shouldn't. It correctly fails to compile if `return scope` is used instead of `scope Container local` but the user shouldn't have to do that manually. Or, at least, they should, but the compiler shouldn't compile buggy code that escapes a reference to a local. See https://github.com/dlang/phobos/pull/8101#issuecomment-843017282. This issue is blocking that PR. ------------------------- import std.stdio; import std.typecons; struct Container { int[] data; } void main () { auto ptr = getPtr(); writeln(ptr); const save = ptr.dup; auto other = getPtr(); assert(save == ptr); } int[] getPtr () { int[42] local; return getPtr2(Container(local)); } int[] getPtr2 (scope Container local) { RefCounted!Container rc = local; return rc.refCountedPayload().data; } -------------------------
Comment #1 by dkorpel — 2021-06-10T15:27:35Z
Can you reduce it to a self-contained test case without Phobos imports?
Comment #2 by stanislav.blinov — 2021-12-06T17:09:18Z
Adding @safe: at the top stops compilation in its tracks as the ctor and dtor for RefCounted get inferred @system. With 2.098 and -dip1000 also an attempt to escape a local into RefCounted's ctor __is__ detected. Escaping a local static array in @safe doesn't seem possible, so it would seem this issue is no longer valid? However, as pertains to the goals of pull request mentioned above, even if the aforementioned problems in RefCounted's implementation are fixed, ref counted slices (even in disguise) can't possibly have their destructor @safe, as long as we can do this: void test() @safe { auto slice = makeSomeKindOfRefCountedSlice!int(10); int[] local = slice.payload.data; static assert(!__traits(compiles, &slice)); // scope static assert(!__traits(compiles, &local)); // scope slice = slice.init; // drop ref count to 0 and deallocate local[1] = 32; // use after free in @safe function } It almost looks like we need a way to define functions that return only temporaries (so that the line `int[] local = slice.payload.data` above doesn't compile). Until we can do that in some form, any @safe destructor of such ref-counted slice could only be safe by convention (i.e. what's the point then?).
Comment #3 by razvan.nitu1305 — 2022-08-31T09:58:15Z
Marking this as a phobos issue.
Comment #4 by robert.schadek — 2024-12-01T16:38:59Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/phobos/issues/10468 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB