Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

ctfe-able move #936

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Member

DmitryOlshansky commented Nov 11, 2012

Finally found a way to make acceptable version of move for CTFE.

Curently I opted to just do a copy. The implementation involves jumping through some hoops to ensure that RVO/NRVO is still applied for the run-time conterpart. See also:
http://d.puremagic.com/issues/show_bug.cgi?id=8991

As a side affect it fixes http://d.puremagic.com/issues/show_bug.cgi?id=8349 and maybe others.

@DmitryOlshansky DmitryOlshansky referenced this pull request Nov 11, 2012

Closed

[RFC] Fix `move` #923

Contributor

denis-sh commented Nov 12, 2012

Sorry, but I'm against this pull. Though move CTFE-ability is a good idea, you proposal is to make a function silently incorrectly working in CT. It's unacceptable.

In my pull #923 I did my best for CTFE so it will work in CT if there is no elaborate stuff and isAssignable!T is true i.e. my move will pass unittest from your pull. And it will fail to compile once it will understand that it can't work properly in CT unlike yours.

In the future, I plant to tweak my setInitialState to work in more CT cases and introduce CT-friendly memcpy analog: memoryCopy(T)(ref T, const ref T) (that will be faster then memcpy in regular case because it knows size at CT).

Note, that my destruct(t, false) is already CTFE-able as it only calls CTFE-able callDestructors.

Contributor

denis-sh commented Nov 12, 2012

And yes, thanks for showing the issue my pull fixes (I mean Issue 8349)

Member

DmitryOlshansky commented Nov 12, 2012

Sorry, but I'm against this pull. Though move CTFE-ability is a good idea, you proposal is to make a function silently > incorrectly working in CT. It's unacceptable.

Well if you talking about elaborate destructors and such then yes it doesn't do a move. My reasoning was that aside of specific tests copy vs move is a question of performance and extra copies/destroys. And since we are in CTFE it doesn't matter that much. Of course if CTFE had ability to cast pointers and reinterpret data as binary chunks I'd do full move.

Either way you may just pick unittest from my pull..

In the future, I plant to tweak my setInitialState to work in more CT cases and introduce CT-friendly memcpy analog: >memoryCopy(T)(ref T, const ref T).

Cool to see you catch this idea. I'm curious how you would make it CTFE friendly though.

Owner

andralex commented Dec 9, 2012

This may cause things like double destruction (of members inside the objects involved); I'm not sure to what extent that's a problem during compilation, but it does look a source of subtle differences between compile-time and run-time behavior.

Member

DmitryOlshansky commented Dec 9, 2012

I'm OK with better options but I fail to see a move being done in CTFE as 'move' as it doesn't allow bitblits like that.
@denis-sh maybe you can shed some light on how you'd do it for structs with elaborate destructor.

Contributor

denis-sh commented Dec 9, 2012

some light on how you'd do it for structs with elaborate destructor

As I already wrote above, my (recently rejected) destruct is already CTFE-able and planned memoryCopy will be CTFE-able too (it will replace memcpy and probably memmove). So move will be just a small function with a few calls to CTFE-able functions and will automatically became CTFE-able.

Member

DmitryOlshansky commented Dec 9, 2012

The problem is no in being able to destroy object. It is an ability to do a bitblit over typed data.
CTFE side of memoryCopy is what I ask about.

I claim that it is impossible in CTFE as it stands unless reinterpret casts are defined in CTFE (they are not).
Again this is a root of problem not the way we go about calling destructors in move.

Contributor

denis-sh commented Dec 10, 2012

As always I may be wrong so wait some time and I will either implement memoryCopy or will report a failure.

Member

DmitryOlshansky commented Dec 22, 2012

Closing as outdated, unsafe and not giving equvivalent semantics at CTFE.

Contributor

denis-sh commented Jan 1, 2013

To @blackwhale:

I claim that it is impossible in CTFE as it stands unless reinterpret casts are defined in CTFE (they are not).

I hope unstd.array.rawCopy will make you happy in this new year!
(click view source at the top of the page and see rawCopy unittests)

Member

DmitryOlshansky commented Jan 1, 2013

I hope unstd.array.rawCopy will make you happy in this new year!
(click view source at the top of the page and see rawCopy unittests)

Woha! The tupleof trick! This stuff is surely impressive.
Wish you more luck with pull requests this year :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment