Make PersistentRooted mozilla::Move aware

RESOLVED FIXED in Firefox 47

Status

()

Core
JavaScript: GC
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: terrence, Assigned: terrence)

Tracking

Trunk
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Created attachment 8717978 [details] [diff] [review]
make_PersistentRooted_move_aware-v0.diff

PersistentRooted with a GCVector (or other Move aware container) does not work completely. Looks like we did not completely succeed when we updated PersistentRooted for Move semantics, probably because it doesn't matter if you're only storing pointers. Now that these can store more complex types, we need to flesh out our support.

This patch Moves the GCPolicy<T>::initial calls and adds forwarding to PersistentRooted::set. This seems to work for the PersistentRooted<GCVector> case I am experimenting with.
Attachment #8717978 - Flags: review?(sphink)
Comment on attachment 8717978 [details] [diff] [review]
make_PersistentRooted_move_aware-v0.diff

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

::: js/public/RootingAPI.h
@@ +1013,5 @@
>      }
>  
>      template <typename RootingContext>
>      void init(const RootingContext& cx) {
> +        init(cx, mozilla::Move(js::GCPolicy<T>::initial()));

redundant

@@ +1024,5 @@
>      }
>  
>      void reset() {
>          if (initialized()) {
> +            set(mozilla::Move(js::GCPolicy<T>::initial()));

redundant

@@ +1052,4 @@
>          MOZ_ASSERT(initialized());
>          ptr = value;
>      }
> +    void set(T&& value) {

as discussed on IRC, r=me with this or a single definition with template <typename U> void set(U&& value) that forwards.
Attachment #8717978 - Flags: review?(sphink) → review+
(Assignee)

Comment 2

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7a08ab979fe6491284d45e4a9342076e00d4613
Bug 1247328 - Make PersistentRooted fully support Move semantics; r=sfink

Comment 3

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d7a08ab979fe
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.