Improve new nsTArray rvalue reference methods

RESOLVED FIXED in mozilla34

Status

()

Core
XPCOM
--
trivial
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

unspecified
mozilla34
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
The new rvalue reference versions of nsTArray::AppendElement and InsertElementAt want to move their parameter to the Construct function. Since this isn't a template-deduced reference parameter, they should Move rather than Forward.

It looks as if jrmuizel did originally try to template on the parameter to AppendElement but was just trying various combinations until it compiled :-\

Note that template<class Item> elem_type* AppendElement(elem_type&& aItem) matches non-const lvalues which is why you can't blindly Move them. In fact it quite happily takes const lvalues too, making the other overload redundant.
(Assignee)

Comment 1

4 years ago
My second try run (the first got hit by our infra issues) for my suggested AppendElement change seems to be reasonably green. Patch is at https://hg.mozilla.org/try/rev/9922524dc8dd
(Assignee)

Comment 2

4 years ago
Created attachment 8473357 [details] [diff] [review]
Proposed patch

https://tbpl.mozilla.org/?tree=Try&rev=34a6b9a59e6d
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #8473357 - Flags: review?(nfroyd)
Attachment #8473357 - Flags: review?(nfroyd) → review+
(Assignee)

Comment 4

4 years ago
I botched the bug# on my checkin comment. Oops.

https://hg.mozilla.org/mozilla-central/rev/2976adaf3278
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.