Remove JSFunction::u::n::clasp

RESOLVED FIXED in mozilla14

Status

()

Core
JavaScript Engine
--
minor
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

Trunk
mozilla14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Created attachment 608142 [details] [diff] [review]
Patch

The object class pointer in native functions is barely used, save in one JSAPI method (JS_NewObjectForConstructor) and in calling the construct hook on a function proxy.  It doesn't really serve a semantic purpose -- or, to the extent it does, it does so confusingly, because almost nothing actually cares what value's stored there.  It would be better to remove it to reduce complexity.

In the JSAPI method case, we can add the desired class to the method signature pretty easily -- and from js-engine list discussion, this shouldn't be too big a problem:

https://groups.google.com/forum/?fromgroups#!topic/mozilla.dev.tech.js-engine/kUEuKi5dVQY

In the proxy case, there are two considerations.  First, under the old proxy proposal, I think we actually were supposed to be passing |undefined| as |this| in that case; that we weren't doing this was just a bug.  Second, it's the *old* proxy proposal.  At some point we'll implement the new one, so what we do to our implementation of the old proposal doesn't much matter.

Anyway.  Less complexity is good.  Let's kill some.
Attachment #608142 - Flags: review?(dmandelin)
Created attachment 608143 [details] [diff] [review]
Followup: remove the Class* argument to GlobalObject::createConstructor

As a followup, once the class field's removed, we can remove an argument from the method that creates the built-in constructor functions.
Attachment #608143 - Flags: review?(dmandelin)
Comment on attachment 608142 [details] [diff] [review]
Patch

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

js_CreateThis does look better this way.
Attachment #608142 - Flags: review?(dmandelin) → review+
Attachment #608143 - Flags: review?(dmandelin) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e89811e547a2
https://hg.mozilla.org/integration/mozilla-inbound/rev/500f3088583f
Target Milestone: --- → mozilla14

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/e89811e547a2
https://hg.mozilla.org/mozilla-central/rev/500f3088583f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.