On Saturday, 22 December 2018 at 02:02:55 UTC, Nicholas Wilson wrote:
> On Saturday, 22 December 2018 at 00:08:51 UTC, Boris-Barboris wrote:
>
> One of the primary use cases for this is ref-counting stuff, the refcount needs to be mutable.

I thought ref-counted stuff is perfectly fine with postblits, was I wrong?

On Saturday, 22 December 2018 at 09:01:09 UTC, Boris-Barboris wrote:
> On Saturday, 22 December 2018 at 03:37:22 UTC, Rubn wrote:
>> On Saturday, 22 December 2018 at 00:08:51 UTC, Boris-Barboris wrote:
>>
>> The DIP goes over why const wasn't used for the source. Consider the following:
>>
>> struct A {
>>     int* ptr;
>> }
>>
>> Now to simulate the copy constructor:
>>
>> A copy(ref const A a) {
>>     A result;
>>     result.ptr = a.ptr; // error can't convert `const int*` to `int*`
>>     return result;
>> }
>>
>> Const is infectious and just causes more problems making it pretty much useless in the general case. It's only suitable for a very narrow use case. A lot of people using D avoid const entirely, except for really basic simple things involving primitive types.
>
> Exactly my intent, to cause problems in this case and force a cast.
>
> For example, your code is actually not performing a copy of A. Your copy constructor is making a shallow copy, by duplicating only one vertex of the memory graph. If you were actually copying it, you would allocate a new int on the heap and initializa it from the const source.ptr without a problem.
>
> Current blitting is perfect for shallow copies as it is.

Blitting can't copy from one object type to another though. This constructor will be able to do that.

> If we are to give default semantical meaning to the copy constructor, that the programmer can generally rely on in the unfamiliar codebase, we need to restrict the developer. If we are to give it a semantic meaning of the function where you do whatever you want with source and dest, why should we call it a COPY constructor?

Sure it's not a very accurate name, changing the name to be something else is fine. Just don't change the implementation to require const and make it that much harder to use.

On Tuesday, 18 December 2018 at 14:51:39 UTC, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review for DIP 1018, "The Copy Constructor":
>
> https://github.com/dlang/DIPs/blob/07da1f2cabc8b1bc3ad66034598a133e5ad13356/DIPs/DIP1018.md
>
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on January 4, or when I make a post declaring it complete. (This time I'm extending the review period by a few days because of the holidays.)
>
> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round of community review. Otherwise, it will be queued for the Final Review and Formal Assessment by the language maintainers.
>
> Please familiarize yourself with the documentation for the Community Review before participating.
>
> https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#community-review
>
> Thanks in advance to all who participate.
Motivation section needs to cover the RC GC algorithm and why the current features is unable to implement the algorithm properly while the introduction of copy constructor allows to implement it. That is the major motivation that can't be over looked.

Alex.

On Saturday, 22 December 2018 at 09:07:52 UTC, Boris-Barboris wrote:
> On Saturday, 22 December 2018 at 02:02:55 UTC, Nicholas Wilson wrote:
>> On Saturday, 22 December 2018 at 00:08:51 UTC, Boris-Barboris wrote:
>>
>> One of the primary use cases for this is ref-counting stuff, the refcount needs to be mutable.
>
> I thought ref-counted stuff is perfectly fine with postblits, was I wrong?

Nope, it turns out you can't implement ref-counting GC with postbilts. Andrei have tried to do this already and failed. That why they are introducing it, (and why it should be covered in the motivation section in the DIP).

On Tuesday, 18 December 2018 at 14:51:39 UTC, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review for DIP 1018, "The Copy Constructor":
>
> https://github.com/dlang/DIPs/blob/07da1f2cabc8b1bc3ad66034598a133e5ad13356/DIPs/DIP1018.md
>
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on January 4, or when I make a post declaring it complete. (This time I'm extending the review period by a few days because of the holidays.)
>
> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round of community review. Otherwise, it will be queued for the Final Review and Formal Assessment by the language maintainers.
>
> Please familiarize yourself with the documentation for the Community Review before participating.
>
> https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#community-review
>
> Thanks in advance to all who participate.

I only realised that there would be a change to move semantics for structs that define a copy constructor at the end. Can one still manually move such structs? I'd like the DIP to go into that.

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)


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


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.


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)

On Friday, 21 December 2018 at 14:43:50 UTC, Boris-Barboris wrote:
> On Tuesday, 18 December 2018 at 14:51:39 UTC, Mike Parker wrote:
>> This is the feedback thread for the first round of Community Review for DIP 1018, "The Copy Constructor"
>
> 1). I do not like the ability to specify a mutable copy source. Under no circumstance should the code like
>
> A a;
> A fun()
> {
>     return a;      // lowered to return tmp.copyCtor(a)
> }
>
> void main()
> {
>     A b = fun();    // the return value of fun() is moved to the location of b
> }
>
> be allowed to modify the value of a. This is an absolute pain to read\debug, and I would instead like to see a mandatory const\immutable on the source reference.
> Copy operation should not modify the source, or it should not be called a copy. If we are talking about a typical smart pointer struct (Refcounted), copy still should not modify the source. It is D's transitive const\immutable that is a problem here and it must be explicitly casted away by the developer.
>
> Only const also mitigates the combinatorial mess caused by 4 combinations of mutable\immutable copy constructors, since immutable is implicitly converted to const.

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.

> 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.

> 3). Section "Copy constructor usage", I don't understand the difference:
>
> void main()
> {
>     A a;
>     A b = a; // copy constructor gets called
>     b = a;   // assignment, not initialization
> }
>
A b = a is an initialization, therefore the copy constructor gets called.
b = a is an assignemnt => opAssign gets called

> and
>
> void main()
> {
>     A a;
>     a = fun();      // NRVO - no copy constructor call
fun() returns by value so a copy constructor call should be performed when returning, but in this case, because named return value optimization may be
applied, it will elide the copy.

>     A b; // b is initialized after this semicolon.
>     // why is this one not an assignment, but initialization?
>     // do we have the difference formally and consistently defined for structs?
>     b = gun();      // NRVO cannot be performed, copy constructor is called
> }

Indeed, these examples are a bit confusing, because the copy constructor is called when returning, not on caller site. Even if you would have `A b = gun()`, the value of gun() would be moved, but a copy constructor may be called when returning from gun.

On Saturday, 22 December 2018 at 08:40:21 UTC, Johannes Loher wrote:
> Those two statements contradict each other. The first one clearly states that whenever no copy constructor is defined, copying is done by blitting (without exception).

You are right, it's not very well stated, but I thought it would be obvious from the examples. The idea here is that when a struct does not define any copy constructors and the fields of the struct do not define any copy constructors, blitting is employed. If at least one field does define a copy constructor, then the above mentioned copy constructor is generated. Hope this sheds some light.

On Saturday, 22 December 2018 at 14:43:46 UTC, 12345swordy wrote:
> On Saturday, 22 December 2018 at 09:07:52 UTC, Boris-Barboris wrote:
>> On Saturday, 22 December 2018 at 02:02:55 UTC, Nicholas Wilson wrote:
>>> On Saturday, 22 December 2018 at 00:08:51 UTC, Boris-Barboris wrote:
>>>
>>> One of the primary use cases for this is ref-counting stuff, the refcount needs to be mutable.
>>
>> I thought ref-counted stuff is perfectly fine with postblits, was I wrong?
>
> Nope, it turns out you can't implement ref-counting GC with postbilts. Andrei have tried to do this already and failed. That why they are introducing it, (and why it should be covered in the motivation section in the DIP).

The motivation section, although it does not link to the RC GC, it describes the problems encountered when trying to implement it. The main problem was the fact that when dealing with immutable fields, after the blitting phase is done it is not obvious in  what state the immutable fields is (raw/cooked?).

On Saturday, 22 December 2018 at 16:40:47 UTC, Atila Neves wrote:
> On Tuesday, 18 December 2018 at 14:51:39 UTC, Mike Parker wrote:
>> This is the feedback thread for the first round of Community Review for DIP 1018, "The Copy Constructor":
>>
>> https://github.com/dlang/DIPs/blob/07da1f2cabc8b1bc3ad66034598a133e5ad13356/DIPs/DIP1018.md
>>
>> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on January 4, or when I make a post declaring it complete. (This time I'm extending the review period by a few days because of the holidays.)
>>
>> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round of community review. Otherwise, it will be queued for the Final Review and Formal Assessment by the language maintainers.
>>
>> Please familiarize yourself with the documentation for the Community Review before participating.
>>
>> https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#community-review
>>
>> Thanks in advance to all who participate.
>
> I only realised that there would be a change to move semantics for structs that define a copy constructor at the end. Can one still manually move such structs? I'd like the DIP to go into that.

I think that move and copy constructors are orthogonal concepts. I don't see how copy constructors may affect this. At this point, there is no move operator in D and the copy constructor is used only when a copy is made. If you are referring to Shackar Shamesh's DIP, then yes, one can still manually move such structs. When a copy is made => copy constructor ; when a move is made => opMove