Comment on attachment 766710 [details] [diff] [review] Remove TokenStream::positionAfterLastFunctionKeyword in favor of TokenStream::tell It seems that positionAfterLastFunctionKeyword is useless since the introduction of lazy parsing. Does this change make sense? Seen while working on function*.

Adding people from bug 819509.

Coincidentally, I just r+d one of bhackett's patches (lost the link to it now) that fixed a fuzz bug by doing exactly this.

Yeah, bug 884920 does this to fix a crash, should land today.

Bizarre synchrony! Note that this patch also removes the keepAtoms argument to TokenStream, as AFAICT it's no longer needed. Not sure about that, though!

Comment on attachment 766710 [details] [diff] [review] Remove TokenStream::positionAfterLastFunctionKeyword in favor of TokenStream::tell Review of attachment 766710 [details] [diff] [review]: ----------------------------------------------------------------- r=me to remove the keepAtoms argument, if that's what remains to be done here.

Updated patch removes keepAtoms argument, setting checkin-needed. Thanks for the review.

Comment on attachment 783781 [details] [diff] [review] Remove keepAtoms member in TokenStream Review of attachment 783781 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/frontend/TokenStream.cpp @@ +1168,5 @@ > } > tt = TOK_NAME; > if (!checkForKeyword(chars, length, &tt)) > goto error; > + if (tt != TOK_NAME) goto out; This looks like a typo. Please change it back!

Comment on attachment 783781 [details] [diff] [review] Remove keepAtoms member in TokenStream Also, if you post an updated version of an r+'d patch for check-in, it's customary to mark it r+ yourself. That way, the non-obsoleted patches all have r+ on them, which saves people thinking "why was that non-r+'d patch checked in?"

(In reply to Nicholas Nethercote [:njn] from comment #11) > Comment on attachment 783781 [details] [diff] [review] > Remove keepAtoms member in TokenStream > > Review of attachment 783781 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: js/src/frontend/TokenStream.cpp > @@ +1168,5 @@ > > } > > tt = TOK_NAME; > > if (!checkForKeyword(chars, length, &tt)) > > goto error; > > + if (tt != TOK_NAME) goto out; > > This looks like a typo. Please change it back! Sorry about that. Good catch! Will fold into bug 885695, I guess.