Fix Issue 18719 - Doubly-called constructor against member when using forwarding constructors #8167
Conversation
|
Thanks for your pull request and interest in making D better, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + dmd#8167" |
|
Blocked by : dlang/phobos#6448 |
|
I don't know how many times this needs to be said: If your compiler change breaks Phobos, it likely breaks user code. There needs to be at least some proof of effort that this couldn't be done as a deprecation, and there must be a changelog entry with rationale, examples, and steps for fixing user code. |
src/dmd/expressionsem.d
Outdated
| // If `exp` is a call expression to another constructor | ||
| // then it means that all struct/class fields will be | ||
| // initialized after this call. | ||
| for (size_t i = 0; i < sc.ctorflow.fieldinit.length; i++) |
| @@ -3271,11 +3271,22 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor | |||
| } | |||
| else | |||
| { | |||
| // `this` call expression must be inside a | |||
| // constructor | |||
| if (!ad || !sc.func.isCtorDeclaration()) | |||
| { | |||
| exp.error("constructor call must be in a constructor"); | |||
There was a problem hiding this comment.
it might if one is noobish enough
|
@CyberShadow I transformed the error into a deprecation and added a changelog entry. |
| } | ||
| --- | ||
|
|
||
| However, issue 18719 ( https://issues.dlang.org/show_bug.cgi?id=18719 ) shows |
There was a problem hiding this comment.
$(LINK2 https://issues.dlang.org/show_bug.cgi?id=18719, issue 18719)
| @@ -29,6 +29,7 @@ enum CSX : ubyte | |||
| return_ = 0x20, /// seen a return statement | |||
| any_ctor = 0x40, /// either this() or super() was called | |||
| halt = 0x80, /// assert(0) | |||
| deprecate_18719 = 0x100, // issue deprecation for Issue 18719 - delete when deprecation period is over | |||
There was a problem hiding this comment.
Can we call it deprecate so it can be used for other deprecations?
There was a problem hiding this comment.
It is highly unlikely that this is going to be used for something else. even so, the modification can be done when it is necessary
RazvanN7
force-pushed
the
Issue_18719
branch
2 times, most recently
from
387f0fc to
9543f5d
Compare
… forwarding constructors
|
This introduced a regression as it only checks whether a |
Whenever a call expression to a constructor is encountered, update the scope ctorflow so that it is known that all member fields have been initialized