Bug 18792 – Incorrect scope analysis with -dip1000 for small-sized-optimized string

Status
RESOLVED
Resolution
DUPLICATE
Severity
normal
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2018-04-23T09:46:43Z
Last change time
2021-06-10T19:22:40Z
Keywords
safe
Assigned to
No Owner
Creator
Per Nordlöw

Comments

Comment #0 by per.nordlow — 2018-04-23T09:46:43Z
The following code should fail to compile in functions shouldFail1 and shouldFail2 when compiled with flag -dip1000: /** Small-size-optimized (SSO) variant of `string`. * * Store on the stack if constructed with <= `smallCapacity` number of * characters, otherwise on the GC heap. * * See_Also: https://forum.dlang.org/post/[email protected] */ struct SSOString { private alias E = immutable(char); // immutable element type pure nothrow @nogc: scope E[] opSlice() const return @trusted // TODO @safe for -dip1000? { if (isLarge) { union RawLarge { Raw raw; Large large; } RawLarge copy = void; copy.large = cast(Large)large; copy.raw.length /= 2; // adjust length return copy.large; } else { return small.data[0 .. small.length/2]; // scoped. TODO use .ptr when proved stable } } /// ditto scope E[] opSlice(size_t i, size_t j) const return @trusted // TODO @safe for -dip1000? { return opSlice()[i .. j]; } private: /** Returns: `true` iff this is a large string, otherwise `false.` */ @property bool isLarge() const @trusted { return large.length & 1; // first bit discriminates small from large } struct Raw // same memory layout as `E[]` { size_t length; // can be bit-fiddled without GC allocation E* ptr; } alias Large = E[]; enum smallCapacity = Large.sizeof - Small.length.sizeof; static assert(smallCapacity > 0, "No room for small source for E being " ~ E.stringof); version(LittleEndian) // see: http://forum.dlang.org/posting/zifyahfohbwavwkwbgmw { struct Small { ubyte length; // TODO only first 4 bits are needed to represent a length between 0-15, use other 4 bits E[smallCapacity] data; } } else { static assert(0, "TODO Add BigEndian support and test"); } union { Raw raw; Large large; Small small; } } /// @safe pure nothrow @nogc unittest { string shouldFail1() @safe pure nothrow @nogc { SSOString x; return x[]; // TODO should fail with -dip1000 } string shouldFail2() @safe pure nothrow @nogc { SSOString x; return x[0 .. 0]; // TODO should fail with -dip1000 } }
Comment #1 by bugzilla — 2020-03-04T10:07:42Z
Please reduce to a much smaller case of what the problem is.
Comment #2 by per.nordlow — 2020-03-04T11:06:48Z
I managed to reduce the code snippet to struct SSOString { pure nothrow @nogc: inout(char)[] opSlice() inout return scope @trusted // TODO @safe for -dip1000? { return small.data[0 .. small.length]; // scoped. TODO use .ptr when proved stable } struct Small { ubyte length; // TODO only first 4 bits are needed to represent a length between 0-15, use other 4 bits char[15] data; } struct Raw // same memory layout as `char[]` { size_t length; // can be bit-fiddled without GC allocation char* ptr; } union { Raw raw; // PROBLEM this declaration prevents DIP-1000 scope analysis from kicking in in `opSlice` Small small; } } @safe pure nothrow @nogc unittest { char[] shouldFail1() @safe pure nothrow @nogc { SSOString x; return x[]; // TODO should fail with -dip1000 } } When compiled with -dip100 the function `shouldFail1` should fail to compile but it does still compile. When I remove the line Raw raw; scope analysis suddenly starts working which correctly triggers the error foo.d(33,17): Error: returning `x.opSlice()` escapes a reference to local variable `x` return x[]; // TODO should fail with -dip1000 Small enough, Walter?
Comment #3 by per.nordlow — 2020-03-04T11:08:43Z
(In reply to Per Nordlöw from comment #2) > Small enough, Walter? Forgot to remove obselete comments. Here's a new version with irrelevant comments removed. struct SSOString { pure nothrow @nogc: inout(char)[] opSlice() inout return scope @trusted { return small.data[0 .. small.length]; } struct Small { ubyte length; char[15] data; } struct Raw { size_t length; char* ptr; } union { Raw raw; // PROBLEM this declaration prevents DIP-1000 scope analysis from kicking in in `opSlice` Small small; } } @safe pure nothrow @nogc unittest { char[] shouldFail1() @safe pure nothrow @nogc { SSOString x; return x[]; // TODO should fail with -dip1000 } }
Comment #4 by bugzilla — 2020-03-04T21:07:07Z
much better, thank you
Comment #5 by dkorpel — 2021-06-10T19:22:40Z
I know this issue was first, but 21868 already got a pull. It is the same problem: `return scope` without `ref return` applies return the values (e.g. SSOString.raw.ptr), not the address of SSOString itself (e.g. SSOString.small.data.ptr), but the compiler doesn't check for return-scope or return-ref, just return. The declaration of `Raw raw` is needed because otherwise SSOString has no pointers, so return scope is stripped away, so the compiler can't mistake the return attribute anymore and raises the correct error. *** This issue has been marked as a duplicate of issue 21868 ***