Test case:
struct Set {
void insert(ulong v) {
aa[v] = true;
}
@disable this(this);
bool[ulong] aa;
}
auto clobber(ref Set x, ref Set o) {
Set ret; // <- This overwrites x
ret.aa = x.aa; // <- Now x.aa is empty
return ret;
}
struct XX {
Set a, b, tmp;
this(int n) {
a.insert(1);
//a.aa[1] = true; <- Swap above line with this doesn't trigger the bug
tmp = a.clobber(b);
a = a.clobber(b);
}
}
void main(){
import std.stdio;
XX xx = XX(0);
writeln(xx.a.aa.length); // 0
writeln(xx.tmp.aa.length); // 1
}
Comment #1 by ag0aep6g — 2016-04-04T17:37:48Z
Slightly reduced:
----
struct Set {
@disable this(this);
int value = 0;
}
Set clobber(Set* a) {
Set ret; // <- This overwrites *a, i.e. &ret is the same as a
ret.value = a.value; // <- Now a.value is 0
return ret;
}
struct XX {
Set a = Set(1);
this(int n) {
a = clobber(&a);
}
}
void main(){
XX xx = XX(0);
assert(xx.a.value == 1); /* fails */
}
----
Comment #2 by yshuiv7 — 2016-04-05T01:03:48Z
I think the expected behavior here is a compile error.
Comment #3 by ag0aep6g — 2016-04-05T20:29:42Z
(In reply to Yuxuan Shui from comment #2)
> I think the expected behavior here is a compile error.
I think it should compile and set xx.a.value to 1. The compiler manages to do that when `a = clobber(&a);` is done outside of the constructor, and I don't see a reason why it being in a constructor should make a difference.
Comment #4 by yshuiv7 — 2016-04-05T21:05:20Z
Looks like if clobber is not called in constructor, the return value is stored into a temporary variable and then copied into xx using Set.opAssign.
I'm not sure if this is correct since Set has disabled this(this).
Comment #5 by ag0aep6g — 2016-04-05T21:12:30Z
(In reply to Yuxuan Shui from comment #4)
> Looks like if clobber is not called in constructor, the return value is
> stored into a temporary variable and then copied into xx using Set.opAssign.
>
> I'm not sure if this is correct since Set has disabled this(this).
`@disable this(this)` does not prevent moving the struct. A move is a bitwise copy that invalidates the source, i.e. there are no destructor or postblit calls.
Comment #6 by default_357-line — 2018-03-14T09:26:19Z
I think what's happening here is that DMD thinks that because a is assigned in the XX constructor, the member a is to be initialized by the constructor and not the default initializer. As a result, &a is technically use-before-initialization and invalid.
If true, then even though it's invalid code this still needs to be added to the docs, because it is somewhat unintuitive.
Comment #7 by yshuiv7 — 2018-03-14T09:29:53Z
> As a result, &a is technically use-before-initialization and invalid.
If that's the case, this shouldn't compile:
struct Set {
@disable this(this);
int value = 0;
}
@safe Set clobber(Set* a) {}
struct XX {
Set a = Set(1);
@safe this(int n) {
a = clobber(&a); // use-before-init
}
}
But it does.
Comment #8 by default_357-line — 2018-03-14T15:20:30Z
Agreed.
Comment #9 by bugzilla — 2018-04-22T02:04:05Z
The most pragmatic solution is to not allow taking a reference to an unconstructed field.
ahem. i was under impression that "constructors" in D are actually "post-initializers". i.e. that object must be fully initialized with default values before calling ctor, and can't have "unconstructed" anything (unless it has `=void` as default value, of course). that is, in the given case, compiler should detect that `a` is actually used, and fully initialize it before calling ctor of `XX`. or generate error on *any* "use-before-init" access, including things like "v++" and such.
that is, it looks to me that "pragmatic solution" is not really solving anything in this case: it just hacks around one very specific case, and in the same time makes D behavior even more unintuitive.
Comment #12 by bugzilla — 2018-04-22T05:47:58Z
(In reply to Walter Bright from comment #9)
> The most pragmatic solution is to not allow taking a reference to an
> unconstructed field.
That proved unworkable.
One that does work is to regard taking the address of a field as "initializing" it. Then,
a = clobber(&a);
is regarded as an assignment, rather than a construction, and the RVO is set to a temporary, not `a`.
Nothing else I could think of was palatable.
Comment #13 by github-bugzilla — 2018-05-14T09:31:49Z