Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2739813003:
Allow WTF::Vector::emplace_back accept one or zero arguments (Closed)
DescriptionAllow WTF::Vector::emplace_back accept one or zero arguments
std::vector has no such limitations, so let's drop them either.
Vector unit test is updated accordingly.
BUG=698998
Review-Url: https://codereview.chromium.org/2739813003
Cr-Commit-Position: refs/heads/master@{#456045}
Committed: https://chromium.googlesource.com/chromium/src/+/3e1c8c2a1cde9d01f965ce41f71e7ca959740fae
Patch Set 1 #Patch Set 2 : Added emplace_back() overload #
Messages
Total messages: 24 (16 generated)
The CQ bit was checked by mikhail.pozdnyakov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Allow WTF::Vector::emplace_back accept one or zero arguments std::vector has no such limitations, so let's drop them either. Vector unit test is updated accordingly. BUG=698998 ========== to ========== Allow WTF::Vector::emplace_back accept one or zero arguments std::vector has no such limitations, so let's drop them either. Vector unit test is updated accordingly. BUG=698998 ==========
mikhail.pozdnyakov@intel.com changed reviewers: + yutak@chromium.org
PTAL
yutak@chromium.org changed reviewers: + pilgrim@chromium.org
+pilgrim I'm not sure if we want to this... pilgrim, WDYT? At least, if the sizeof...(Args) is zero, we want it to be equivalent to grow(1); you should use TypeOperations::initialize() instead of placement new.
On 2017/03/09 11:07:03, Yuta Kitamura wrote:
> +pilgrim
>
> I'm not sure if we want to this... pilgrim, WDYT?
>
> At least, if the sizeof...(Args) is zero, we want it to be
> equivalent to grow(1); you should use
> TypeOperations::initialize() instead of placement new.
Good point, thanks! We could achieve this and keep the 'emplace_back()'
semantics adding an overload:
ALWAYS_INLINE T& emplace_back() { grow(1); return back(); }
On 2017/03/09 at 11:07:03, yutak wrote: > +pilgrim > > I'm not sure if we want to this... pilgrim, WDYT? > I have no opinion. +haraken
pilgrim@chromium.org changed reviewers: + haraken@chromium.org - pilgrim@chromium.org
CL-wise, this LGTM if grow(1) case is addressed. Defer the final decision to haraken@.
On 2017/03/10 08:11:49, Yuta Kitamura wrote: > CL-wise, this LGTM if grow(1) case is addressed. > > Defer the final decision to haraken@. LGTM with the grow(1) case addressed.
The CQ bit was checked by mikhail.pozdnyakov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mikhail.pozdnyakov@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org, yutak@chromium.org Link to the patchset: https://codereview.chromium.org/2739813003/#ps20001 (title: "Added emplace_back() overload")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489148289617270,
"parent_rev": "ab369a14712059bfbdca40ccff278d6b7c8ec7dc", "commit_rev":
"3e1c8c2a1cde9d01f965ce41f71e7ca959740fae"}
Message was sent while issue was closed.
Description was changed from ========== Allow WTF::Vector::emplace_back accept one or zero arguments std::vector has no such limitations, so let's drop them either. Vector unit test is updated accordingly. BUG=698998 ========== to ========== Allow WTF::Vector::emplace_back accept one or zero arguments std::vector has no such limitations, so let's drop them either. Vector unit test is updated accordingly. BUG=698998 Review-Url: https://codereview.chromium.org/2739813003 Cr-Commit-Position: refs/heads/master@{#456045} Committed: https://chromium.googlesource.com/chromium/src/+/3e1c8c2a1cde9d01f965ce41f71e... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/3e1c8c2a1cde9d01f965ce41f71e... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
