Hi,

I’ve recently run into an issue with DIP1000.

After a lot of confusion by myself and suggestions from fellow coders on Discord (Thanks everyone!) I went on with implementing a potential workaround. Well… until my issue was suddenly gone although there was no actual workaround in place yet.

Yesterday, it came to my mind that there must be something off. And that our/my original conclusions might not be exactly sound.

So I went back to the reduced sample code provided by Mayonix (special thanks for throwing it into dustmite!) and compared it to the working version. I found something interesting and prepared this post. Then double checked my code samples… Just to notice the error is gone (again!). “What’s going on?”

The issue comes down to:

noscope360.d(5): Error: scope variable `c` assigned to non-scope parameter `_param_0` calling `write`
noscope360.d(13):        which is not `scope` because of `buffer = _param_0`

Here’s the code snippet:

void main() @safe {
    auto mb = MBuf();
    auto vr = VRes!X();
    foreach(c; vr.errors)
        mb.write(c);
}

struct X { }

struct MBuf {
    const(ubyte)[] _bufferList;
    void write(Buffers...)(Buffers buffers) {
        foreach (buffer; buffers) { }
    }
}

struct VErr { string m; }

struct VRes(Data) {
    VErr[Data.tupleof.length] _errors;
    auto errors() {
        import std.algorithm;
        return _errors[].filter!(e => e.m);
    }
}

And two potential fixes:

--- noscope360.d
+++ yoscope360.d
@@ -8,7 +8,6 @@
 struct X { }

 struct MBuf {
-    const(ubyte)[] _bufferList;
     void write(Buffers...)(Buffers buffers) {
         foreach (buffer; buffers) { }
     }

--- noscope360.d
+++ throwscope360.d
@@ -10,7 +10,7 @@
 struct MBuf {
     const(ubyte)[] _bufferList;
     void write(Buffers...)(Buffers buffers) {
-        foreach (buffer; buffers) { }
+        static foreach (buffer; buffers) { }
     }
 }

The changes from throwscope360 are obviously not doable in practice, as they remove code that would have been used eventually.

Does it make sense to have *static* foreach mandatory here?

I don’t feel like those changes should matter.
While I’m by no means sure that this is a bug, I would really hope for it to be one.

Steven told me this has to do with inference.

Not sure how to put this…
Is attribute inference “supposed” to create such issues?

On Tuesday, 7 February 2023 at 23:42:38 UTC, 0xEAB wrote:

On Tuesday, 7 February 2023 at 23:45:05 UTC, 0xEAB wrote:

There are no relevant attributes being incorrectly inferred in your sample code, as far as I can tell.

Rewriting your code to avoid attribute inference entirely - by manually instantiating templates and supplying explicit types in place of auto - does not fix the problem. (It may be possible to fix it with changes to std.algorithm.filter, though.)

On Tuesday, 7 February 2023 at 23:42:38 UTC, 0xEAB wrote:

Neither of those "fixes" would reliably fix your code in more realistic contexts; they only work in your reduced test case by accident.

The actual problem is a combination of three things:

(1) You allocate vr on the stack, meaning it has a finite lifetime.

(2) DIP1000 considers c to be restricted by vr's lifetime.

(3) You pass c as a non-scope parameter, which by DIP1000's logic means it might escape beyond the lifetime of vr's stack frame, and potentially cause a use-after-free error.

Fix at least one of those properly to eliminate the error while preserving memory safety. Valid solutions include:

(A) Fix (1) by allocating vr with "infinite" lifetime (like globals, string literals, and new GC allocations):

    auto vr = new VRes!X();

(B) Fix (3) by marking buffers as scope:

    void write(Buffers...)(scope Buffers buffers) {

(C) Fixing (2) is the hard one.

Even with DIP1000, D currently lacks a way to directly indicate what the relationship is between an aggregate instance's lifetime, and the lifetime of the target of one of its member indirections: an instance member cannot be scope, nor can it be the opposite of scope.

Instead, the compiler relies on a combination of conservative assumptions, each member function's attributes, their parameter's attributes, and analysis of their data flow.

In practice, it seems to be possible with @safe code to induce the compiler to treat a member indirection's target in one of three ways:

(1) As always restricted to the lifetime of the aggregate instance. (This is what I think scope should do, if it were allowed to apply to member variables.)

(2) As restricted to the lifetime of the aggregate instance only if the aggregate instance itself is scope. (This is the default.)

(3) Inconsistently, if some member functions are correctly written and annotated to provoke (1), and others are not. (This is perhaps the easiest to achieve, despite having no obvious valid uses.)

The missing option is:

(4) As possessing "infinite" lifetime, regardless of whether the aggregate instance is scope, with the compiler forbidding any operation in @safe code which might cause the indirection to be assigned a target with finite lifetime. (This is the opposite of scope, a concept which has no name in D at the moment.)

Since there is no @safe way to increase a target's perceived lifetime, restrictions are transitive and contagious. In order to achieve something resembling (4) we must somehow launder our indirection, to hide its origin from DIP1000's data flow analysis.

A very stupid, but @safe way:

struct VErr {
    // For simplicity, this assumes that each VErr instance will only ever be accessed from a single thread.
    private static string[size_t] _mTable;
    @property ref string m() scope @safe {
        return _mTable.require(cast(size_t) &this, null); }
    @property ref const(string) m() scope const @safe {
        return _mTable.require(cast(size_t) &this, null); }

    ~this() @safe {
        _mTable.remove(cast(size_t) &this);
    }
}

An efficient way, but it requires @trusted and might need more work to ensure it doesn't accidentally enable memory safety violations in some way that I missed:

struct VErr {
    private string _m;
    @property ref inout(string) m() scope inout pure @trusted {
        return _m; }

    this(scope ref inout(VErr) that) scope inout pure @safe {
        this._m = that.m; }
    ref typeof(this) opAssign(scope ref inout(VErr) that) return scope pure @safe {
        this._m = that.m;
        return this;
    }
}
version(unittest)
    static string VErr_escape;
@safe unittest {
    scope string z = "z";

    scope VErr a;
    a.m = "y";
    assert(a.m == "y");
    static assert(!__traits(compiles, a.m = z));
    VErr_escape = a.m;

    VErr b = a;
    b.m = "x";
    assert(b.m == "x");
    b.m = a.m;
    assert(b.m == "y");
    static assert(!__traits(compiles, b.m = z));
}

Again, you don't need to apply all three of those fixes - just one will do. Which one you should use depends on whether MBuf.write needs to escape, and what allocation strategies you want to use for vr and the target(s) of VErr.m.

On Wednesday, 8 February 2023 at 13:00:35 UTC, ag0aep6g wrote:

alias VErr = char*;

ref front_r(ref VErr r) { return r; }
ref front_p(VErr* p) { return *p; }
ref front_s(VErr[] s) { return s[0]; }

VErr g;

void main() @safe
{
    VErr[1] _errors;
    g = _errors[0]; /* copy from static array: ok */

That's inside the same function, so data flow analysis can see that no scope address has been assigned to any element of _errors.

    g = front_r(_errors[0]); /* copy from `ref` via `ref`: ok */

ref is head scope, and does not infect the char* itself.

    g = front_p(&_errors[0]); /* copy from `ref` via pointer:
                                 fails, should work */

Since &_errors[0] is a pointer to a stack variable, it must be scope. Pointers get transitive scope, so the char* is infected, too.

    g = front_s(_errors[]); /* copy from `ref` via slice:
                               fails, should work */

Again, the _errors[] slice contains a pointer to a stack variable, so it must be scope. Slices get transitive scope, like pointers.

}

I think most of the confusion surrounding scope and related attributes stems from the fact that scope is transitive, when it really shouldn't be.

Instead, we should be allowed to directly specify for each level of indirection, and for each aggregate field whether it is always scope, follows the scopeness of its parent (as now), or is never scope.

The current design is fundamentally confusing and incoherent, because the true root of most every chain of indirections is some offset from the stack pointer, which must be scope if its existence is explicitly acknowledged. Since scope is transitive, this implies that most everything should be scope, which is stupid.

On Wednesday, 8 February 2023 at 22:25:32 UTC, tsbockman wrote:

This really is a good illustration of the confusion surrounding scope, because scope is not transitive and never has been:

char* global;

void main() @safe
{
    char* local;
    scope char** p = &local;
    global = *p; // ok
}

On Wednesday, 8 February 2023 at 23:03:17 UTC, Paul Backus wrote:

char* global;

void main() @safe
{
    char* local;
    scope char** p = &local;
    global = *p; // ok
}

Well then, I guess I don't understand the rules either.

In the original sample the compiler insists that the value of c and c.m must be scope if &vr is, when it is only the next level of indirection up - the memory storing that value - that is actually on the stack within vr.

Aggregates sometimes behave in ways that at least resemble scope transitivity, even if it's not really the same thing:

struct S {
    int* i;
    int* j;
}

int* escape;
int x, y;

void main() @safe
{
    int z;

    S a;
    a.i = &x;
    a.j = &y;
    escape = a.i; // Allowed

    S b;
    b.i = &y;
    b.j = &z;
    escape = b.i; // Rejected because b.j needs scope, which apparently forces b and all its fields to be scope, too.
}