Closed Bug 1710893 Opened 5 years ago Closed 4 years ago

Revisit allocation strategy for unified intl components.

Categories

(Core :: Internationalization, enhancement)

enhancement

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: dminor, Unassigned)

References

Details

(Whiteboard: [i18n-unification])

In Bug 1701695, I'm adding a NumberFormat::TryCreate that returns a Result<UniquePtr, Err> that allows for an error to be signaled if ICU initialization fails. My initial implementation [1] allowed for the allocator and DeletePolicy for the UniquePtr to be supplied as optional arguments. This would allow the use of SpiderMonkey allocator and for fallible OOM handling. Anba proposed something along the same lines in [2].

We ended up going with new/delete and adding a suppression to the lint check in check_vanilla_allocations.py, on the rationale that this is similar to what code in mozglue is doing.

This bug is a chance to revisit that decision and see if we want to allow caller control over the UniquePtr for SpiderMonkey embedding.

Nordzilla suggested another option which would be to switch to a fallible allocator in NumberFormat::TryCreate and return an OOM error if that failed.

As far as I can tell, OOM in ICU4C or eventually ICU4x will still cause a crash, so I'm not certain if making the NumberFormat allocation fallible gains us much.

[1] https://phabricator.services.mozilla.com/D113793
[2] https://phabricator.services.mozilla.com/D114593#inline-638542

Whiteboard: [i18n-unification]

Do we still need to consider this? Or can we close it now.

Flags: needinfo?(dminor)

I'm ok with closing it. I think most of the allocations are going to happen in ICU4C or ICU4X anyway, so I don't think there's a good reason to worry about this here.

Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(dminor)
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.