Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1696093002:
Support smart pointers in base::DispatchToMethod (Closed)
DescriptionSupport smart pointers in base::DispatchToMethod
* Make base::DispatchToMethod to accept smart pointers as the this pointer
* Remove base::internal::UnwrapTraits from //storage code
BUG=554299
Committed: https://crrev.com/1ea87e3a0101b7860efe4bbec8f3c539a33facd9
Cr-Commit-Position: refs/heads/master@{#375516}
Patch Set 1 #
Total comments: 3
Messages
Total messages: 20 (9 generated)
Description was changed from ========== Support smart pointers in base::DispatchToMethod BUG= ========== to ========== Support smart pointers in base::DispatchToMethod * Make it possible to use smart pointers as the this pointer in base::DispatchToMethod * Remove base::internal::UnwrapTraits from //storage code BUG= ==========
tzik@chromium.org changed reviewers: + nhiroki@chromium.org, thakis@chromium.org
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Description was changed from ========== Support smart pointers in base::DispatchToMethod * Make it possible to use smart pointers as the this pointer in base::DispatchToMethod * Remove base::internal::UnwrapTraits from //storage code BUG= ========== to ========== Support smart pointers in base::DispatchToMethod * Make it possible to use smart pointers as the this pointer in base::DispatchToMethod * Remove base::internal::UnwrapTraits from //storage code BUG=554299 ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1696093002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1696093002/1
Description was changed from ========== Support smart pointers in base::DispatchToMethod * Make it possible to use smart pointers as the this pointer in base::DispatchToMethod * Remove base::internal::UnwrapTraits from //storage code BUG=554299 ========== to ========== Support smart pointers in base::DispatchToMethod * Make base::DispatchToMethod to accept smart pointers as the this pointer * Remove base::internal::UnwrapTraits from //storage code BUG=554299 ==========
PTAL
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1696093002/diff/1/base/tuple.h File base/tuple.h (right): https://codereview.chromium.org/1696093002/diff/1/base/tuple.h#newcode203 base/tuple.h:203: (obj->*method)(base::internal::UnwrapTraits<Ts>::Unwrap(get<Ns>(arg))...); Why doesn't this have to be (obj.*method) now? https://codereview.chromium.org/1696093002/diff/1/base/tuple.h#newcode241 base/tuple.h:241: (obj->*method)(base::internal::UnwrapTraits<InTs>::Unwrap(get<InNs>(in))..., same question
https://codereview.chromium.org/1696093002/diff/1/base/tuple.h File base/tuple.h (right): https://codereview.chromium.org/1696093002/diff/1/base/tuple.h#newcode203 base/tuple.h:203: (obj->*method)(base::internal::UnwrapTraits<Ts>::Unwrap(get<Ns>(arg))...); On 2016/02/15 15:22:34, Nico wrote: > Why doesn't this have to be (obj.*method) now? In new code, obj will be a const ref to a T* or a const ref to a scoped_refptr<T> or scoped_ptr<T>. It's possible to assume the type is a ref to the object, but that requires a caller side change from ptr to *ptr. Hm, there are only small number of caller of DispatchToMethod. I can update them if we should.
I see. lgtm then. I think updating callers would be ever so slightly less confusing, but up to you if you do this or not.
On 2016/02/15 21:36:10, Nico wrote: > I see. lgtm then. I think updating callers would be ever so slightly less > confusing, but up to you if you do this or not. Let me revisit it later. I'll revisit this to update it for move-only types.
The CQ bit was checked by tzik@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1696093002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1696093002/1
Message was sent while issue was closed.
Description was changed from ========== Support smart pointers in base::DispatchToMethod * Make base::DispatchToMethod to accept smart pointers as the this pointer * Remove base::internal::UnwrapTraits from //storage code BUG=554299 ========== to ========== Support smart pointers in base::DispatchToMethod * Make base::DispatchToMethod to accept smart pointers as the this pointer * Remove base::internal::UnwrapTraits from //storage code BUG=554299 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Support smart pointers in base::DispatchToMethod * Make base::DispatchToMethod to accept smart pointers as the this pointer * Remove base::internal::UnwrapTraits from //storage code BUG=554299 ========== to ========== Support smart pointers in base::DispatchToMethod * Make base::DispatchToMethod to accept smart pointers as the this pointer * Remove base::internal::UnwrapTraits from //storage code BUG=554299 Committed: https://crrev.com/1ea87e3a0101b7860efe4bbec8f3c539a33facd9 Cr-Commit-Position: refs/heads/master@{#375516} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/1ea87e3a0101b7860efe4bbec8f3c539a33facd9 Cr-Commit-Position: refs/heads/master@{#375516} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
