Closed
Bug 987243
Opened 11 years ago
Closed 11 years ago
Don't use .call(...) in self-hosted code
Categories
(Core :: JavaScript: Standard Library, defect)
Core
JavaScript: Standard Library
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: Waldo, Assigned: Waldo)
References
Details
Attachments
(1 file)
(deleted),
patch
|
till
:
review+
|
Details | Diff | Splinter Review |
Using .call(...) in self-hosted code is either a bug, because .call can be overwritten, or it's not optimal for perf, because there's a callprop/etc. of overhead to eat. (At least, assuming the spec doesn't directly call for "get the 'call' property and then call it with callee and arguments...", which it never has yet.) Any use should instead be callFunction.
I have no idea why I can't create a testcase that fails because of this, but I can't. I tried stepping through in a debugger, and std_Map_has and such were *not* the same as the ones in the actual script. (Maybe the ones in the SHG?) Apparently nobody understands this wrinkle of self-hosting code, because the people I pointed it out to either thought it was a bug, or agreed with me that it was a bug when I explained why I thought it was. Wonderful. Let's just fix this and not hugely overthink things, I guess.
Attachment #8395778 -
Flags: review?(till)
Comment 1•11 years ago
|
||
Comment on attachment 8395778 [details] [diff] [review]
Patch
Review of attachment 8395778 [details] [diff] [review]:
-----------------------------------------------------------------
r=me
Attachment #8395778 -
Flags: review?(till) → review+
Comment 2•11 years ago
|
||
Also, as a note to evilpie and sankha, callFunction and why it exists is described here:
https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/self-hosting
Implementers and reviewers working on self-hosting code should probably study that page carefully. If in doubt, r? me.
Assignee | ||
Comment 3•11 years ago
|
||
With some extra tests for issues as demonstrated by till -- need to overwrite Function.prototype.call to expose issues, overwriting .call on std_*_* isn't a workable demo:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5d272a61907
Comment 4•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•