Support using mozilla::Variant inside Rooted

RESOLVED FIXED in Firefox 48

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: shu, Unassigned)

Tracking

unspecified
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Would be nice.
Attachment #8727737 - Flags: review?(terrence)
Attachment #8727737 - Attachment is obsolete: true
Attachment #8727737 - Flags: review?(terrence)
Attachment #8727742 - Flags: review?(terrence)
Attachment #8727742 - Flags: feedback?(nfitzgerald)
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.
Attachment #8727742 - Flags: feedback?(nfitzgerald) → feedback+
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.
Attachment #8727742 - Flags: review?(terrence) → review+
Blocks: 1254893
Comments addressed, carrying r=terrence.
Attachment #8727742 - Attachment is obsolete: true
Attachment #8728309 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/92e23bc3f31d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.