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 ***