Hi,

Just tried to upgrade mir-algorithm with the copy constructor.

Mir has only a few types that should define copy constructors explicitly.

However, Slice (main structure ) and more than 30 internal generic types like Iterators and Fields should be upgraded now as well. The compiler adds an automatically generated constructor. So, default structure construction that worked before, would not work anymore:

struct S(T)
{
    T t;
}

T oneT;

S(oneT); // would not work anymore if T has a copy constructor.

So, it is a big breaking change for user code as well.

Can we fix it at least for types that have compiler generated copy-constructors?

Best,
Ilya

On Wednesday, 8 May 2019 at 03:18:14 UTC, 9il wrote:
> struct S(T)

struct S // just a non-generic type

On Wednesday, 8 May 2019 at 03:18:14 UTC, 9il wrote:
> Hi,
>
> Just tried to upgrade mir-algorithm with the copy constructor.
>
> Mir has only a few types that should define copy constructors explicitly.
>
> However, Slice (main structure ) and more than 30 internal generic types like Iterators and Fields should be upgraded now as well. The compiler adds an automatically generated constructor. So, default structure construction that worked before, would not work anymore:
>
> struct S(T)
> {
>     T t;
> }
>
> T oneT;
>
> S(oneT); // would not work anymore if T has a copy constructor.
>
> So, it is a big breaking change for user code as well.
>
> Can we fix it at least for types that have compiler generated copy-constructors?
>
> Best,
> Ilya

What is the signature of the copy constructor that you have defined? Have you tried the this(inout typeof(this)) inout one?

On Wednesday, 8 May 2019 at 09:05:48 UTC, RazvanN wrote:
> On Wednesday, 8 May 2019 at 03:18:14 UTC, 9il wrote:
>> Hi,
>>
>> Just tried to upgrade mir-algorithm with the copy constructor.
>>
>> Mir has only a few types that should define copy constructors explicitly.
>>
>> However, Slice (main structure ) and more than 30 internal generic types like Iterators and Fields should be upgraded now as well. The compiler adds an automatically generated constructor. So, default structure construction that worked before, would not work anymore:
>>
>> struct S(T)
>> {
>>     T t;
>> }
>>
>> T oneT;
>>
>> S(oneT); // would not work anymore if T has a copy constructor.
>>
>> So, it is a big breaking change for user code as well.
>>
>> Can we fix it at least for types that have compiler generated copy-constructors?
>>
>> Best,
>> Ilya
>
> What is the signature of the copy constructor that you have defined? Have you tried the this(inout typeof(this)) inout one?

The copy constructor is autogenerated for S.

Check: https://run.dlang.io/is/3sJTrf
-------
struct T
{
    int i;
    this(ref return scope inout typeof(this) src)
        inout @safe pure nothrow @nogc
    {
        i = src.i;
    }
}

struct S
{
    T t;
}

auto bar(T a)
{
	S(a);
}
-------

onlineapp.d(18): Error: copy constructor onlineapp.S.this(ref inout(S) p) inout is not callable using argument types (T)
onlineapp.d(18):        cannot pass argument a of type T to parameter ref inout(S) p

On Wednesday, 8 May 2019 at 11:19:02 UTC, 9il wrote:
> On Wednesday, 8 May 2019 at 09:05:48 UTC, RazvanN wrote:
>> On Wednesday, 8 May 2019 at 03:18:14 UTC, 9il wrote:
>>> [...]
>>
>> What is the signature of the copy constructor that you have defined? Have you tried the this(inout typeof(this)) inout one?
>
> The copy constructor is autogenerated for S.
>
> Check: https://run.dlang.io/is/3sJTrf
> -------
> struct T
> {
>     int i;
>     this(ref return scope inout typeof(this) src)
>         inout @safe pure nothrow @nogc
>     {
>         i = src.i;
>     }
> }
>
> struct S
> {
>     T t;
> }
>
> auto bar(T a)
> {
> 	S(a);
> }
> -------
>
> onlineapp.d(18): Error: copy constructor onlineapp.S.this(ref inout(S) p) inout is not callable using argument types (T)
> onlineapp.d(18):        cannot pass argument a of type T to parameter ref inout(S) p

It introduces regression for Phobos as well. For example,

`DontCallDestructorT(arg)` is no longer possible for T arg.

---
struct Nullable(T)
{
    private union DontCallDestructorT
    {
        T payload;
    }
 ....
}

On Wednesday, 8 May 2019 at 12:47:10 UTC, 9il wrote:
> On Wednesday, 8 May 2019 at 11:19:02 UTC, 9il wrote:
>> On Wednesday, 8 May 2019 at 09:05:48 UTC, RazvanN wrote:
>>> [...]
>>
>> The copy constructor is autogenerated for S.
>>
>> Check: https://run.dlang.io/is/3sJTrf
>> -------
>> struct T
>> {
>>     int i;
>>     this(ref return scope inout typeof(this) src)
>>         inout @safe pure nothrow @nogc
>>     {
>>         i = src.i;
>>     }
>> }
>>
>> struct S
>> {
>>     T t;
>> }
>>
>> auto bar(T a)
>> {
>> 	S(a);
>> }
>> -------
>>
>> onlineapp.d(18): Error: copy constructor onlineapp.S.this(ref inout(S) p) inout is not callable using argument types (T)
>> onlineapp.d(18):        cannot pass argument a of type T to parameter ref inout(S) p
>
> It introduces regression for Phobos as well. For example,
>
> `DontCallDestructorT(arg)` is no longer possible for T arg.
>
> ---
> struct Nullable(T)
> {
>     private union DontCallDestructorT
>     {
>         T payload;
>     }
>  ....
> }

The problems seems to be that the default constructor takes the arguments by value so a copy needs to be performed, hence the copy constructor use. This is pretty nasty. I will return with the details after I sort this out.

On Wednesday, 8 May 2019 at 13:19:24 UTC, RazvanN wrote:
>
> The problems seems to be that the default constructor takes the arguments by value so a copy needs to be performed, hence the copy constructor use. This is pretty nasty. I will return with the details after I sort this out.

Isn't the issue that the presence of a constructor disables the use of "struct literal" syntax [1] for construction? The compiler is trying to find a constructor that matches the given argument, but since the only constructor is the auto-generated copy constructor, it can't.

If you add an explicit constructor to S, the problem goes away:

struct HasCopyCtor
{
    this(ref HasCopyCtor other) {}
}

struct S
{
    HasCopyCtor member;
    // Comment out the line below and compilation fails
    this(HasCopyCtor arg) { member = arg; }
}

void main()
{
    S s = S(HasCopyCtor());
}

Runnable: https://run.dlang.io/is/F4tHky

[1] https://dlang.org/spec/struct.html#struct-literal

On Wednesday, 8 May 2019 at 18:01:34 UTC, Paul Backus wrote:
> On Wednesday, 8 May 2019 at 13:19:24 UTC, RazvanN wrote:
>>
>> The problems seems to be that the default constructor takes the arguments by value so a copy needs to be performed, hence the copy constructor use. This is pretty nasty. I will return with the details after I sort this out.
>
> Isn't the issue that the presence of a constructor disables the use of "struct literal" syntax [1] for construction? The compiler is trying to find a constructor that matches the given argument, but since the only constructor is the auto-generated copy constructor, it can't.
>

Yes, exactly. --Ilya

On Thursday, 9 May 2019 at 04:58:14 UTC, 9il wrote:
> On Wednesday, 8 May 2019 at 18:01:34 UTC, Paul Backus wrote:
>> On Wednesday, 8 May 2019 at 13:19:24 UTC, RazvanN wrote:
>>>
>>> The problems seems to be that the default constructor takes the arguments by value so a copy needs to be performed, hence the copy constructor use. This is pretty nasty. I will return with the details after I sort this out.
>>
>> Isn't the issue that the presence of a constructor disables the use of "struct literal" syntax [1] for construction? The compiler is trying to find a constructor that matches the given argument, but since the only constructor is the auto-generated copy constructor, it can't.
>>
>
> Yes, exactly. --Ilya

PR: https://github.com/dlang/dmd/pull/9790

On Thursday, 9 May 2019 at 04:58:14 UTC, 9il wrote:
> On Wednesday, 8 May 2019 at 18:01:34 UTC, Paul Backus wrote:
>> On Wednesday, 8 May 2019 at 13:19:24 UTC, RazvanN wrote:
>>>
>>> The problems seems to be that the default constructor takes the arguments by value so a copy needs to be performed, hence the copy constructor use. This is pretty nasty. I will return with the details after I sort this out.
>>
>> Isn't the issue that the presence of a constructor disables the use of "struct literal" syntax [1] for construction? The compiler is trying to find a constructor that matches the given argument, but since the only constructor is the auto-generated copy constructor, it can't.
>>
>
> Yes, exactly. --Ilya

Thanks for the report. Please notify me of any other bugs regarding the copy constructor that you find. After you finish the transition of mir from postblit
to copy constructor maybe we can get Mike to add a blog post with your experience
so that others migrate to the copy constructor as well,

Thanks,
RazvanN