Closed
Bug 580337
Opened 15 years ago
Closed 2 years ago
avoid bugs in fast-natives with more useful types
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
INCOMPLETE
People
(Reporter: luke, Unassigned)
Details
(This is a post 4.0 idea I talked about with Brendan... don't really want to make another large highly-mechanical change just yet.)
One of the standard new-to-spidermonkey confusions is the widespread vp/argv/argc ritual. We could make this all much easier to understand and definitely save a few future bugs by changing the signature of fast natives to:
JSBool (*FastNative)(JSContext *cx, FastNativeArgs args);
class FastNativeArgs {
Value *vp;
uintN argc_;
public:
Value &callee() const { return vp[0]; }
Value &thisv() const { return vp[1]; }
uintN argc() const { return argc_; }
Value *argv() const { return vp + 2; }
// maybe, if needed:
Value *asValueArray() const { return vp; }
};
A few notes:
1. most ABIs mandate structs on the stack for argument passing, so this change would conceivably be a small performance ding
2. this is public JSAPI stuff so, for C embeddings to work, we may have to do some trickery in the same manner as JSClass vs. js::Class. This would actually work well because JSFastNative would use the C-friendly struct + inline functions and js::FastNative would use the C++ified version. Since this would make js::FastNativeArgs internal, we could feel free to add all the internal helpers we wanted (e.g., computeThis()), so not a bad thing.
![]() |
Reporter | |
Comment 1•14 years ago
|
||
Btw, this is half done (the easy easy half) with this data type
http://mxr.mozilla.org/mozilla-central/source/js/src/jsinterp.h#841
which is the argument to js::Invoke. The hard half is changing JSNative to take this type.
Assignee | ||
Updated•11 years ago
|
Assignee: general → nobody
Updated•2 years ago
|
Severity: normal → S3
Updated•2 years ago
|
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•