Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1738883004:
Support for move-only types in SmallMap. (Closed)
|
Created:
4 years, 9 months ago by btolsch Modified:
4 years, 9 months ago Reviewers:
danakj CC:
chromium-reviews, gavinp+memory_chromium.org, vmpstr+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdded support for move-only types to SmallMap.
Also added move-only type support to ManualConstructor since SmallMap depends
on it. This enables SmallMap to be used with scoped_ptr and other move-only
types as a mapped type. This change was motivated by wanting to use SmallMap
instead of std::map to store a typically small set of scoped_ptr objects in this
issue: https://codereview.chromium.org/1744673002/ .
BUG=
R=danakj@chromium.org
Committed: https://crrev.com/0b80170cc8634062fc65c7d24ef47bdd8a66c017
Cr-Commit-Position: refs/heads/master@{#378287}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Move => InitFromMove rename #Patch Set 3 : Rebase #
Messages
Total messages: 23 (12 generated)
Description was changed from ========== Added support for move-only types to SmallMap. Also added move-only type support to ManualConstructor since SmallMap depends on it. This enables SmallMap to be used with scoped_ptr and other move-only types as a mapped type. This change was motivated by wanting to use SmallMap instead of std::map to store a typically small set of scoped_ptr objects in this issue: https://codereview.chromium.org/1744673002/ . BUG= R=thestig@chromium.org ========== to ========== Added support for move-only types to SmallMap. Also added move-only type support to ManualConstructor since SmallMap depends on it. This enables SmallMap to be used with scoped_ptr and other move-only types as a mapped type. This change was motivated by wanting to use SmallMap instead of std::map to store a typically small set of scoped_ptr objects in this issue: https://codereview.chromium.org/1744673002/ . BUG= R=thestig@chromium.org ==========
I realize this might not be a high priority for base/ but I think it would be useful going forward with C++11 integration. Also, the issue mentioned in the description could make use of it. PTAL.
thestig@chromium.org changed reviewers: + danakj@chromium.org
I'm taking some time off, as indicated by "OOO", so tossing this to danakj.
https://codereview.chromium.org/1738883004/diff/1/base/memory/manual_construc... File base/memory/manual_constructor.h (right): https://codereview.chromium.org/1738883004/diff/1/base/memory/manual_construc... base/memory/manual_constructor.h:61: inline void Move(ManualConstructor<Type>&& o) { std::move() means you're moving the object, this is a member function and you're initializing the object, by moving the parameter.. Can we name it InitFromMove()?
Description was changed from ========== Added support for move-only types to SmallMap. Also added move-only type support to ManualConstructor since SmallMap depends on it. This enables SmallMap to be used with scoped_ptr and other move-only types as a mapped type. This change was motivated by wanting to use SmallMap instead of std::map to store a typically small set of scoped_ptr objects in this issue: https://codereview.chromium.org/1744673002/ . BUG= R=thestig@chromium.org ========== to ========== Added support for move-only types to SmallMap. Also added move-only type support to ManualConstructor since SmallMap depends on it. This enables SmallMap to be used with scoped_ptr and other move-only types as a mapped type. This change was motivated by wanting to use SmallMap instead of std::map to store a typically small set of scoped_ptr objects in this issue: https://codereview.chromium.org/1744673002/ . BUG= R=danakj@chromium.org ==========
btolsch@chromium.org changed reviewers: - thestig@chromium.org
You're about the name Move() being subtly wrong so that is changed. https://codereview.chromium.org/1738883004/diff/1/base/memory/manual_construc... File base/memory/manual_constructor.h (right): https://codereview.chromium.org/1738883004/diff/1/base/memory/manual_construc... base/memory/manual_constructor.h:61: inline void Move(ManualConstructor<Type>&& o) { On 2016/02/27 00:32:02, danakj wrote: > std::move() means you're moving the object, this is a member function and you're > initializing the object, by moving the parameter.. > > Can we name it InitFromMove()? Done.
LGTM
The CQ bit was checked by btolsch@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1738883004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1738883004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
The CQ bit was checked by btolsch@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1738883004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1738883004/40001
The CQ bit was unchecked by btolsch@chromium.org
The CQ bit was checked by btolsch@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org Link to the patchset: https://codereview.chromium.org/1738883004/#ps40001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1738883004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1738883004/40001
Message was sent while issue was closed.
Description was changed from ========== Added support for move-only types to SmallMap. Also added move-only type support to ManualConstructor since SmallMap depends on it. This enables SmallMap to be used with scoped_ptr and other move-only types as a mapped type. This change was motivated by wanting to use SmallMap instead of std::map to store a typically small set of scoped_ptr objects in this issue: https://codereview.chromium.org/1744673002/ . BUG= R=danakj@chromium.org ========== to ========== Added support for move-only types to SmallMap. Also added move-only type support to ManualConstructor since SmallMap depends on it. This enables SmallMap to be used with scoped_ptr and other move-only types as a mapped type. This change was motivated by wanting to use SmallMap instead of std::map to store a typically small set of scoped_ptr objects in this issue: https://codereview.chromium.org/1744673002/ . BUG= R=danakj@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Added support for move-only types to SmallMap. Also added move-only type support to ManualConstructor since SmallMap depends on it. This enables SmallMap to be used with scoped_ptr and other move-only types as a mapped type. This change was motivated by wanting to use SmallMap instead of std::map to store a typically small set of scoped_ptr objects in this issue: https://codereview.chromium.org/1744673002/ . BUG= R=danakj@chromium.org ========== to ========== Added support for move-only types to SmallMap. Also added move-only type support to ManualConstructor since SmallMap depends on it. This enables SmallMap to be used with scoped_ptr and other move-only types as a mapped type. This change was motivated by wanting to use SmallMap instead of std::map to store a typically small set of scoped_ptr objects in this issue: https://codereview.chromium.org/1744673002/ . BUG= R=danakj@chromium.org Committed: https://crrev.com/0b80170cc8634062fc65c7d24ef47bdd8a66c017 Cr-Commit-Position: refs/heads/master@{#378287} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/0b80170cc8634062fc65c7d24ef47bdd8a66c017 Cr-Commit-Position: refs/heads/master@{#378287} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
