1254453 - Support using mozilla::Variant inside Rooted

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Shu-yu Guo [:shu]]

Would be nice.

Comment 1

[Shu-yu Guo [:shu]]

Attachment: Allow using Variant with Rooted.

Comment 2

[Shu-yu Guo [:shu]]

Attachment: Allow using Variant with Rooted.

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8727742 [details] [diff] [review]
Allow using Variant with Rooted.

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

That was a doozy, but this is going to be really nice. May be worth having a sanity jsapi test that we can trace and match on a GCVariant<T, Ts...>.

An aside about matching that totally already exists before this patch: I think we can use the perfect forwarding style for matchers instead of having a const and non-const version, and also throw some && in there so that we can do v.match(SomeMatcher()) instead of declaring the matcher above the match call. I will file a bug to investigate this.

Comment 4

[Terrence Cole [:terrence]]

Comment on attachment 8727742 [details] [diff] [review]
Allow using Variant with Rooted.

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

Nice!

::: js/public/GCVariant.h
@@ +114,5 @@
> +    }
> +};
> +
> +template <typename... Ts>
> +struct RootedBase<mozilla::Variant<Ts...>>

This is only going to allow easy use of a Rooted<Variant>. Allowing the same methods on a Handle<Variant>, MutableHandle<Variant>, and Heap<Variant> is actually not substantially more code. What we do is implement: |template <typename Outer> class FooOperations { ...const ops...};| and |template <typename Outer> class MutableFooOperations : public FooOperations { ...non-const ops...};|. Then we mix those into RootedBase, HandleBase, MutableHandleBase and HeapBase and pass the Rooted<T> as the Outer so that we can query the underlying values, since all of these classes support get(). Please see js/public/GCHashTable.h and js/public/GCVector.h for canonical examples.

Comment 5

[Shu-yu Guo [:shu]]

Attachment: Support using Variant inside GC wrappers.

Comments addressed, carrying r=terrence.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/92e23bc3f31d

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/92e23bc3f31d