Bug 22639 – Copy constructors with default arguments not getting called

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
All
Creation time
2021-12-31T03:26:45Z
Last change time
2022-03-27T02:24:27Z
Keywords
pull
Assigned to
No Owner
Creator
Tejas_Garhewal

Comments

Comment #0 by scienticman — 2021-12-31T03:26:45Z
``` import std.stdio:writeln; struct A { this(ref return scope A rhs) inout {} this(ref return scope const A rhs, int b = 7) inout { if (b != 7) { this.b = b; } } this(this) @disable; int a=4; int b=3; } void main() { A a = A(); //A c = A(a,10); // Doesn't work :( /+ A c = void; c.__ctor(a, 200); //works if I write like this +/ A* b = new A(a, 10); //But this works! writeln(b.b); writeln(c.b); } ```
Comment #1 by razvan.nitu1305 — 2022-03-15T11:43:08Z
This is a 2.096.1 regression.
Comment #2 by razvan.nitu1305 — 2022-03-15T12:56:40Z
What happens here: before https://github.com/dlang/dmd/pull/12132 , the copy constructor had priority over default construction, however, there were numerous bug reports which argued that a copy constructor is not actually a normal constructor and therefore should not disable default initialization. I implemented what was asked for, but now this PR complains the other way, that copy constructors should have priority over default initialization. Now what can be done about this. Well, we could modify the implementation so that if a copy constructor has additional default parameters than it will disable default construction, but, honestly, this seems like a hack. Another solution would be to first try the copy constructors (if there aren't any regular constructors) and if a match is found, then just go with it, otherwise, go with default initialization, but this is problematic since it (1) complicates the implementation and (2) it complicates the spec (before copy constructors it was simple: you have a constructor => default construction is disabled). Personally, I think that if we decided that copy constructors are just constructors that additionally may be called implicitly by the compiler in some situations, then we should just stick with that (this means reverting the aforementioned PR). However, since there are folks that consider that the copy constructor is not a normal constructor and therefore default initialization should be affected by their presence, then this bug is invalid.
Comment #3 by stanislav.blinov — 2022-03-15T14:07:13Z
I don't see how this is a matter of priority. A copy constructor cannot be ambiguous with field initialization, therefore it shouldn't prohibit it. struct A { A a; // this is impossible... } ...therefore any initialization that takes `A` as first argument must be a constructor call, and the implementation has to figure out which constructor it should be. Given that in the future D would likely also be getting move constructors (i.e. constructors taking rvalue argument of typeof(this) would also be "special"), it'd be advantageous to sort out this problem now, otherwise we stand at risk of magnifying its impact. I think that both them and copy constructors should not interfere with field initialization. Let's not follow C++ further down its constructor hell. The line `A c = A(a, 10);` from the original example should "just work" (c). As should `A c = A(1, 2);`, without requiring the programmer to spell out any other constructors.
Comment #4 by dlang-bot — 2022-03-21T10:46:36Z
@RazvanN7 created dlang/dmd pull request #13853 "Fix Issue 22639 - Copy constructors with default arguments not getting called" fixing this issue: - Fix Issue 22639 - Copy constructors with default arguments not getting called https://github.com/dlang/dmd/pull/13853
Comment #5 by dlang-bot — 2022-03-21T22:51:47Z
dlang/dmd pull request #13853 "Fix Issue 22639 - Copy constructors with default arguments not getting called" was merged into stable: - 293da5306c9b052054fabe199eb2112665fd4add by RazvanN7: Fix Issue 22639 - Copy constructors with default arguments not getting called https://github.com/dlang/dmd/pull/13853
Comment #6 by dlang-bot — 2022-03-27T02:24:27Z
dlang/dmd pull request #13892 "Merge stable into master" was merged into master: - e36640f01633b09763e76650ef607615e0832820 by RazvanN7: Fix Issue 22639 - Copy constructors with default arguments not getting called https://github.com/dlang/dmd/pull/13892