Handle conversion rules totally break static typing

RESOLVED FIXED in mozilla14

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: luke, Assigned: bhackett)

Tracking

unspecified
mozilla14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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?
(Assignee)

Comment 1

5 years ago
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.
Assignee: general → bhackett1024
Attachment #615047 - Flags: review?(luke)
(Reporter)

Comment 2

5 years ago
Comment on attachment 615047 [details] [diff] [review]
patch

Ahh, I didn't see testAssign.  Cool.
Attachment #615047 - Flags: review?(luke) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4597dcf0842a

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/4597dcf0842a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.