On Tuesday, 7 February 2023 at 23:42:38 UTC, 0xEAB wrote:
> I’ve recently run into an issue with DIP1000.
On Tuesday, 7 February 2023 at 23:45:05 UTC, 0xEAB wrote:
> Not sure how to put this…
Is attribute inference “supposed” to create such issues?
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:
> And two potential fixes:
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
.