580337 - avoid bugs in fast-natives with more useful types

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

(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.

Comment 1

[Luke Wagner [:luke]]

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.