Make _d_arrayctor and _d_arraysetctor aware of copy constructor

[edi33416]

This PR makes the copy construction of arrays aware of the copy constructor.
IMHO, in the long run, this should be used in #3139 and other similar fixes.

[dlang-bot]

Thanks for your pull request and interest in making D better, @edi33416! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + druntime#3177"

[kinke]

Not related to this PR, but memcpying each element separately is not optimal, and doesn't exploit the postblit advantage over copy ctors. The loop can additionally be completely elided if the type has no postblit.

[kinke]

Btw, is this stuff actually used at all? The compiler still lowers to the non-templated extern(C) hook...

Edit: Seems totally unused in 2.093, incl. no usages in druntime/Phobos.

[edi33416]

You are right. This doesn't get lowered to by the compiler.
As you've pointed out, the compiler lowers to the array function from rt.arrayassign.

The course of action going forward is to hook the call to _d_arrayctor in dmd, as @RazvanN7 pointed out.

Before doing so, we'll have to convert _d_arrayassign, _d_arrayassign_l and _d_arrayassign_r to into templates and then change dmd to call the templated versions of _d_arrayassign and the current templated version of _d_arrayctor and their set cousins _d_arraysetctor and _d_arraysetassign

[edi33416]

Not related to this PR, but memcpying each element separately is not optimal, and doesn't exploit the postblit advantage over copy ctors.

I believe the memcpying is done separately so it will be able to correctly destroy previously constructed elements in case a of a throw from the postblit of the current ith element

The loop can additionally be completely elided if the type has no postblit.

I think the memcopying should still be performed.

[kinke]

When performing a single memcpy, the 'unconstructed' elements >= i will have to be reset (edit: blitted) to T.init in the exception case, yes.

[edi33416]

So you're suggesting that the code should be changed to

try
    {
        static if (__traits(hasPostblit, T))
        {
            memcpy(to, from, element_size * to.length);
            for (i = 0; i < to.length; i++)
            {
                auto elem = cast(Unqual!T*)&to[i];
                postblitRecurse(*elem);
            }
        }
        else static if (__traits(hasCopyConstructor, T))
        {
            for (i = 0; i < to.length; i++)
            {
                to[i].__ctor(from[i]);
            }
        }
        else
        {
            memcpy(to, from, element_size * to.length);
        }
    catch (Exception o)
    {
        /* Destroy, in reverse order, what we've constructed so far
        */
        static if (__traits(hasPostblit, T) || !__traits(hasCopyConstructor, T))
        {
            for (int j = i; j < to.length; j++)
            {
                auto elem = cast(Unqual!T*)&to[i];
                memcpy(elem, &T.init, element_size);
            }
        }
        while (i--)
        {
            auto elem = cast(Unqual!T*)&to[i];
            destroy(*elem);
        }
        throw o;
    }

[kinke]

Something similar to this, yes. Elements without postblit/cpctor don't need a try/catch, that's a plain memcpy. T.init is an rvalue and cannot be used for memcpy; there's some emplace version blitting the default initializer which can be used - in case the T.init reset is really necessary (the array wasn't successfully constructed, so is it going to be destructed at some point later? have the elements already been initialized with T.init in the first place?).

[edi33416]

I'd keep this as a separate PR in an attempt to keep the changes small

[RazvanN7]

@edi33416 Fix the conflicts and let's merge this!

[edi33416]

Looks like this has been implemented in #3239