Bug 20965 – Implicitly generated postblit overrides disabled copy ctor

There's a bit of a contradiction in the spec. On one hand, it says [1]: "When a copy constructor is defined for a struct, all implicit blitting is disabled for that struct" On the other [2]: "WARNING: The postblit is considered legacy and is not recommended for new code. Code should use copy constructors defined in the previous section. For backward compatibility reasons, a struct that defines both a copy constructor and a postblit will only use the postblit for implicit copying." and "...the compiler may generate ... postblit functions" [1] https://dlang.org/spec/struct.html#struct-copy-constructor [2] https://dlang.org/spec/struct.html#struct-postblit What that results in in practice is that a generated postblit overrides copy constructors. Consider: struct C { this(this) {} } struct S { C c; @disable this(ref typeof(this)); } void main() { S s1; auto s2 = s1; // problem } The line 'problem' compiles. I posit that it shouldn't. Alas: - a copy ctor is present (and disabled), therefore implicit blitting should be disabled, however - C defines a postblit, therefore a postblit is implicitly generated for S, therefore S "defines" both a copy ctor and a postblit after all, therefore only the postblit is considered. Even though it is implicit and so should be disabled :) Consider that type C comes from elsewhere. Programmers that write new code, ostensibly following advice from the WARNING cited above, shouldn't have to manually introspect dependencies to find out whether they have postblits (that is, if the whole point is to get rid of postblits). The language should make the right call here, i.e. adhere to the first rule from the copy ctors spec: disable all implicit blits (i.e. include generated postblits in that rule).

You are correct, however, the initial idea was to favor the use of postblits until they are deprecated. If you want to make sure that all copying is disabled, simply do: @disable this(this); @disable this(ref typeof(this)); You don't have to reflect on any code this way. The rule of thumb is: if you want to make sure that a posblit does not get in your way you must defensively disable the postblit and then the copy constructor will have priority.

Sorry, that's not a good design. If postblit is going to be deprecated, let people gradually prune their codebases of postblits instead of forcing them to add more!

(In reply to RazvanN from comment #1) > @disable this(this); > @disable this(ref typeof(this)); > > You don't have to reflect on any code this way. If I don't want to further infest the codebase with postblits, I have to. > The rule of thumb is: if you want to make sure that a posblit does not get > in your way you must defensively disable the postblit and then the copy > constructor will have priority. Postblits will never go away then, until they stop compiling.

(In reply to Stanislav Blinov from comment #3) > (In reply to RazvanN from comment #1) > > > @disable this(this); > > @disable this(ref typeof(this)); > > > > You don't have to reflect on any code this way. > > If I don't want to further infest the codebase with postblits, I have to. > > > The rule of thumb is: if you want to make sure that a posblit does not get > > in your way you must defensively disable the postblit and then the copy > > constructor will have priority. > > Postblits will never go away then, until they stop compiling. I agree, however, initially you start with tons of codebases that have postblit and 0 that have copy constructor. The DIP [1] explicitly says: "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." This was Walter's request and I don't think that this will change. [1] https://github.com/dlang/DIPs/pull/129/files#diff-ecee0474c4314cd47dd8c2656b485c0cfd56e704a85de75839ec2850fb61f0ebR282

I will make a PR to try and fix this and see what happens.

@RazvanN7 created dlang/dmd pull request #11945 "Fix Issues 20714, 20965 - Postblit has priority over copy constructor" fixing this issue: - Fix Issues 20714, 20965 - Postblit has priority over copy constructor https://github.com/dlang/dmd/pull/11945

dlang/dmd pull request #11945 "Fix Issues 20714, 20965 - Postblit has priority over copy constructor" was merged into master: - 40d0661190ac132dbb5d61e3804dd22bbea26602 by RazvanN7: Fix Issues 20714, 20965 - Postblit has priority over copy constructor https://github.com/dlang/dmd/pull/11945

dlang/dmd pull request #11947 "Revert PR 11945 (Postblit has priority over copy constructor)" was merged into master: - bc6976a4cc7abe799856689c077406a269dc0c51 by Geod24: Revert "Fix Issues 20714, 20965 - Postblit has priority over copy constructor" This reverts commit c35ace622492da4ca53c055c9af0fa0346aa178b. PR 11945 / this commit introduces a silent change of behavior, which leads to fields with postblit not having their postblit called anymore. A deprecation path was proposed, and would require the user to disable postblit when a copy constructor is present, or issue a deprecation message otherwise. Other options are likely available to avoid a silent behavior change. However, since the PR was merged within 4 minutes of being submitted, there were no time for such suggestions. Additionally, the PR was lacking both a spec PR and a changelog entry. https://github.com/dlang/dmd/pull/11947