Jump to page: 1 2
Thread overview
[Issue 15662] Cannot move struct with defined opAssign due to @disabled post-blit
Feb 09, 2016
Kenji Hara
Feb 10, 2016
Dicebot
Feb 18, 2016
Dicebot
Feb 18, 2016
Kenji Hara
Feb 18, 2016
Dicebot
Feb 18, 2016
Kenji Hara
Feb 18, 2016
Kenji Hara
Feb 18, 2016
Dicebot
Mar 20, 2016
Martin Nowak
Apr 29, 2016
Martin Nowak
May 13, 2016
Martin Nowak
Dec 17, 2022
Iain Buclaw
February 09, 2016
https://issues.dlang.org/show_bug.cgi?id=15662

--- Comment #1 from Kenji Hara <k.hara.pg@gmail.com> ---
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.

--
February 09, 2016
https://issues.dlang.org/show_bug.cgi?id=15662

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com

--- Comment #2 from Steven Schveighoffer <schveiguy@yahoo.com> ---
we should create a moveAppend function that allows this (would destroy old elements if they have to be moved).

--
February 10, 2016
https://issues.dlang.org/show_bug.cgi?id=15662

Dicebot <public@dicebot.lv> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |public@dicebot.lv

--- Comment #3 from Dicebot <public@dicebot.lv> ---
(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).

--
February 18, 2016
https://issues.dlang.org/show_bug.cgi?id=15662

--- Comment #4 from Dicebot <public@dicebot.lv> ---
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.

--
February 18, 2016
https://issues.dlang.org/show_bug.cgi?id=15662

--- Comment #5 from Kenji Hara <k.hara.pg@gmail.com> ---
(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.

--
February 18, 2016
https://issues.dlang.org/show_bug.cgi?id=15662

--- Comment #6 from Dicebot <public@dicebot.lv> ---
(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.

--
February 18, 2016
https://issues.dlang.org/show_bug.cgi?id=15662

Kenji Hara <k.hara.pg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://issues.dlang.org/sh
                   |                            |ow_bug.cgi?id=15699

--- Comment #7 from Kenji Hara <k.hara.pg@gmail.com> ---
(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

--
February 18, 2016
https://issues.dlang.org/show_bug.cgi?id=15662

--- Comment #8 from Kenji Hara <k.hara.pg@gmail.com> ---
(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);
}

--
February 18, 2016
https://issues.dlang.org/show_bug.cgi?id=15662

--- Comment #9 from Dicebot <public@dicebot.lv> ---
(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.

--
March 20, 2016
https://issues.dlang.org/show_bug.cgi?id=15662

Martin Nowak <code@dawg.eu> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |code@dawg.eu

--- Comment #10 from Martin Nowak <code@dawg.eu> ---
(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.

--
« First   ‹ Prev
1 2