Bug 15662 – Cannot move struct with defined opAssign due to @disabled post-blit

Given the following definition: [code] import std.algorithm; struct ResourceHandle(T, alias Deleter, T Default = T.init) { // Constructors/Destructor this(T handle) {m_handle = handle;} @disable this(this); ~this() {Deleter(m_handle);} // Operators @disable void opAssign(ref ResourceHandle lvalue); ref ResourceHandle opAssign(ResourceHandle rvalue) {swap(m_handle, rvalue.m_handle); return this;} // Methods @property inout(T) handle() inout {return m_handle;} @property T handle(T handle) {Deleter(m_handle); m_handle = handle; return m_handle;} T release() {T result = m_handle; m_handle = Default; return result;} private: T m_handle = Default; } [/code] The following will generate a compile error, making it awkward to move unique resources: [code] unittest { alias RH = ResourceHandle!(uint, (uint) {}); RH[] handles; handles ~= RH(5); // Compile error: ResourceHandle is not copyable because it is annotated with @disable } [/code] See discussion at https://forum.dlang.org/post/[email protected] At first I was uncertain whether this was a bug or by design, but Andrei Alexandrescu confirmed the bug status in the linked discussion.

This is expected behavior. Appending element to array would cause reallocation if there's no array.capacity. It's not related whether the appended element is unique. D's built-in array is a view of original memory. The viewed memory is not owned by the array slice, so we cannot destroy the original elements freely. Therefore, when the reallocation happens, the original elements will be copied to a newly allocated memory (its size is enough larger than the original array.length), then the old elements are copied into it _by using postblit_. Finally the unique element will be moved into the allocated room. During compilation, compiler cannot know whether the appending won't cause reallocation, so it's conservatively rejected because of @disable-d postblit.

we should create a moveAppend function that allows this (would destroy old elements if they have to be moved).

(In reply to Steven Schveighoffer from comment #2) > we should create a moveAppend function that allows this (would destroy old > elements if they have to be moved). This seems quite fitting for a `Buffer` concept which we are switching to from built-in arrays to keep D1 array stomping behavior (effectively a wrapper over array which owns that array and calls `assumeSafeAppend` each time length is changed).

Btw note that this compiles: struct Buffer { @disable this(this); void[] data; } void main ( ) { Buffer[] many; many.length += 1; many[$-1] = Buffer(new void[42]); } .. though it should be semantically identical to appending. Is this a bug or feature? I really need a reliable way to force compiler to do such appending to implement concept pointed out by Steven.

(In reply to Dicebot from comment #4) > Btw note that this compiles: > [snip] > .. though it should be semantically identical to appending. Is this a bug or > feature? Wow, yes, it's definitely a bug. I'll open a new issue. > I really need a reliable way to force compiler to do such appending > to implement concept pointed out by Steven. Rather it would be a library function.

(In reply to Kenji Hara from comment #5) > (In reply to Dicebot from comment #4) > > Btw note that this compiles: > > > [snip] > > .. though it should be semantically identical to appending. Is this a bug or > > feature? > > Wow, yes, it's definitely a bug. I'll open a new issue. Was afraid of that :) > > I really need a reliable way to force compiler to do such appending > > to implement concept pointed out by Steven. > > Rather it would be a library function. No objections here. Question is - how to implement it inside a library (without modifying compiler) if this array.length loophole is closed? Only thing that comes to my mind is to reinterpret cast it to array of `ubyte[T.sizeof]` and do manual blit copy but that feels very error-prone.

(In reply to Kenji Hara from comment #5) > (In reply to Dicebot from comment #4) > > .. though it should be semantically identical to appending. Is this a bug or > > feature? > > Wow, yes, it's definitely a bug. I'll open a new issue. Done: https://issues.dlang.org/show_bug.cgi?id=15699

(In reply to Dicebot from comment #6) > No objections here. Question is - how to implement it inside a library > (without modifying compiler) if this array.length loophole is closed? Only > thing that comes to my mind is to reinterpret cast it to array of > `ubyte[T.sizeof]` and do manual blit copy but that feels very error-prone. It would need some @trusted code and runtime check, because currently there's not enough compile-time information to guarantee its safety. My quick implementation: void moveAppend(T)(ref T[] arr, T elem) { if (!arr.capacity) throw new Error("cannot append"); swap(*(arr.ptr + arr.length), elem); arr = arr.ptr[0 .. arr.length + 1]; // *uninitialize* elem, as same as std.algorithm.move memcpy(&elem, typeid(T).initializer().ptr, sz); }

(In reply to Kenji Hara from comment #8) > void moveAppend(T)(ref T[] arr, T elem) > { > if (!arr.capacity) throw new Error("cannot append"); > > swap(*(arr.ptr + arr.length), elem); > arr = arr.ptr[0 .. arr.length + 1]; > > // *uninitialize* elem, as same as std.algorithm.move > memcpy(&elem, typeid(T).initializer().ptr, sz); > } Yes, this is close to what I had in mind (main difference is that I need re-allocation to be allowed too, invalidating previous block, but that is also trivially doable). The problem I see with such approach is usual type erasure druntime often suffers from. Requiring same qualifiers for array elements and appended one is overly limited but relaxing it means the functions needs to reimplement all the implicit conversion checks compiler already does.

(In reply to Dicebot from comment #9) > Yes, this is close to what I had in mind (main difference is that I need > re-allocation to be allowed too, invalidating previous block, but that is > also trivially doable). You can't really do what you want w/o being the sole owner of the underlying array. If that's guaranteed, i.e. noone else has a slice or any other reference, you can just realloc and bit blit. So it'd be very unsafe for blank arrays (b/c they allow escaping), but a custom array wrapper/implementation would work.

Here is how you define a properly typed insertBack method. struct Buffer(T) // T can be const/immutable { // value type, requires insertBack(move(val)) for non-copyable types // compiler will perform any implicit conversions void insertBack(T value) { reserve(1); memcpy(ptr + idx, &value, T.sizeof); // clear value, so it's destructor won't double free anything static if (hasElaborateDestructor!T) { static if (!hasElaborateAssign!T && isAssignable!T) chunk = T.init; else { import core.stdc.string : memcpy; static immutable T init = T.init; memcpy(&value, &init, T.sizeof); } } } }

(In reply to Martin Nowak from comment #11) > static if (!hasElaborateAssign!T && isAssignable!T) > chunk = T.init; That needs to be `value = T.init;`. Direct assignment is an optional optimization over using memcpy. > else > { > import core.stdc.string : memcpy; > static immutable T init = T.init; > memcpy(&value, &init, T.sizeof); > }

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