Rebindable2 does not correctly handle types with copy constructors or postblit constructors which aren't assignable

[dlangBugzillaToGithub]

issues.dlang (@jmdavis) reported this on 2024-10-23T12:26:30Z

Transfered from https://issues.dlang.org/show_bug.cgi?id=24829

Description

In working on a fix for https://issues.dlang.org/show_bug.cgi?id=24827, I've determined that the useQualifierCast == false version of Rebindable2 does not do anything to handle postblit constructors or copy constructors, meaning that it will do the wrong thing when given a struct with a postblit constructor or copy constructor. E.G. this is the test that I have at the moment for it, and the copy isn't done (which isn't surprising when looking at the code, since instead of putting the struct directly in the Rebindable2, it's put in an array of void or ubyte depending on whether it has indirections):

---
unittest
{
    {
        static struct S
        {
            int* ptr;
            static bool copied;

            this(int i)
            {
                ptr = new int(i);
            }

            this(this) @safe
            {
                if(ptr !is null)
                    ptr = new int(*ptr);
                copied = true;
            }

            @disable ref S opAssign()(auto ref S rhs);
        }

        {
            auto foo = rebindable2(S(42));
            assert(!typeof(foo).useQualifierCast);
            S.copied = false;

            auto bar = foo;
            assert(S.copied);
            assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr);
            assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr);
        }
        {
            auto foo = rebindable2(const S(42));
            S.copied = false;

            auto bar = foo;
            assert(S.copied);
            assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr);
            assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr);
        }
    }

    // copy constructor without type qualifier cast
    {
        static struct S
        {
            int* ptr;
            static bool copied;

            this(int i)
            {
                ptr = new int(i);
            }

            this(ref inout S rhs) @safe inout
            {
                if(rhs.ptr !is null)
                    ptr = new inout int(*rhs.ptr);
                copied = true;
            }

            @disable ref S opAssign()(auto ref S rhs);
        }

        {
            auto foo = rebindable2(S(42));
            assert(!typeof(foo).useQualifierCast);
            S.copied = false;

            auto bar = foo;
            assert(S.copied);
            assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr);
            assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr);
        }
        {
            auto foo = rebindable2(const S(42));
            S.copied = false;

            auto bar = foo;
            assert(S.copied);
            assert(*(cast(S*)&(foo.data)).ptr == *(cast(S*)&(bar.data)).ptr);
            assert((cast(S*)&(foo.data)).ptr !is (cast(S*)&(bar.data)).ptr);
        }
    }
}
---

Note that that test would have to be inside of std.typecons alongside Rebindable2 (or at least somewhere within Phobos), since Rebindable2 is package(std).

It's not entirely clear to me what circumstances exactly reasonably result in useQualifierCast being false, since it requires that

    isAssignable!(typeof(cast() T.init))

be false. The test I put together has a disabled opAssign to make that happen, but I find it very bizarre that we'd even be attempting to make the code work with a disabled opAssign. So, I don't know if that's what it's trying to solve or whether it's something else that somehow manages to not work with qualifier-free assignment.

The affected code would appear to be std.range.repeat, std.algorithm.searching.minElement, and std.algorithm.maxElement, since those are the public symbols which ultimately use Rebindable2. So, if any of them are given a type which fails to be assignable in the fashion that Rebindable2 is testing for, _and_ that type has either a postblit constructor or a copy constructor, then that code is going to be wrong.

Fixing this does not look like it will be at all straightforward. Ideally, we'd make it so that the compiler could generate the postblit constructor or copy constructor like it would do when useQualiferCast is true, but I don't know how possible that is. And if we have to generate the postblit or copy constructor ourselves to get the correct behavior, that gets incredibly messy if we want to infer the attributes correctly, since we would not only need to use @trusted to deal with some casting, but we'd probably need to use a string mixin to generate the appropriate constructor with all of the correct attributes tagged on.