959013 - Add a mechanism to describe the behavior of a standard class with the JSClass, and test-drive it with Date

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bobby Holley (:bholley)]

This is one of the major pieces of XrayToJS, as suggested by jorendorff in bug 914970 comment 1.

This is my plan:

(1) Add an optional js::Class member called |specs| (feel free to suggest another name). This will contain:

{
    JSFunctionSpec instanceFunctions,
    JSPropertySpec instanceProperties,
    JSFunctionSpec prototypeFunctions,
    JSPropertySpec prototypeProperties,
    JSFunctionSpec interfaceFunctions,
    JSPropertySpec interfaceProperties
}

The first set of two are defined as |own| props on instances of the class. The second on the prototype, the third on the class object (constructor).

(2) Rejigger the js_InitFooClass machinery such that it calls a common helper routine first, which defines the {prototype,interface}{Properties,Functions} before calling any custom bootstrapping code. We could also hoist the js_InitFooClass pointers directly into the Class while we're at it, though that might be a lot of work.

(3) Rejigger object creation to define the |own| props.

(4) Start hoisting hand-rolled code from js_InitFooClass into the JSClass, and letting the generic machinery take care of it. The entire conversion process if a mammoth task, and will not be limited to this bug.

Thoughts?

Comment 1

[Bobby Holley (:bholley)]

Attachment: Strawman - Create storage for JS{Property,Function}Specs on js::Class. v1

Here's a brief prototype of the js::Class changes, so that we can bikeshed the
naming and whatnot before I build too much on top of this. I ended up calling
it js::ClassSpec, and calling the member |spec|.

I considered rolling this into ClassExtension, but decided that it was probably
cleaner to keep it separate. Otherwise, we'd eventually be declaring all the
junk in ClassExtension in every builtin type's Class.

Comment 2

[Bobby Holley (:bholley)]

Comment on attachment 8359021 [details] [diff] [review]
Strawman - Create storage for JS{Property,Function}Specs on js::Class. v1

My approach has evolved a bit. This is obsolete enough that it's probably not worth looking at for now.

Comment 3

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=4d11b9f4e840

Comment 4

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=4cdf8e7dc61a

Comment 5

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=196eb1211252

Comment 6

[Bobby Holley (:bholley)]

This is green.

Comment 7

[Bobby Holley (:bholley)]

Attachment: Part 1 - Create storage for declarative class bootstrapping information on js::Class. v1

Comment 8

[Bobby Holley (:bholley)]

Attachment: Part 2 - Add a mechanism to map from JSProtoKey to js::Class*. v1

Comment 9

[Bobby Holley (:bholley)]

Attachment: Part 3 - Add a standardized initialization mechanism based on the ClassSpec. v1

Comment 10

[Bobby Holley (:bholley)]

Attachment: Part 4 - Convert the Date object to use ClassSpec. v1

Comment 11

[Luke Wagner [:luke]]

Comment on attachment 8364729 [details] [diff] [review]
Part 3 - Add a standardized initialization mechanism based on the ClassSpec. v1

Review of attachment 8364729 [details] [diff] [review]:
-----------------------------------------------------------------

Beautious

::: js/src/vm/GlobalObject.cpp
@@ +457,4 @@
>      ClassInitializerOp init = protoTable[key].init;
> +    if (init == js_InitViaClassSpec)
> +        init = nullptr;
> +    const Class *clasp = ProtoKeyToClass(key);

Could you add a \n before clasp declaration?

Comment 12

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=26d54adda7ab

Comment 13

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=afae898a7fa0

Comment 14

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=79ccfa417430

Comment 15

[Bobby Holley (:bholley)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/91edc89dc13a
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4f19286d5b9f
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/48762db06b68
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/18c901d3e4c5

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/91edc89dc13a
https://hg.mozilla.org/mozilla-central/rev/4f19286d5b9f
https://hg.mozilla.org/mozilla-central/rev/48762db06b68
https://hg.mozilla.org/mozilla-central/rev/18c901d3e4c5