Bug 23037

Summary: Parsing and reparsing disagree on automatic semicolon insertion
Product: WebKit Reporter: Cameron Zwarich (cpst) <zwarich>
Component: JavaScriptCoreAssignee: Cameron Zwarich (cpst) <zwarich>
Status: RESOLVED FIXED    
Severity: Normal CC: pknight, sam
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Proposed patch sam: review+

Description Cameron Zwarich (cpst) 2008-12-30 11:28:16 PST
Parsing and reparsing disagree about automatic semicolon insertion, so that a function like

function() { a = 1, }

is parsed as being syntactically valid but gets a syntax error upon reparsing. This leads to an assertion failure in Parser::reparse(). It is not that big of an issue in practice, because in a release build such a function will return 'undefined' when called. Other browsers would throw a syntax error, and there aren't many people who write incorrect JS like this and only check their sites with WebKit.

In this case, we are not following the spec and it should be a syntax error. However, unless there is a newline separating the ',' and the '}', JavaScriptCore would never treat it as a syntax error in the past either. It will be a bit of work to make our automatic semicolon insertion match the spec, so I am just going to match our past behaviour.
Comment 1 Cameron Zwarich (cpst) 2008-12-30 11:29:10 PST
<rdar://problem/6467124>
Comment 2 Cameron Zwarich (cpst) 2008-12-30 11:35:42 PST
Created attachment 26314 [details]
Proposed patch
Comment 3 Darin Adler 2008-12-30 12:10:01 PST
Comment on attachment 26314 [details]
Proposed patch

I don't think that "during reparsing" is a good name for a boolean. I think that "is reparsing" would be better, or possibly "is reparsing underway".

I don't see any code setting m_duringReparsing back to false. How can subsequent parsing possibly work properly?
Comment 4 Sam Weinig 2008-12-30 13:01:48 PST
It is reset in Lexer::clear()
Comment 5 Cameron Zwarich (cpst) 2008-12-30 15:36:19 PST
(In reply to comment #3)
> (From update of attachment 26314 [details] [review])
> I don't think that "during reparsing" is a good name for a boolean. I think
> that "is reparsing" would be better, or possibly "is reparsing underway".

I had it like that originally, and it seemed awkward. I'll change it back.

> I don't see any code setting m_duringReparsing back to false. How can
> subsequent parsing possibly work properly?

Like Sam said, it is reset during clear().
Comment 6 Cameron Zwarich (cpst) 2008-12-30 16:10:35 PST
Landed in r39521 with Darin's comment.