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+
Target Milestone: --- → mozilla14
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.