1103152 - Remove some JSClass stub functions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

I want to make all JSClass function pointer members nullable. This is the first step. I'll make these nullable:

    addProperty
    delProperty
    enumerate
    resolve
    convert

and I'll remove the corresponding stub functions JS_DeletePropertyStub, JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub. (addProperty uses JS_PropertyStub which can't be removed yet.)

After this, only getProperty and setProperty will remain.

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: Remove JS_DeletePropertyStub, JS_EnumerateStub, JS_ResolveStub, and JS_ConvertStub. Make five mandatory JSClass hooks optional (nullable)

Comment 2

[Jason Orendorff [:jorendorff]]

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

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 8527106 [details] [diff] [review]
Remove JS_DeletePropertyStub, JS_EnumerateStub, JS_ResolveStub, and JS_ConvertStub. Make five mandatory JSClass hooks optional (nullable)

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

Shifting review. I really want to land this and the patches in bug 1103368 as soon as I can. They're going to bitrot like crazy.

Comment 4

[Brian Hackett [Laid off!]]

Comment on attachment 8527106 [details] [diff] [review]
Remove JS_DeletePropertyStub, JS_EnumerateStub, JS_ResolveStub, and JS_ConvertStub. Make five mandatory JSClass hooks optional (nullable)

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

Nice!!!!

::: js/src/jsapi.cpp
@@ +2143,5 @@
>      return true;
>  }
>  
>  JS_PUBLIC_API(bool)
> +JS::OrdinaryToPrimitive(JSContext *cx, HandleObject obj, JSType type, MutableHandleValue vp)

OrdinaryConvert, or OrdinaryDefaultValue?

Comment 5

[Jeff Walden [:Waldo]]

(In reply to Brian Hackett (:bhackett) from comment #4)
> OrdinaryConvert, or OrdinaryDefaultValue?

The algorithm in the spec is named OrdinaryToPrimitive:

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-toprimitive (scroll down a little there)

Beyond that, this entire mechanism is going to more or less go away when we implement conversion-to-primitive using a @@toPrimitive symbol, so it's not worth polishing things (like renaming the field from convert to toPrimitive or similar).

Comment 6

[Jason Orendorff [:jorendorff]]

(In reply to Brian Hackett (:bhackett) from comment #4)
> > +JS::OrdinaryToPrimitive(JSContext *cx, HandleObject obj, JSType type, MutableHandleValue vp)
> 
> OrdinaryConvert, or OrdinaryDefaultValue?

Waldo's right, the name just follows the spec.

I admit, I did think "this name sucks" as I typed it in.

Added a comment to help explain:

    /*
     * Implements ES6 draft rev 28 (2014 Oct 14) 7.1.1, second algorithm.
     *
     * Most users should not call this -- use JS::ToNumber, ToBoolean, or ToString
     * instead. This should only be called from custom convert hooks. It implements
     * the default conversion behavior shared by most objects in JS, so it's useful
     * as a fallback.
     */
    extern JS_PUBLIC_API(bool)
    OrdinaryToPrimitive(JSContext *cx, HandleObject obj, JSType type, MutableHandleValue vp);

Comment 7

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b3f742c00ba1

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

b2g build bustage https://treeherder.mozilla.org/ui/logviewer.html#?job_id=4173512&repo=mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/63f71ff39bbc

Comment 9

[Jason Orendorff [:jorendorff]]

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

Comment 10

[Jason Orendorff [:jorendorff]]

14:20:40     INFO -  {standard input}: Assembler messages:
14:20:40     INFO -  {standard input}:49744: Error: can't resolve `_ZN2js16TypedArrayObject7classesE' {*UND* section} - `.LPIC442' {*UND* section}
14:20:40    ERROR -  make[6]: *** [Unified_cpp_js_src8.o] Error 1
14:20:40     INFO -  make[6]: *** Waiting for unfinished jobs....

**Assembler**?!

No clue what's going on here.

Comment 11

[Jason Orendorff [:jorendorff]]

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

Comment 12

[Jason Orendorff [:jorendorff]]

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

Comment 13

[Jason Orendorff [:jorendorff]]

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

Comment 14

[Jason Orendorff [:jorendorff]]

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

Comment 15

[Jason Orendorff [:jorendorff]]

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

Comment 16

[Jason Orendorff [:jorendorff]]

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

Comment 17

[Jason Orendorff [:jorendorff]]

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

Comment 18

[Jason Orendorff [:jorendorff]]

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

Comment 19

[Jason Orendorff [:jorendorff]]

Try-bisecting continues apace. The good news is, it looks like 3 of the 5 hooks can be nulled out without angering the fickle Ice Cream Sandwich gods. I'm guessing 4 would be fine. Still no idea what's really causing this.

Comment 20

[Jason Orendorff [:jorendorff]]

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

Comment 21

[Jason Orendorff [:jorendorff]]

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

Comment 22

[Jason Orendorff [:jorendorff]]

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

Comment 23

[Jason Orendorff [:jorendorff]]

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

Comment 24

[Jason Orendorff [:jorendorff]]

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

Comment 25

[Jason Orendorff [:jorendorff]]

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

Comment 26

[Jason Orendorff [:jorendorff]]

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

Comment 27

[Jason Orendorff [:jorendorff]]

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

Comment 28

[Jason Orendorff [:jorendorff]]

Re: comment 27, I gave up and #ifdef'd some code just for GCC 4.4.

If that works, let's land it and move on...

Comment 29

[Jason Orendorff [:jorendorff]]

Attachment: This bizarre little patch causes to compile successfully in GCC 4.4 for ICS, working around a compiler bug

Comment 30

[Jason Orendorff [:jorendorff]]

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

Comment 31

[Jason Orendorff [:jorendorff]]

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

Comment 32

[Jason Orendorff [:jorendorff]]

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

Comment 33

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a7d403088a16

Comment 34

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a7d403088a16