-
-
Notifications
You must be signed in to change notification settings - Fork 421
Move doesPointTo #2447
base: master
Are you sure you want to change the base?
Move doesPointTo #2447
Conversation
Thanks for your pull request and interest in making D better, @TurkeyMan! 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 referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. 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 + druntime#2447" |
Is the second commit dependent on the first one? i.e. does the first one need to be there if its already in another PR? |
Yes, the first PR should be merged, this one should not be merged... unless someone can convince me otherwise. |
Another thought; |
Yeah, for compile generated moves you need DIP1014 implemented, but for explicit moves there's no reason why we can't start supporting it now. |
Note that without this PR we can't get rid of the now duplicate implementation of |
Yeah see, I didn't move that function, because it's obscene. The fact that function is like 2 thousand lines suggests at some serious deficiency somewhere. |
As a bonus on top of #2442, this re-enables the message that complains if you try and
move
a type with an interior pointer.I'm not even convinced the assert should exist; if I want to move something, I want to bloody move it!
If I was stupid enough to create a thing with an interior pointer; then shame on me, but that shouldn't cause a compile error when I move it... perhaps the interior pointer is a tiny subset of the thing, which I intend to handle manually. If I need to move it, there's no escape hatch here. It feels like a higher-level concern.
Anyway, aside from the question of whether that assert should even be there at all, this PR pulls ~2000 new lines of code... to emit that error message.
I don't know if I'm comfortable with the idea of introducing 2000 lines to druntime that do nothing except emit an error message when the user made a possible mistake.
I'm not convinced the value justifies the cost...