Skip to content
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 checked move #378

Merged
merged 3 commits into from
Sep 1, 2019
Merged

Add checked move #378

merged 3 commits into from
Sep 1, 2019

Conversation

redboltz
Copy link
Owner

No description provided.

force_move() is std::move() wrapper that detect unexpected copy fallback.
@redboltz
Copy link
Owner Author

I'm not sure the name of the function force_move() is appropriate?
Other choice is checked_move(). Or something else...

@jonesmz
Copy link
Contributor

jonesmz commented Aug 31, 2019

I'm not sure I understand the goal of this change.

Just to ensure that std::move() is only called on things that will actually be moved?

Is there a bug?

Or is this to just make the code easier to understand?

@redboltz
Copy link
Owner Author

redboltz commented Aug 31, 2019

See https://stackoverflow.com/questions/57105812/how-to-detect-unexpected-copy-in-move-expected-case/57106531#57106531

std::move(v) invokes copy constructor if v is not movable but copyable.
But in mqtt_cpp, I don't expect such behavior. When I call std::move(), I always expect move operation. So I need to avoid fallback copy operation. The PR does it.

The PR contains two commits. The first one is introduced wrapped move. Then I got some unexpected copy fallbacks. The second commit fixed them.

@jonesmz
Copy link
Contributor

jonesmz commented Aug 31, 2019

Alright, that's reasonable

@redboltz
Copy link
Owner Author

redboltz commented Sep 1, 2019

https://github.com/redboltz/mqtt_cpp/pull/378/files#diff-45ca9626e6c7c611be2ceb2ff63031a5R470

I replaced move with copy at the point above.

See
https://stackoverflow.com/questions/57335172/is-there-any-way-to-move-an-element-from-ordered-index-ed-multi-index

In modify() function, if the element in the container is moved, then access moved from object to re-arrange elements.

@redboltz redboltz merged commit 84bff9b into master Sep 1, 2019
@redboltz redboltz deleted the add_checked_move branch September 1, 2019 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants