Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1807153002:
WTF: Implement move semantics for values of {List,Linked}HashSet. (Closed)
|
Created:
4 years, 9 months ago by Yuta Kitamura Modified:
4 years, 9 months ago CC:
chromium-reviews, blink-reviews, blink-reviews-wtf_chromium.org, Mikhail Base URL:
https://chromium.googlesource.com/chromium/src.git@ListHashSet-AddResult Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionWTF: Implement move semantics for values of {List,Linked}HashSet.
This is similar to two earlier patches that implemented move semantics
in HashMap and HashSet:
https://codereview.chromium.org/1764973002/
https://codereview.chromium.org/1798913002/
This patch is for ListHashSet and LinkedHashSet. The changes in this
patch are mostly the same as the previous patches: taking an incoming
value as a universal reference (T&&), and forwarding it as appropriate.
BUG=567139, 582349
Committed: https://crrev.com/b1d3886202efe86f89e711a3b61ab250fadffbd2
Cr-Commit-Position: refs/heads/master@{#381918}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Remove ValuePassInType. #
Depends on Patchset: Messages
Total messages: 29 (13 generated)
Description was changed from
==========
WTF: Implement move semantics for values of {List,Linked}HashSet.
This is similar to two earlier patches that implemented move semantics
in HashMap and HashSet:
https://codereview.chromium.org/1764973002/
https://codereview.chromium.org/1798913002/
This patch is for ListHashSet and LinkedHashSet. The changes in this
patch are mostly the same as the previous patches: taking an incoming
value as a universal reference (T&&), and forwarding it as appropriate.
BUG=567139, 582349
==========
to
==========
WTF: Implement move semantics for values of {List,Linked}HashSet.
This is similar to two earlier patches that implemented move semantics
in HashMap and HashSet:
https://codereview.chromium.org/1764973002/
https://codereview.chromium.org/1798913002/
This patch is for ListHashSet and LinkedHashSet. The changes in this
patch are mostly the same as the previous patches: taking an incoming
value as a universal reference (T&&), and forwarding it as appropriate.
BUG=567139, 582349
==========
yutak@chromium.org changed reviewers: + haraken@chromium.org, mikhail.pozdnyakov@intel.com, tzik@chromium.org
yutak@chromium.org changed required reviewers: + haraken@chromium.org, tzik@chromium.org
PTAL
https://codereview.chromium.org/1807153002/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/wtf/ListHashSet.h (left): https://codereview.chromium.org/1807153002/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/wtf/ListHashSet.h:147: AddResult add(ValuePassInType); if 'ValuePassInType' is not used any more, maybe it should not be declared? https://codereview.chromium.org/1807153002/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/wtf/ListHashSet.h (right): https://codereview.chromium.org/1807153002/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/wtf/ListHashSet.h:148: AddResult add(IncomingValueType&&); Does this converting to template methods (here and in the other places in the patch) affect binary size?
lgtm
LGTM
The CQ bit was checked by yutak@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/1807153002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1807153002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1807153002/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/wtf/ListHashSet.h (left): https://codereview.chromium.org/1807153002/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/wtf/ListHashSet.h:147: AddResult add(ValuePassInType); On 2016/03/17 09:17:48, Mikhail wrote: > if 'ValuePassInType' is not used any more, maybe it should not be declared? Done. https://codereview.chromium.org/1807153002/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/wtf/ListHashSet.h (right): https://codereview.chromium.org/1807153002/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/wtf/ListHashSet.h:148: AddResult add(IncomingValueType&&); On 2016/03/17 09:17:48, Mikhail wrote: > Does this converting to template methods (here and in the other places in the > patch) affect binary size? I did this kind of transmutation many times in the earlier patches and I haven't received any complaints so far. In general compilers are good at inlining this kind of stuff.
The CQ bit was checked by yutak@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/1807153002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1807153002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by yutak@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/1807153002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1807153002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Mikhail: I'm landing this now with the given approvals so far, but feel free to leave comments if you find any more issues.
The CQ bit was checked by yutak@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org, tzik@chromium.org Link to the patchset: https://codereview.chromium.org/1807153002/#ps20001 (title: "Remove ValuePassInType.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1807153002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1807153002/20001
lgtm
Message was sent while issue was closed.
Description was changed from
==========
WTF: Implement move semantics for values of {List,Linked}HashSet.
This is similar to two earlier patches that implemented move semantics
in HashMap and HashSet:
https://codereview.chromium.org/1764973002/
https://codereview.chromium.org/1798913002/
This patch is for ListHashSet and LinkedHashSet. The changes in this
patch are mostly the same as the previous patches: taking an incoming
value as a universal reference (T&&), and forwarding it as appropriate.
BUG=567139, 582349
==========
to
==========
WTF: Implement move semantics for values of {List,Linked}HashSet.
This is similar to two earlier patches that implemented move semantics
in HashMap and HashSet:
https://codereview.chromium.org/1764973002/
https://codereview.chromium.org/1798913002/
This patch is for ListHashSet and LinkedHashSet. The changes in this
patch are mostly the same as the previous patches: taking an incoming
value as a universal reference (T&&), and forwarding it as appropriate.
BUG=567139, 582349
==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from
==========
WTF: Implement move semantics for values of {List,Linked}HashSet.
This is similar to two earlier patches that implemented move semantics
in HashMap and HashSet:
https://codereview.chromium.org/1764973002/
https://codereview.chromium.org/1798913002/
This patch is for ListHashSet and LinkedHashSet. The changes in this
patch are mostly the same as the previous patches: taking an incoming
value as a universal reference (T&&), and forwarding it as appropriate.
BUG=567139, 582349
==========
to
==========
WTF: Implement move semantics for values of {List,Linked}HashSet.
This is similar to two earlier patches that implemented move semantics
in HashMap and HashSet:
https://codereview.chromium.org/1764973002/
https://codereview.chromium.org/1798913002/
This patch is for ListHashSet and LinkedHashSet. The changes in this
patch are mostly the same as the previous patches: taking an incoming
value as a universal reference (T&&), and forwarding it as appropriate.
BUG=567139, 582349
Committed: https://crrev.com/b1d3886202efe86f89e711a3b61ab250fadffbd2
Cr-Commit-Position: refs/heads/master@{#381918}
==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b1d3886202efe86f89e711a3b61ab250fadffbd2 Cr-Commit-Position: refs/heads/master@{#381918} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
