The scripted version would not take a native but would take a parent, for now. The native version would take no funobj and no parent.

Comment on attachment 8575013 [details] [diff] [review] part 2. Stop passing non-null funobjArg to js::NewFunction and js::NewFunctionWithProto Review of attachment 8575013 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsfun.cpp @@ +777,5 @@ > static JSObject * > CreateFunctionConstructor(JSContext *cx, JSProtoKey key) > { > Rooted<GlobalObject*> self(cx, cx->global()); > + RootedObject functionProto(cx, &cx->global()->getPrototype(JSProto_Function).toObject()); Mild preference to s/self/global/ and use that here if you're changing stuff in this area.

Comment on attachment 8575016 [details] [diff] [review] part 5. Split up js::NewFunction into several different APIs that are more clear in terms of what they do and don't need parents as much Review of attachment 8575016 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsapi.cpp @@ +3148,5 @@ > } > > + return (flags & JSFUN_CONSTRUCTOR) ? > + NewNativeConstructor(cx, native, nargs, atom) : > + NewNativeFunction(cx, native, nargs, atom); return (flags & JSFUN_CONSTRUCTOR) ? NewNativeConstructor(...) : NewNativeFunction(...); @@ +3164,5 @@ > > RootedAtom name(cx, JSID_TO_ATOM(id)); > + return (flags & JSFUN_CONSTRUCTOR) ? > + NewNativeConstructor(cx, native, nargs, name) : > + NewNativeFunction(cx, native, nargs, name); Same styling. ::: js/src/jsfun.cpp @@ +1706,5 @@ > if (!fun->initBoundFunction(cx, target, thisArg, boundArgs, argslen)) > return nullptr; > > + // Steps 9-10. Set length again, because NewNativeFunction/NewNativeConstructor > + // sometimes truncates. /me eyes that comment beadily @@ +1997,5 @@ > > JSFunction * > +js::NewNativeFunction(ExclusiveContext *cx, Native native, unsigned nargs, HandleAtom atom, > + gc::AllocKind allocKind /* = JSFunction::FinalizeKind */, > + NewObjectKind newKind /* = GenericObject */) If newKind is unused here, you should remove it from the signature and all callers, right? @@ +2009,5 @@ > + gc::AllocKind allocKind /* = JSFunction::FinalizeKind */, > + NewObjectKind newKind /* = GenericObject */, > + JSFunction::Flags flags /* = JSFunction::NATIVE_CTOR */) > +{ > + MOZ_ASSERT(flags & JSFunction::NATIVE_CTOR); I might (separate patch) make this separateFlags and not require NATIVE_CTOR be passed by callers (indeed, *require* that it not be passed). Separate patch. ::: js/src/jsfun.h @@ +514,5 @@ > + gc::AllocKind allocKind = JSFunction::FinalizeKind, > + NewObjectKind newKind = GenericObject, > + JSFunction::Flags flags = JSFunction::NATIVE_CTOR); > + > +// Allocate a new scripted function backed by a JSNative. s/ backed by a JSNative// ::: js/src/vm/SharedArrayObject.cpp @@ +345,5 @@ > > RootedId byteLengthId(cx, NameToId(cx->names().byteLength)); > unsigned attrs = JSPROP_SHARED | JSPROP_GETTER | JSPROP_PERMANENT; > + JSObject *getter = > + NewNativeFunction(cx, SharedArrayBufferObject::byteLengthGetter, 0, NullPtr()); I'm not a huge fan of these NullPtr() everywhere with no obvious meaning. :-\ Maybe I'll get used to it when there's no parent and it can only be one thing.

> Mild preference to s/self/global/ and use that here if you're changing stuff in this area. Done. > /me eyes that comment beadily I just moved it! But yes, it sounds bogosityfull. > If newKind is unused here, you should remove it from the signature and all callers, > right? Ouch. Good catch. It should be passed to NewFunctionWithProto. It _might_ be unused if it's never passed to js::DefineFunction when passing a Native, but I'd rather not assume that for the moment. > I might (separate patch) make this separateFlags I'm not sure it's worth the bother. The only callers who pass something weird here are asm.js, basically. > s/ backed by a JSNative// Er, yes. ;) > Maybe I'll get used to it when there's no parent and it can only be one thing. Yeah...

https://hg.mozilla.org/integration/mozilla-inbound/rev/b60ccf33795d https://hg.mozilla.org/integration/mozilla-inbound/rev/e2d7fdba0593 https://hg.mozilla.org/integration/mozilla-inbound/rev/2d8eb9a843bf https://hg.mozilla.org/integration/mozilla-inbound/rev/56e47c184dcc https://hg.mozilla.org/integration/mozilla-inbound/rev/bd142e2ac19c

https://hg.mozilla.org/mozilla-central/rev/b60ccf33795d https://hg.mozilla.org/mozilla-central/rev/e2d7fdba0593 https://hg.mozilla.org/mozilla-central/rev/2d8eb9a843bf https://hg.mozilla.org/mozilla-central/rev/56e47c184dcc https://hg.mozilla.org/mozilla-central/rev/bd142e2ac19c