569487 - generalize Reflect.parse to accept JS builder functions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Dave Herman [:dherman]]

Generalize the Reflect.parse API to accept an optional JS builder function, a la:

    Reflect.parse(src, builder)

where builder is an object with callbacks the serializer calls on every sub-node to generate the result.

Dave

Comment 1

[Dave Herman [:dherman]]

Attachment: builder parameter

Implementation attached. Next I'll document the API.

Dave

Comment 2

[Dave Herman [:dherman]]

BTW, this patch builds off of the patch for 594060. The builder argument is provided as an optional 'builder' property of the configuration object.

Dave

Comment 3

[Dave Herman [:dherman]]

Attachment: updated based on changes in bug 594060

Updated based on recent changes in bug 594060.

Dave

Comment 4

[Dave Herman [:dherman]]

Attachment: refreshed stale patch

Comment 5

[Dave Herman [:dherman]]

Attachment: minor bugfix

Comment 6

[Dave Herman [:dherman]]

The docs now pretty exhaustively detail the API for the optional builder object:

    https://developer.mozilla.org/en/SpiderMonkey/Parser_API

The docs have gotten pretty big, so they probably want to be split out into multiple pages. But MDC is such hell to edit I'm not sure I'm quite up to the task at the moment. At any rate, I think this is ready for review now.

Dave

Comment 7

[Andreas Gal :gal]

Comment on attachment 500409 [details] [diff] [review]
minor bugfix

>From: Dave Herman <[e-mail]>
>
>bug 569487, r=?: Reflect.parse(): custom builder object
>
>diff --git a/js/src/jsast.tbl b/js/src/jsast.tbl
>--- a/js/src/jsast.tbl
>+++ b/js/src/jsast.tbl
>@@ -34,85 +34,86 @@
>  * and other provisions required by the GPL or the LGPL. If you do not delete
>  * the provisions above, a recipient may use your version of this file under
>  * the terms of any one of the MPL, the GPL or the LGPL.
>  *
>+            for (uintN i = 0; i < AST_LIMIT; i++) {
>+                callbacks[i].setNull();
>+            }

No {} I think.

Looks good. NPOTB so just land it.

Comment 8

[Dave Herman [:dherman]]

http://hg.mozilla.org/tracemonkey/rev/351e87c78237

Comment 9

[Dave Herman [:dherman]]

Burned the tree; backed out:

http://hg.mozilla.org/tracemonkey/rev/83f778ef2a77

Comment 10

[Dave Herman [:dherman]]

http://hg.mozilla.org/tracemonkey/rev/1155e7cf1914

Comment 11

[Chris Leary [:cdleary] (not checking bugmail)]

http://hg.mozilla.org/mozilla-central/rev/1155e7cf1914