-
Notifications
You must be signed in to change notification settings - Fork 163
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
Add move semantics support #438
Comments
Speaking of Signature of move constructor should be like: image(image&& img) Although there're some design spaces here:
image_t a(...);
image_t b(std::move(a)); here's list of internal states on move construction I can think of:
|
@mocabe Any chance that you are working on this issue? Just to make sure we are not duplicating work... About the template copy constructor of |
@sdebionne I haven't really worked on this issue extensively yet, and it'll take several weeks before I can put more effort into this over other tasks I have right now. So if you can actively work on this right now, go for it!
Yeah, I though of that but current templated copy constructor is actually copying template <typename P2, bool IP2, typename Alloc2>
image(const image<P2,IP2,Alloc2>& img) : _memory(nullptr), _align_in_bytes(img._align_in_bytes), _alloc(img._alloc)
, _allocated_bytes( img._allocated_bytes ) {
allocate_and_copy(img.dimensions(),img._view);
} |
On move constructors, I'm still not sure we can just put Move assignments are even more tricky when we think of allocators. |
@sdebionne & @mocabe FYI, I'm not ignoring this discussion and the move semantics is a big deal for GIL, it is just taking time for me to find moment and think about it myself. Thanks for your help! |
@mloskot Thanks for your support! @mocabe Shall we just remove the |
@sdebionne & @mocabe I posted my initial review of #457
Feel free to open issues/tasks/todos whenever you think it's useful for yourself or others. Actually, the move assignment operator sample I attached to the review may address some of those issues that you have in mind. |
@mloskot Do you see any other core types that need move semantic or shall we mark this issue as done? |
@sdebionne I haven't checked in details for any other obvious types. The iterators and locators could be. I don't think we have any that manage heavy resources. |
AFAIK, all iterators and locators are lightweight (as one would expect such objects to be). |
Yes, they are, and all simple values should be moveable, in the standard library they must be. Here is nice summary of the gist https://stackoverflow.com/a/14316175/151641 That's why I mentioned them. |
Since GIL switched to C++11, it's the right time to consider enabling move semantics for the core types which own resources, especially the
image
class.Contributions are welcome. Please, see the CONTRIBUTING.md for guidelines on how to.
The text was updated successfully, but these errors were encountered: