1606084 - ValueToSource should be useful without toSource methods

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tom S [:evilpie]]

We use ValueToSource and JS_ValueToSource in Gecko and SpiderMonkey code for various purposes, but mostly for error reporting and debugging. Right now ValueToSource produces useful pretty printed output by calling the toSource method of the passed in object. Without those there is only a default formatter for objects (and functions since bug 1605658). To avoid regressing error messages for cases like new [1, 2, 3] or making the JS shell super ugly, we have to start producing better pretty printed output without toSource.

I think we could follow a similar path to bug 1605658, where we directly call the C++ code of Function.prototype.toSource. We probably do not need to do this for every kind of object, but array and error objects are definitely the most important from my POV.

Comment 1

[Tom S [:evilpie]]

To make testing easier without uneval we should expose a JS shell function that calls ValueToSource.

Comment 2

[Tom S [:evilpie]]

Attachment: Bug 1606084 - Add a valueToSource method to the shell for testing. r?arai

We will keep improving this test when we are adding new implementations like ErrorToSource that don't require a toSource method.
(--disable-tosource can be removed in the future)

Comment 3

[Tom S [:evilpie]]

Attachment: Bug 1606084 - Add ArrayToSource. r?arai

Comment 4

[Tom S [:evilpie]]

Attachment: Bug 1606084 - Add ErrorToSource. r?arai

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38ffdad9b953
Add a valueToSource method to the shell for testing. r=arai
https://hg.mozilla.org/integration/autoland/rev/1536cf66a302
Add ArrayToSource. r=arai

Comment 6

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/38ffdad9b953
https://hg.mozilla.org/mozilla-central/rev/1536cf66a302

Comment 7

[Tom S [:evilpie]]

Attachment: Bug 1606084 - Move ValueToSource to a new file. r?arai

I am not sure if I should create a separate header just for ValueToSource?
In the future after we remove toSource from chrome-js we can move more ToSource methods to this file.

Comment 8

[Tom S [:evilpie]]

Attachment: Bug 1606084 - Add RegExp support to ValueToSource fallback. r?arai

Depends on D59684

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0a175e745b6
Add ErrorToSource. r=arai

Comment 10

[Andreea Pavel [:apavel]]

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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e89f71a08de
Move ValueToSource to a new file. r=arai
https://hg.mozilla.org/integration/autoland/rev/d1cca1ec8ee0
Add RegExp support to ValueToSource fallback. r=arai

Comment 12

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/6e89f71a08de
https://hg.mozilla.org/mozilla-central/rev/d1cca1ec8ee0

Comment 13

[Tom S [:evilpie]]

I think we can implement some generalized handling for Date, Boolean, String, Number etc. by leveraging js:Unbox, but maybe that is too clever and we should just hardcode it.

Comment 14

[Tom S [:evilpie]]

Attachment: Bug 1606084 - ValueToSource should be able to handle wrapped objects. r?jwalden

Comment 15

[Tom S [:evilpie]]

Attachment: Bug 1606084 - Add support for Boolean, String, Number and Date objects to ValueToSource. r?jwalden

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e02b75f47c9b
ValueToSource should be able to handle wrapped objects. r=jwalden
https://hg.mozilla.org/integration/autoland/rev/5e253b12e2b6
Add support for Boolean, String, Number and Date objects to ValueToSource. r=jwalden

Comment 17

[Tom S [:evilpie]]

I think this covers most of the important object types for now.

Comment 18

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/e02b75f47c9b
https://hg.mozilla.org/mozilla-central/rev/5e253b12e2b6