590766 - "Assertion failure: PN_TYPE(pn) == TOK_NAME || PN_TYPE(pn) == TOK_ASSIGN" with Reflect.parse, let-block

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Reflect.parse("let(x) { }")
Assertion failure: PN_TYPE(pn) == TOK_NAME || PN_TYPE(pn) == TOK_ASSIGN, at ../jsreflect.cpp:1518

Comment 1

[Dave Herman [:dherman]]

Attachment: implements let expressions and let statements

What an oversight! I didn't implement let-expressions or let-statements. This patch implements them and adds tests for them.

(These crashing tests are exactly why I wanted LOCAL_ASSERT instead of JS_ASSERT as much as possible. After addressing the lion's share of these crashers, I will want to do a review of all the JS_ASSERT's and catch-all (else/default) branches that could be making unsound assumptions, and possibly replace them with dynamic checks that aren't disabled in production.)

Dave

Comment 2

[Dave Herman [:dherman]]

Added let-statements and let-expressions to the docs:

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

Dave

Comment 3

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

Comment on attachment 469347 [details] [diff] [review]
implements let expressions and let statements

Looks good, just prefer JS_ALWAYS_TRUE(expr) to (void)expr when you do things like reserve up front.

Maybe also try a test for recursive let-expressions? Don't think there's any special corner case there, but it'd be nice to have that covered.

Comment 4

[Dave Herman [:dherman]]

http://hg.mozilla.org/tracemonkey/rev/e05e8e974c6a

Comment 5

[Dave Herman [:dherman]]

> Looks good, just prefer JS_ALWAYS_TRUE(expr) to (void)expr when you do things
> like reserve up front.
> 
> Maybe also try a test for recursive let-expressions? Don't think there's any
> special corner case there, but it'd be nice to have that covered.

Sorry, I forgot to address these before I pushed. I'll add them to the patch for bug 590772.

Dave

Comment 6

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

http://hg.mozilla.org/mozilla-central/rev/e05e8e974c6a