Jump to page: 1 2
Thread overview
[Issue 21349] copy and postblit constructors aren't compatible
Oct 29, 2020
Paul Backus
Oct 29, 2020
Paul Backus
Nov 11, 2020
RazvanN
Nov 11, 2020
RazvanN
Nov 11, 2020
RazvanN
Nov 11, 2020
RazvanN
Nov 11, 2020
RazvanN
Nov 17, 2020
RazvanN
October 29, 2020
https://issues.dlang.org/show_bug.cgi?id=21349

--- Comment #1 from Илья Ярошенко <ilyayaroshenko@gmail.com> ---
The last assert fails

--
October 29, 2020
https://issues.dlang.org/show_bug.cgi?id=21349

Paul Backus <snarwin+bugzilla@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |snarwin+bugzilla@gmail.com
           Severity|blocker                     |enhancement

--- Comment #2 from Paul Backus <snarwin+bugzilla@gmail.com> ---
This is an enhancement request, not a bug. Per the language spec:

> For backward compatibility reasons, a struct that defines both a copy constructor and a postblit will only use the postblit for implicit copying.

Source: https://dlang.org/spec/struct.html#struct-postblit

You can work around this by placing sOld in a union, since unions do not have generated postblits:

struct C
{
    union { SOld sOld; }
    SNew sNew;

    this(ref C other) {
        pragma(inline, false);
        sOld = other.sOld;
        sNew = other.sNew;
    }
}

Of course, if SOld has any other special member functions, such as a destructor or identity opAssign overload, you will have to implement those manually for C as well.

--
October 29, 2020
https://issues.dlang.org/show_bug.cgi?id=21349

--- Comment #3 from Илья Ярошенко <ilyayaroshenko@gmail.com> ---
well. The language spec is buggy.

--
October 29, 2020
https://issues.dlang.org/show_bug.cgi?id=21349

--- Comment #4 from Илья Ярошенко <ilyayaroshenko@gmail.com> ---
btw, I don't see whare spec says that an aggregate can't hold both old and new style members

struct C
{
    SOld sOld;
    SNew sNew;
}

void main()
{
    C c;
    auto d = c;
    assert(d.sOld.s);
    assert(d.sNew.s);
}

This is a buggy definition too and doesn't pass second assert too.

The spec doesn't say that this shouldn't work.
It just ignores this case. But this is definitely the language design bug.

--
October 29, 2020
https://issues.dlang.org/show_bug.cgi?id=21349

Илья Ярошенко <ilyayaroshenko@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|enhancement                 |blocker

--
October 29, 2020
https://issues.dlang.org/show_bug.cgi?id=21349

--- Comment #5 from Paul Backus <snarwin+bugzilla@gmail.com> ---
When an aggregate has both old and new style members, the compiler generates both a copy constructor and a postblit, and the postblit takes precedence.

I agree that this is a bug in the language design: the generated copy constructor will call the members' postblits, so it should be the one that takes precedence.

--
October 29, 2020
https://issues.dlang.org/show_bug.cgi?id=21349

--- Comment #6 from Илья Ярошенко <ilyayaroshenko@gmail.com> ---
I have thought think about these lines

> For backward compatibility reasons, a struct that defines both a copy constructor and a postblit will only use the postblit for implicit copying.

What they are really about?

First, they say about "struct that defines both ", but in the second case the struct doesn't define neither. The compiler generates something, and we can suppose that compiler can generate something good. For example a new style copy constructor. And it should generate it instead of postblit. Why? because the second:

Second, "For backward compatibility reasons". The reason is backward compatibility, not something else.

I mean that "struct that defines both" should be understood as "struct that _explicilty_ defines both".

So, even with the current spec, this can work, and if it can work, then it should work.

--
November 11, 2020
https://issues.dlang.org/show_bug.cgi?id=21349

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

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

--- Comment #7 from RazvanN <razvan.nitu1305@gmail.com> ---
(In reply to Илья Ярошенко from comment #6)
> I have thought think about these lines
> 
> > For backward compatibility reasons, a struct that defines both a copy constructor and a postblit will only use the postblit for implicit copying.
> 
> What they are really about?
> 
> First, they say about "struct that defines both ", but in the second case the struct doesn't define neither. The compiler generates something, and we can suppose that compiler can generate something good. For example a new style copy constructor. And it should generate it instead of postblit. Why? because the second:
> 
> Second, "For backward compatibility reasons". The reason is backward compatibility, not something else.
> 
> I mean that "struct that defines both" should be understood as "struct that _explicilty_ defines both".
> 
> So, even with the current spec, this can work, and if it can work, then it should work.

I agree that the spec might be misleading, however, the DIP has a more exact formulation [1]:

"In order to assure a smooth transition from postblit to copy constructor, the
following strategy is employed: if a `struct` defines a postblit (user-defined
or generated) all copy constructor definitions will be ignored for that
particular `struct` and the postblit will be preferred. Existing code bases
that do not use the postblit may start using the copy constructor without any
problems, while codebases that rely on the postblit may
start writing new code using the copy constructor and remove the deprecated
postblit from their code."

So from the beginning you must either use one or other; having both in your code will not fly. In this specific example, you have 2 solutions: either you replace the postblit with a copy constructor everywhere (which is the preferred solution) or you simply switch C to a copy constructor and later on update the code base to get rid of postblits.


[1] https://github.com/dlang/DIPs/pull/129/files#diff-ecee0474c4314cd47dd8c2656b485c0cfd56e704a85de75839ec2850fb61f0ebR282

--
November 11, 2020
https://issues.dlang.org/show_bug.cgi?id=21349

--- Comment #8 from RazvanN <razvan.nitu1305@gmail.com> ---
(In reply to Paul Backus from comment #5)
> When an aggregate has both old and new style members, the compiler generates both a copy constructor and a postblit, and the postblit takes precedence.
> 
> I agree that this is a bug in the language design: the generated copy constructor will call the members' postblits, so it should be the one that takes precedence.

No, it does not generate both. It first looks for user defined postblits in the struct or in its fields and if it finds any, it will ignore user defined copy constructors. So in this case, C will not have a copy constructor because what the user wants takes precedence over generated methods.

--
November 11, 2020
https://issues.dlang.org/show_bug.cgi?id=21349

--- Comment #9 from RazvanN <razvan.nitu1305@gmail.com> ---
(In reply to Илья Ярошенко from comment #4)
> btw, I don't see whare spec says that an aggregate can't hold both old and new style members
> 
> struct C
> {
>     SOld sOld;
>     SNew sNew;
> }
> 
> void main()
> {
>     C c;
>     auto d = c;
>     assert(d.sOld.s);
>     assert(d.sNew.s);
> }
> 
> This is a buggy definition too and doesn't pass second assert too.
> 
> The spec doesn't say that this shouldn't work.
> It just ignores this case. But this is definitely the language design bug.

It does not say that explicitly, however, the DIP specifies that if you have a postblit (user-defined or generated) then copy constructors will be ignored. In your case, one of the fields has a postblit and therefore struct C will have a postblit generated for it as per the rules in [1]. Now the rules stated in the copy constructor DIP enter the scene and therefore a copy constructor is not going to get generated.

It is not a bug, it a language design choice to favor backwards compatibility so that codebases that heavily rely on the postblit will not be affected by the insertion of new structs that define copy constructors.

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

--
« First   ‹ Prev
1 2