add a MakeUniqueFallible<T[]> (and maybe <T>) function

RESOLVED FIXED in Firefox 44, Firefox OS v2.5

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: froydnj, Unassigned)

Tracking

43 Branch
mozilla45
Points:
---

Firefox Tracking Flags

(firefox44 fixed, firefox45 fixed, b2g-v2.5 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
In converting a few nsAuto*Ptr things to UniquePtr, one pattern that comes up is:

  mAutoPtr = new (fallible) T[...];

which has to be converted like so:

  mUniquePtr.reset(new (fallible) T[...]);

I think converting it like so would be nicer:

  mUniquePtr = MakeUniqueFallible<T[]>(...);

which is a little longer, but reads a little more idiomatically and doesn't use |new|.

Comment 1

3 years ago
Reasonable.  Separate header from the existing, is all I want here.  :-)  If we adopt <memory> at some point, want to keep extensions separated out.
How about mUniquePtr = MakeUnique<T[]>(..., fallible) ?

Comment 3

3 years ago
Don't think so.  It seems very worth preserving the sense that the arguments passed in, are exactly passed along.  Not to mention there'd then be ambiguity with a constructor that took a |fallible| as its last argument.
(Reporter)

Comment 4

3 years ago
Created attachment 8682614 [details] [diff] [review]
add mozilla::MakeUniqueFallible and convert uses throughout the tree

Chose to call the file MakeUniqueExtensions in case we ever want to have
UniqueFreePtr etc.
Attachment #8682614 - Flags: review?(jwalden+bmo)

Updated

3 years ago
Attachment #8682614 - Flags: review?(jwalden+bmo) → review+

Comment 6

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9bfc35f6d7ab
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Created attachment 8703908 [details] [diff] [review]
header only uplift for beta

Approval Request Comment
[Feature/regressing bug #]: none
[User impact if declined]: need this to uplift bug 1235605
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: safe
[String/UUID change made/needed]: none
Attachment #8703908 - Flags: feedback?(nfroyd)
Attachment #8703908 - Flags: approval-mozilla-beta?
(Reporter)

Comment 8

3 years ago
Comment on attachment 8703908 [details] [diff] [review]
header only uplift for beta

Review of attachment 8703908 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!
Attachment #8703908 - Flags: feedback?(nfroyd) → feedback+

Comment 9

3 years ago
Comment on attachment 8703908 [details] [diff] [review]
header only uplift for beta

Need to fix a sec-mod issue. Beta44+
Attachment #8703908 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Updated

3 years ago
status-firefox44: --- → affected

Comment 10

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/3b68f716c82b
status-firefox44: affected → fixed
You need to log in before you can comment on or make changes to this bug.