Closed Bug 770759 Opened 8 years ago Closed 8 years ago

Add mutable handles

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: billm, Assigned: billm)

Details

Attachments

(2 files)

Attached patch patchSplinter Review
This is useful for outparams. This patch doesn't change all the possible places that could use mutables handles, but I think it fixes some of the most common cases.
Attachment #638925 - Flags: review?(bhackett1024)
Comment on attachment 638925 [details] [diff] [review]
patch

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

::: js/src/gc/Root.h
@@ +145,5 @@
> +                  typename mozilla::EnableIf<mozilla::IsConvertible<S, T>::value, int>::Type dummy = 0);
> +
> +    void set(T v) { *ptr = v; }
> +
> +    const T *address() const { return ptr; }

Can this return T* rather than const T*?
Attachment #638925 - Flags: review?(bhackett1024) → review+
Looks like I forgot a bunch of JSCLASS_NEW_RESOLVE resolve hooks. It's annoying that their type gets cast away when they're put in the class. I wonder why we didn't just use a separate field.
Attachment #639153 - Flags: review?(bhackett1024)
Comment on attachment 639153 [details] [diff] [review]
patch for resolve hooks

Yeah, I had to grep for JSCLASS_NEW_RESOLVE when making earlier changes to the signatures.  If there were two fields we'd have a good way to incorrectly check for the presence of resolve hooks on a class.  I wonder if there's any value in keeping the old resolve hook format around, given the breakage the handles are already making to the old API.
Attachment #639153 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/13897ce0f3a2
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in before you can comment on or make changes to this bug.