Closed Bug 1606084 Opened 2 years ago Closed 2 years ago

ValueToSource should be useful without toSource methods

Categories

(Core :: JavaScript Engine, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: evilpie, Assigned: evilpie)

References

(Blocks 1 open bug)

Details

Attachments

(7 files)

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.

Priority: -- → P3
Assignee: nobody → evilpies
Keywords: leave-open

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

Blocks: 1418769
No longer blocks: 1565170

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)

Pushed by evilpies@gmail.com:
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
Depends on: 1605658

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.

Pushed by evilpies@gmail.com:
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

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.

Pushed by evilpies@gmail.com:
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

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

Keywords: leave-open
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74
You need to log in before you can comment on or make changes to this bug.