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)

defect

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.
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: general → nobody
Severity: normal → S3
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.