1036710 - Use UniquePtr to manage ownership of the array of PNK_CASE nodes in table switches. NOT REVIEWED YET

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

Bug 953296 just landed UniquePtr, a smart pointer that handles single ownership of a resource, freeing it (using the custom means selected) on ultimate destruction, or handing off ownership if the smart pointer is moved.  (UniquePtr isn't copyable, so no worries about duplicate ownership.)

This patch straightforwardly uses UniquePtr to manage the array of PNK_CASE nodes tracked when emitting a JSOP_TABLESWITCH.  The nice thing is we can have the allocator method *return* a UniquePtr so that it's impossible to screw things up unless the result of get() is misused, or unless a release() isn't correctly freed.  (Of course in this case neither concern is an issue, but in general it might be.)

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Slightly better function naming

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 8453971 [details] [diff] [review]
Slightly better function naming

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

Cool.

::: js/src/frontend/BytecodeEmitter.cpp
@@ +2748,4 @@
>  
>          /*
>           * Use malloc to avoid arena bloat for programs with many switches.
> +         * Ptr takes care of freeing it on exit.

"UniquePtr"?

::: js/src/vm/MallocProvider.h
@@ +127,5 @@
> +    make_zeroed_pod_array(size_t numElems,
> +                          JSCompartment *comp = nullptr,
> +                          JSContext *cx = nullptr)
> +    {
> +        return mozilla::UniquePtr<T[], JS::FreePolicy>(pod_calloc<T>(numElems, comp, cx));

Huh. We have a template that does IsPod<T>, so it seems weird that pod_malloc and pod_calloc don't static_assert that the type being allocated really is a POD type. Feel free to add the assertion... if it compiles...

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cd9057d90944