Last Comment Bug 569487 - generalize Reflect.parse to accept JS builder functions
: generalize Reflect.parse to accept JS builder functions
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Dave Herman [:dherman]
: Jason Orendorff [:jorendorff]
Depends on: 533874 594060
  Show dependency treegraph
Reported: 2010-06-01 16:07 PDT by Dave Herman [:dherman]
Modified: 2011-01-08 01:04 PST (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

builder parameter (70.12 KB, patch)
2010-09-17 17:10 PDT, Dave Herman [:dherman]
no flags Details | Diff | Splinter Review
updated based on changes in bug 594060 (70.39 KB, patch)
2010-10-08 08:13 PDT, Dave Herman [:dherman]
no flags Details | Diff | Splinter Review
refreshed stale patch (69.97 KB, patch)
2010-12-29 19:45 PST, Dave Herman [:dherman]
no flags Details | Diff | Splinter Review
minor bugfix (69.86 KB, patch)
2010-12-30 14:42 PST, Dave Herman [:dherman]
gal: review+
Details | Diff | Splinter Review

Description Dave Herman [:dherman] 2010-06-01 16:07:41 PDT
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.

Comment 1 Dave Herman [:dherman] 2010-09-17 17:10:37 PDT
Created attachment 476438 [details] [diff] [review]
builder parameter

Implementation attached. Next I'll document the API.

Comment 2 Dave Herman [:dherman] 2010-09-17 17:13:01 PDT
BTW, this patch builds off of the patch for 594060. The builder argument is provided as an optional 'builder' property of the configuration object.

Comment 3 Dave Herman [:dherman] 2010-10-08 08:13:11 PDT
Created attachment 481835 [details] [diff] [review]
updated based on changes in bug 594060

Updated based on recent changes in bug 594060.

Comment 4 Dave Herman [:dherman] 2010-12-29 19:45:21 PST
Created attachment 500297 [details] [diff] [review]
refreshed stale patch
Comment 5 Dave Herman [:dherman] 2010-12-30 14:42:07 PST
Created attachment 500409 [details] [diff] [review]
minor bugfix
Comment 6 Dave Herman [:dherman] 2010-12-30 14:43:27 PST
The docs now pretty exhaustively detail the API for the optional builder object:

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.

Comment 7 Andreas Gal :gal 2011-01-03 16:20:04 PST
Comment on attachment 500409 [details] [diff] [review]
minor bugfix

>From: Dave Herman <>
>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] 2011-01-03 16:33:21 PST
Comment 9 Dave Herman [:dherman] 2011-01-03 16:49:52 PST
Burned the tree; backed out:
Comment 10 Dave Herman [:dherman] 2011-01-07 16:34:19 PST
Comment 11 Chris Leary [:cdleary] (not checking bugmail) 2011-01-08 01:04:11 PST

Note You need to log in before you can comment on or make changes to this bug.