760304 - Reflect.parse needs to be updated to support defaults and rest parameters

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[:Benjamin Peterson]

The reflect AST needs to have additions for rest parameters and defaults. I need bug 759498 first, since it alters the parse tree for defaults.

Comment 1

[:Benjamin Peterson]

Attachment: support rest and defaults

Comment 2

[Dave Herman [:dherman]]

Comment on attachment 629676 [details] [diff] [review]
support rest and defaults

>     return newNode(type, pos,
>                    "id", id,
>                    "params", array,
>+                   "defaults", defarray,
>                    "body", body,
>+                   "rest", BooleanValue(hasRest),

The rest-argument has a name -- are you including the name in the params list and this just indicates whether the last name is a rest-arg? Would it be a nicer API to have the params array only have the non-rest params, and the .rest field to be either a string or null?

>                    "generator", BooleanValue(isGenerator),
>                    "expression", BooleanValue(isExpression),
>                    dst);
> }

>diff --git a/js/src/tests/js1_8_5/extensions/reflect-parse.js b/js/src/tests/js1_8_5/extensions/reflect-parse.js
>--- a/js/src/tests/js1_8_5/extensions/reflect-parse.js
>+++ b/js/src/tests/js1_8_5/extensions/reflect-parse.js
>@@ -20,24 +20,28 @@ var _ = Pattern.ANY;

...

>+assertDecl("function foo(...rest) { }",
>+           funDecl(ident("foo"), [ident("rest")], blockStmt([]), [], true));

Yeah, so I was thinking it might make more sense for the API to return

    funDecl(ident("foo"), [], blockStmt([]), [], "rest")

But I'm not adamant about this. It just seems a little less error-prone somehow. What do you think?

Please update the docs at https://developer.mozilla.org/en/SpiderMonkey/Parser_API if you haven't already.

Overall looks good. r=me; it's up to you if you want to make that API change I suggested, or we can discuss in the bug.

Thanks,
Dave

Comment 3

[:Benjamin Peterson]

I think your suggested API is better.

Comment 4

[:Benjamin Peterson]

Attachment: store rest as an identifier separate from params

Comment 5

[:Benjamin Peterson]

The wiki has been updated.

Comment 6

[David Zbarsky (:dzbarsky)]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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