Closed Bug 738075 Opened 9 years ago Closed 9 years ago

Remove JSFunction::u::n::clasp

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: Waldo, Assigned: Waldo)

Details

Attachments

(2 files)

Attached patch PatchSplinter Review
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)
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+
You need to log in before you can comment on or make changes to this bug.