Last Comment Bug 745361 - Handle conversion rules totally break static typing
: Handle conversion rules totally break static typing
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Brian Hackett (:bhackett)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-13 15:20 PDT by Luke Wagner [:luke]
Modified: 2012-04-16 18:59 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (642 bytes, patch)
2012-04-14 07:32 PDT, Brian Hackett (:bhackett)
luke: review+
Details | Diff | Review

Description Luke Wagner [:luke] 2012-04-13 15:20:53 PDT
This type checks but I don't think it should

  RootedVarString r(...);
  HandleObject h = r;

I think the solution is to remove this constructor and fix other things with explicit (restrict) casts.

  template <class T>
  template <class U>
  Handle<T>::Handle(const RootedVar<U> &);

and the analogous one for Root<U>.

Also, can we move Handle out of jsprvtd.h?
Comment 1 Brian Hackett (:bhackett) 2012-04-14 07:32:07 PDT
Created attachment 615047 [details] [diff] [review]
patch

Oops, this constructor was missing a testAssign, which is supposed to only allow handle assignments that follow normal C++ assignment rules for the wrapped type.  See the other Handle constructors, which are also templated on a type other than the handle.  Requiring casts whenever converting between handles of different but compatible types would be a mess I think, with all the subclassing we do for objects and strings.
Comment 2 Luke Wagner [:luke] 2012-04-15 21:08:29 PDT
Comment on attachment 615047 [details] [diff] [review]
patch

Ahh, I didn't see testAssign.  Cool.
Comment 3 Brian Hackett (:bhackett) 2012-04-15 21:36:43 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/4597dcf0842a
Comment 4 :Ehsan Akhgari (busy, don't ask for review please) 2012-04-16 08:35:03 PDT
https://hg.mozilla.org/mozilla-central/rev/4597dcf0842a

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