Bug 23164 – [REG 2.097] Infinite loop on assertion failure + DMD moves struct with copy constructor

The following code shows 2 assertions failures in v2.096.1, an infinite loop on v2.097.0: ``` struct std_string { std_string* ptr; ulong[3] data; this (ulong data) { this.data[] = data; this.ptr = &this; } this(const scope ref std_string from) { assert(&from is from.ptr); assert(this.ptr is null); this.ptr = &this; this.data = from.data; } ~this () { assert(this.ptr is &this); } alias opAssign = assign; ref std_string assign () (const auto ref std_string rhs) { return this.assign2(rhs); } ref std_string assign2 (const ref std_string rhs) return { assert(rhs.ptr is &rhs); assert(this.ptr is null || this.ptr is &this); this.data = rhs.data; return this; } } void main () { std_string tmp = 42; assert(tmp.ptr is &tmp); tmp = std_string(42); assert(tmp.ptr is &tmp); } ``` The original test was to see whether or not DMD would move a struct with a copy constructor. The above code compiles and runs fine with LDC & GDC, but asserts with DMD.

Remember that D does not allow internal pointers to members.

If I add @safe and compile with -preview=dip1000, I get: test23164.d(15): Error: address of variable `this` assigned to `this` with longer lifetime test23164.d(22): Error: address of variable `this` assigned to `this` with longer lifetime Line 15 is: this.ptr = &this; in the constructor Line 22 is: this.ptr = &this; in the copy constructor

I find the `auto ref` parameter suspicious. What are you expecting to happen there? Do you expect it to be passed by ref or by value? It doesn't compile with making it just `ref`, so simply removing `auto ref` produces the same result (infinite loop assert in the destructor). But, passing it by value means another constructor call, and another destructor call. I'm not sure what you're expecting to happen here. I suspect the problem is with the `auto ref`. Copy constructors should be passing their rvalue by ref, amirite?

> Remember that D does not allow internal pointers to members. If memory serves me well, Andrei mentioned a few years ago that this position is no longer tenable. Not supporting internal pointers means users cannot interface with `std::string`, which is a huge blow too C++ interop. And internal pointers are one of the reasons we got the copy constructors, aren't they ? > I find the `auto ref` parameter suspicious. What are you expecting to happen there? Do you expect it to be passed by ref or by value? I expect a single constructor call for a value constructed in the caller. In other word, I expect: ``` tmp = std_string(42); ``` and: ``` auto someLValue = std_string(42); tmp = someLValue; ``` to only call `std_string` constructor once. If I don't use `auto ref` (pass by value), the code will compile for both snippets above but in the case of `someLValue`, it will call the constructor one more time. If I use plain `ref`, the code will not compile for the first snipped (passing a rvalue). Note that the original test case was using `in`, which is just `ref` accepting rvalue, so I doubt `auto ref` is at fault here (especially considering LDC and GDC don't have the bug, only DMD does).

(In reply to Mathias LANG from comment #0) > The following code shows 2 assertions failures in v2.096.1, an infinite loop > on v2.097.0: > ``` --snip-- > ~this () > { > assert(this.ptr is &this); > } --snip-- > ``` > > The original test was to see whether or not DMD would move a struct with a > copy constructor. The above code compiles and runs fine with LDC & GDC, but > asserts with DMD. To explain GDC behaviour. *Because* there is a destructor (or postblit, copy constructor, or anything that would other make the struct non-trivially copyable) then the type is *always* *implicitly* passed and returned by invisible reference. The right reduction would be this: ``` struct std_string { std_string* ptr; ulong[3] data; this (ulong data) { this.data[] = data; this.ptr = &this; } ~this () { } // GDC and LDC forces 'auto ref' to be 'ref', even when the // front-end decides to drop the 'ref' from the signature. ref std_string opAssign()(const auto ref std_string rhs) { assert(rhs.ptr is &rhs); return this; } } void main () { std_string tmp; tmp = std_string(42); } ```

(In reply to Iain Buclaw from comment #5) > // GDC and LDC forces 'auto ref' to be 'ref', even when the > // front-end decides to drop the 'ref' from the signature. > ref std_string opAssign()(const auto ref std_string rhs) Do you mean it does this for all functions, or just for opAssign? That regardless of `ref` or `auto ref` or ``, it passes by `ref`?

(In reply to Walter Bright from comment #6) > (In reply to Iain Buclaw from comment #5) > > // GDC and LDC forces 'auto ref' to be 'ref', even when the > > // front-end decides to drop the 'ref' from the signature. > > ref std_string opAssign()(const auto ref std_string rhs) > > Do you mean it does this for all functions, or just for opAssign? That > regardless of `ref` or `auto ref` or ``, it passes by `ref`? Not functions, types. Either you let the optimizer/backend copy around, make temporaries, or put pieces of an object in registers if it so requires. Or you tag it with a flag that disallows all the above. In such cases the only meaningful semantic you can apply to such types is to force all objects to be fully addressable, so passing and returning in memory only. What kind of objects get this flag? Any type that has non-trivial copy or destruction semantics - i.e: `~this()`, `this(this)`, and `this(ref const this)`

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/18111 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB