Last Comment Bug 770759 - Add mutable handles
: Add mutable handles
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Bill McCloskey (:billm)
: Jason Orendorff [:jorendorff]
Depends on:
  Show dependency treegraph
Reported: 2012-07-03 17:41 PDT by Bill McCloskey (:billm)
Modified: 2012-07-04 18:19 PDT (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (129.91 KB, patch)
2012-07-03 17:41 PDT, Bill McCloskey (:billm)
bhackett1024: review+
Details | Diff | Splinter Review
patch for resolve hooks (31.21 KB, patch)
2012-07-04 11:55 PDT, Bill McCloskey (:billm)
bhackett1024: review+
Details | Diff | Splinter Review

Description Bill McCloskey (:billm) 2012-07-03 17:41:00 PDT
Created attachment 638925 [details] [diff] [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.
Comment 1 Brian Hackett (:bhackett) 2012-07-04 08:06:10 PDT
Comment on attachment 638925 [details] [diff] [review]

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*?
Comment 2 Bill McCloskey (:billm) 2012-07-04 11:55:26 PDT
Created attachment 639153 [details] [diff] [review]
patch for resolve hooks

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.
Comment 3 Brian Hackett (:bhackett) 2012-07-04 12:12:49 PDT
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.
Comment 4 Bill McCloskey (:billm) 2012-07-04 18:19:53 PDT

Note You need to log in before you can comment on or make changes to this bug.