New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Issue 23182 - Can't assign struct with opAssign to SumType in CTFE #8474
Conversation
SumType.opAssign now avoids calling core.lifetime.move or core.lifetime.forward during CTFE whenever possible.
Thanks for your pull request and interest in making D better, @pbackus! 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 run digger -- build "master + phobos#8474" |
static if (isCopyable!(typeof(value))) | ||
{ | ||
// Workaround for https://issues.dlang.org/show_bug.cgi?id=21542 | ||
this = __ctfe ? value : move(value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about also adding a CTFE-version of move
being executable at compile-time by replacing the memcpy
with an void[]-copy
for the __ctfe
-case? Ping, @RazvanN7.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing a proper move
in CTFE is impossible, because CTFE does not support reinterpreting casts (which includes casting to void[]
). See PR #936 for related discussion. The best that can be done is to fall back to a copy, as is done here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Afaict, the core of this limitation is rather that the compiler is lacking the ability to automatically (internally) perform a move of value
into this
at
this = value;
as this statment is the last statement that references value
before this
is returned. The are more such cornercases where the compiler should move (but currently doesn't) thereby avoiding these static if
-branchings on isCopyable!T
. Afaict, if the compiler had that ability that would unlock being able to use non-copyable types at CTFE at least in situations like these. Ping, @andralex @RazvanN7 @WalterBright.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if the compiler could automatically turn these last-use assignments into moves, the explicit calls to move
and forward
would no longer be necessary in the first place, so there would be no need to add workarounds to avoid calling them in CTFE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. That would be awesome. Moreover, there are many Phobos' ranges that would be able to operate on non-copyable source range types if that restriction was lifted.
SumType.opAssign now avoids calling core.lifetime.move or core.lifetime.forward during CTFE whenever possible. Dlang issue: https://issues.dlang.org/show_bug.cgi?id=23182 Phobos PR: dlang/phobos#8474
SumType.opAssign now avoids calling core.lifetime.move or
core.lifetime.forward during CTFE whenever possible.