On Saturday, 22 December 2018 at 23:26:47 UTC, Dru wrote:
> First I want to say that I hope it gets implemented
> Semantic consistency demands it:
>
> B b;
> A a = b; //calls this(B) or this(ref B)
> A a2 = a; //*should* call this(ref A)
>

That is indeed the case.

>
> I have a few points:
>
> 1) "ref" vs "ref const" dilemma
>
> The user can overload different kinds of copy ctors,
> but what is the *default* copy ctor ?
> I think the default copy ctor should be "ref const"
> - "ref const" is safer and fits most cases
> - "ref" has priority over "ref const" so the user can simply overload
> and his "ref" ctor will be used instead of the default
>
There is no default copy constructor. You define the copy constructors you need.
There is a generated copy constructor which is defined if a struct does not have a copy constructor, but has fields that define one. Currently, the generation strategy is pretty dumb, but I plan on enhancing it with a later DIP, in which copy constructors are generated based on the qualifiers of the fields copy constructors:

struct A
{
    this(ref A rhs) immutable {}
    this(ref A rhs) {}
}

struct B
{
    this(ref B rhs) immutable {}
}

struct C
{
    A a;
    B b;
}

For C, one can simply generate the following copy constructors:

this(ref C rhs) immutable
{
    this.a = rhs.a;
    this.b = rhs.b;
}

this(ref C rhs)
{
    this.a = rhs.a;     // copy constructor call
    this.b = rhs.b;     // error, no copy ctor
}

The first one succeeds in type checking, while the second one does not. I think that this is a superior design to the one in the DIP, but Walter thinks that we should keep things simple in the first iteration and add enhancements later.
>
> 2) "Generating copy constructors"
>
> Should the user care when a constructor is "generated" if semantics are consistent ?
> May just say there is *always* a default copy ctor that can be overriden by the user.
> The implementation of default ctor can change and use memcpy as needed.
>
The copy constructor is generated, only if there are copy constructors for some fields that need to be called. The idea is that if a struct defines a copy constructor, it cannot be blitted in any circumstances; the copy constructor should always be used.
>
> 3) Not very related but maybe also needs a fix:
> I noticed that init to rvalue of a different type is weird right now
> A a1 = b; //calls this(ref B)
> A a2 = B(); //does not call this(ref B)

It cannot call it, because the constructor receives a reference and you are passing an rvalue. That is not even a copy constructor, but a regular constructor, so if you want the compiler to call it, simply delete the ref.

On Saturday, 5 January 2019 at 13:34:09 UTC, RazvanN wrote:
> On Friday, 21 December 2018 at 14:43:50 UTC, Boris-Barboris wrote:
> As it was stated above, the transitive nature of const and immutable
> makes it impossible to safely copy objects with indirections. If you do
> not want to modify the source, then don't modify it from the copy constructor.
> This can be seen as a feature, not a bug.

1). I personally do not want such feature, I find it dangerous.
2). That would make sense, if that was ever an ambiguous choice. I cannot think of any case where you need to modify the source outside of move operation, and that has nothing to do with copy constructors. You are giving a choice in the situation where only one, known answer is always right.
We even know what's the problem is - our wonderful transitive const, that proves itself inadequate for typical smart pointer copying. Maybe instead of hacking in another poorly-integrated feature, we should fix that first? There were plenty of topics dedicated to head const semantics, why not address them now? It will solve lots of other range-related problems as well.
3). As someone who would like to stick to C++, I think you would say that you would feel right at home with the following semantics:

this(ref const typeof(this) rhs) - copy constructor
this(ref typeof(this) rhs) - move constructor

I'm not a fan of the proposed opMove.


> Agree, that was my initial design too, but in C++ you can define any number of
> default parameters and people coming from that background might have some cases
> where it is useful for them to have default parameters. Since this is the first
> iteration of a copy constructor implementation, we should try to stick as much
> as possible to the C++ design. Of course, in the case of const source we cannot
> do that since const means something else in D.

The key difference between copy\move constructors and other constructors is that they are implicitly inserted by the compiler in certain scenarios. Documentation that describes such constructors should not be littered with alternative exotic semantics that have nothing to do with implicitly inserted function calls.

this(ref typeof(this) rhs, int b) is just a constructor and should not be mentioned anywhere in the copy constructor documentation, nor in the DIP.

On Saturday, 5 January 2019 at 14:03:34 UTC, Boris-Barboris wrote:
> 1). I personally do not want such feature, I find it dangerous.
I think that the discussion should transcend feelings and stick to what can be done.
> 2). That would make sense, if that was ever an ambiguous choice. I cannot think of any case where you need to modify the source outside of move operation, and that has nothing to do with copy constructors. You are giving a choice in the situation where only one, known answer is always right.
> We even know what's the problem is - our wonderful transitive const, that proves itself inadequate for typical smart pointer copying. Maybe instead of hacking in another poorly-integrated feature, we should fix that first? There were plenty of topics dedicated to head const semantics, why not address them now? It will solve lots of other range-related problems as well.
I think that discussing whether const should be transitive or not is beyond the scope of this DIP.

> 3). As someone who would like to stick to C++, I think you would say that you would feel right at home with the following semantics:
>
> this(ref const typeof(this) rhs) - copy constructor
> this(ref typeof(this) rhs) - move constructor
>
> I'm not a fan of the proposed opMove.
>
But the opMove DIP proposes a different syntax: https://github.com/dlang/DIPs/pull/109/files#diff-50c61cb5afd3ffe27ddb9c5279fd9fc4R205

>
>> Agree, that was my initial design too, but in C++ you can define any number of
>> default parameters and people coming from that background might have some cases
>> where it is useful for them to have default parameters. Since this is the first
>> iteration of a copy constructor implementation, we should try to stick as much
>> as possible to the C++ design. Of course, in the case of const source we cannot
>> do that since const means something else in D.
>
> The key difference between copy\move constructors and other constructors is that they are implicitly inserted by the compiler in certain scenarios. Documentation that describes such constructors should not be littered with alternative exotic semantics that have nothing to do with implicitly inserted function calls.
>
> this(ref typeof(this) rhs, int b) is just a constructor and should not be mentioned anywhere in the copy constructor documentation, nor in the DIP.

Since the copy constructor can also be used as a normal constructor (calling it explicitly), then there might be situations where you would need the extra parameters.

On Saturday, 5 January 2019 at 14:16:41 UTC, RazvanN wrote:
> I think that the discussion should transcend feelings and stick to what can be done.

That feeling is coming from reason I highlighted in my original post.
"Can be done" is not enough, "should it be done?" is just as important.

> I think that discussing whether const should be transitive or not is beyond the scope of this DIP.

I think no DIP should ever be viewed in isolation. The is no DIP scope. There is one, unified scope of language evolution. All concurrent DIPs and ideas must be accounted for.

> But the opMove DIP proposes a different syntax: https://github.com/dlang/DIPs/pull/109/files#diff-50c61cb5afd3ffe27ddb9c5279fd9fc4R205

Yes, I meant I'm not a fan of that different syntax. Postblit and opPostMove should be merged into one move constructor call if it's defined IMO.

> Since the copy constructor can also be used as a normal constructor (calling it explicitly), then there might be situations where you would need the extra parameters.

Why would you mentioned it anywhere though? It's already implemented, we have such constructors. What does it have to do with copy constructor, besides the first argument?

On Saturday, 5 January 2019 at 14:39:06 UTC, Boris-Barboris wrote:

> Why would you mentioned it anywhere though? It's already implemented, we have such constructors. What does it have to do with copy constructor, besides the first argument?

Prior to this DIP, one might have defined:

this(ref S a, int b=0) {}

This function is called only explicitly, but after this DIP, it will get called whenever a S instance will be copied. If you are against this function being a copy constructor, I can understand that, but C++ is considering it a copy constructor and for this first iteration, we are too.

> The copy constructor is generated, only if there are copy constructors for some fields that need to be called. The idea is that if a struct defines a copy constructor, it cannot be blitted in any circumstances; the copy constructor should always be used.

I offer a simpler logic:
A "default" copy constructor is always generated,
except when the user defines a constructor of the same signature.
Effectively the user can override the default.



> It cannot call it, because the constructor receives a reference and you are passing an rvalue. That is not even a copy constructor, but a regular constructor, so if you want the compiler to call it, simply delete the ref.

yes not related to the DIP
just wondering why block the option to pass rvalue by ref

On Saturday, 5 January 2019 at 14:44:12 UTC, RazvanN wrote:
> This function is called only explicitly, but after this DIP, it will get called whenever a S instance will be copied. If you are against this function being a copy constructor, I can understand that, but C++ is considering it a copy constructor and for this first iteration, we are too.

Oh... I didn't know C++ went that way, I now understand your reasoning, sorry.
You're right, I'm against it, but if that's an expected behavior, it's probably for the best.

How do they deal with ambiguity (multiple copy constructors with different argument tuples), just halt with a compiler error?

On Saturday, 5 January 2019 at 14:54:41 UTC, Dru wrote:
>> The copy constructor is generated, only if there are copy constructors for some fields that need to be called. The idea is that if a struct defines a copy constructor, it cannot be blitted in any circumstances; the copy constructor should always be used.
>
> I offer a simpler logic:
> A "default" copy constructor is always generated,
> except when the user defines a constructor of the same signature.
> Effectively the user can override the default.
>
Why would you need a default copy constructor? If there aren't any copy constructors defined, the compiler will just blit the fields. Generating
a single default copy constructor is not enough. Consider:

struct A
{
    int a;
}

fun(immutable A a) {}

void main()
{
    immutable A a;
    fun(a);
}

If we define a single copy constructor like you suggested, then this will not work because you have immutable to immutable copy. In this situation it is best to leave the copying to the existing mechanism.

>
>
>> It cannot call it, because the constructor receives a reference and you are passing an rvalue. That is not even a copy constructor, but a regular constructor, so if you want the compiler to call it, simply delete the ref.
>
> yes not related to the DIP
> just wondering why block the option to pass rvalue by ref

Currently, the only solution to this is to bind the rvalue to an lvalue: (B __tmp = B(); tmp), but this is problematic because assignment has side effects while in the original case a move is performed. It is not an unsolvable problem, but it would lead to unexpected behavior in some cases.

On Saturday, 5 January 2019 at 15:00:32 UTC, Boris-Barboris wrote:
> On Saturday, 5 January 2019 at 14:44:12 UTC, RazvanN wrote:
>> This function is called only explicitly, but after this DIP, it will get called whenever a S instance will be copied. If you are against this function being a copy constructor, I can understand that, but C++ is considering it a copy constructor and for this first iteration, we are too.
>
> Oh... I didn't know C++ went that way, I now understand your reasoning, sorry.
> You're right, I'm against it, but if that's an expected behavior, it's probably for the best.
>
> How do they deal with ambiguity (multiple copy constructors with different argument tuples), just halt with a compiler error?

Yes.

On Saturday, 5 January 2019 at 13:34:09 UTC, RazvanN wrote:
> On Friday, 21 December 2018 at 14:43:50 UTC, Boris-Barboris wrote:
>> 2). "A declaration is a copy constructor declaration if it is a constructor declaration that takes only one non-default parameter by reference that is of the same type as typeof(this), followed by any number of default parameters..."
>>
>> If you need other parameters, you are not performing a copy. Copy constructor needs no additional parameters. If the semantics of your domain problem involve parametrized post-copy operations, the code should state that explicitly - by using specialized properly-named methods, that notify the reader about this particularity.
>>
>>
> Agree, that was my initial design too, but in C++ you can define any number of
> default parameters and people coming from that background might have some cases
> where it is useful for them to have default parameters. Since this is the first
> iteration of a copy constructor implementation, we should try to stick as much
> as possible to the C++ design. Of course, in the case of const source we cannot
> do that since const means something else in D.

This seems like a weak justification to me. There are many things in C++ that are permitted by the language but considered poor practice by the C++ community, and continue to exist only for the sake of backward compatibility. D should not blindly adopt behavior from C++ without considering why that behavior exists in C++, and whether D could do better.

I have not been able to find many sources on whether copy constructors with optional parameters are regarded as useful or good practice in the C++ community, but the results I have found are not encouraging. In the C++ Core Guidelines, the section on copy constructors [1] does not even acknowledge the possibility of additional optional parameters. In addition, at least one static analysis tool for C++ considers copy constructors with optional parameters an error [2], because they may be called implicitly in places the programmer did not intend them to be. None of this is conclusive evidence, of course, but at the very least, it indicates that this is an issue on which the C++ community is divided.

If there are specific, known benefits to allowing copy constructors with optional parameters in D, then the DIP should articulate those benefits. "C++ does it that way" is not good enough. Otherwise, it may be better to go ahead with only single-parameter copy constructors, and leave the question of copy constructors with optional parameters for a future DIP.

[1] https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#S-ctor
[2] https://help.semmle.com/wiki/display/CCPPOBJ/Constructor+with+default+arguments+will+be+used+as+a+copy+constructor