Thread overview
[Issue 23987] Copy construction should not disable implicit conversion
Jun 12, 2023
timon.gehr@gmx.ch
Jun 19, 2023
RazvanN
Jun 19, 2023
RazvanN
Jun 19, 2023
timon.gehr@gmx.ch
June 12, 2023
https://issues.dlang.org/show_bug.cgi?id=23987

--- Comment #1 from timon.gehr@gmx.ch ---
A workaround is to mark the copy constructor `pure`.

--
June 19, 2023
https://issues.dlang.org/show_bug.cgi?id=23987

RazvanN <razvan.nitu1305@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |razvan.nitu1305@gmail.com

--- Comment #2 from RazvanN <razvan.nitu1305@gmail.com> ---
I don't think this bug report is valid. Since a copy constructor is defined for S, then all copies that are being made for it need to call a copy constructor. Since the copy constructor is only defined for mutable destinations, I don't see how we could safely support the automatic conversion from mutable to immutable. That would open the door for code like this one:

int* s;

struct S
{
    int p;

    this(const ref S src)
    {
        s = &p;
    }
}

void main()
{
    immutable(S) s1 = S(2);
    immutable(S) s2 = s1;
    writeln(*s2.p);
    *s = 9;
    writeln(*s2.p);
}

--
June 19, 2023
https://issues.dlang.org/show_bug.cgi?id=23987

--- Comment #3 from RazvanN <razvan.nitu1305@gmail.com> ---
(In reply to RazvanN from comment #2)
> 
> void main()
> {
>     immutable(S) s1 = S(2);
>     immutable(S) s2 = s1;
>     writeln(*s2.p);
>     *s = 9;
>     writeln(*s2.p);
> }

`p` is not a pointer so no need for the `*`. Sorry for the bogus test case. Correct code:

```
int* s;

struct S
{
    int p;

    this(const ref S src)
    {
        s = &p;
    }
}

void main()
{
    immutable(S) s1 = S(2);
    immutable(S) s2 = s1;
    writeln(s2.p);
    *s = 9;
    writeln(s2.p);
}
```

--
June 19, 2023
https://issues.dlang.org/show_bug.cgi?id=23987

timon.gehr@gmx.ch changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |WONTFIX

--- Comment #4 from timon.gehr@gmx.ch ---
(This is an enhancement request, not a bug report.)

Taking the address of `p` in your example is not `@safe` (and subsequently the pointer in your main function refers to a dead memory location, as the value after implicit conversion is separate from the one before implicit conversion).

This enhancement request grew out of the existing behavior being confusing.

In particular:

```d
import std.typecons;

struct T {
    // no copy constructor
}

struct S {
    this(const ref S _){
        // dummy copy constructor
    }
}

void main(){
    Nullable!T t; // ok
    Nullable!S s; // error
}
```

The error happens here:
```d
this(ref return scope inout Nullable!T rhs) inout
{
    _isNull = rhs._isNull;
    if (!_isNull)
        _value.payload = rhs._value.payload;
    else
        _value = DontCallDestructorT.init;
}
```

I had hoped we can avoid the requirement for people to provide an `inout` copy constructor if they are generating a mutable `struct` without indirections from a `const` instance.

However, I agree that generating an implicit second copy constructor call to a generated copy constructor after the user has already provided their own copy constructor is probably even worse because copies/moves may get missed.

Maybe requiring the copy constructor to be annotated `pure` is good enough, but the overall situation is a bit unsatisfactory.

Closing this for now.

--