-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
PinnableSlice move assignment #2997
Conversation
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.
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@yiwu-arbug has updated the pull request. View: changes, changes since last import |
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.
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
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.
lgtm.
Minor comments. Take care of them before landing.
src.mk
Outdated
@@ -307,11 +307,12 @@ MAIN_SOURCES = \ | |||
memtable/skiplist_test.cc \ | |||
memtable/write_buffer_manager_test.cc \ | |||
monitoring/histogram_test.cc \ | |||
monitoring/iostats_context_test.cc \ | |||
monitoring/iostats_context_st.cc \ |
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.
I think you didn't mean to change this.
src.mk
Outdated
@@ -336,6 +337,7 @@ MAIN_SOURCES = \ | |||
util/filelock_test.cc \ | |||
util/log_write_bench.cc \ | |||
util/rate_limiter_test.cc \ | |||
util/slice_test.cc \ |
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.
fix indent.
@yiwu-arbug has updated the pull request. View: changes, changes since last import |
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.
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
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.
@yiwu-arbug is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Allow `std::move(pinnable_slice)`. Closes #2997 Differential Revision: D6036782 Pulled By: yiwu-arbug fbshipit-source-id: 583fb0419a97e437ff530f4305822341cd3381fa
Summary:
Allow
std::move(pinnable_slice)
.Test Plan:
Add the new slice_test and run with ASAN.