[webkit-reviews] review granted: [Bug 201897] Syntax checker should report duplicate __proto__ properties : [Attachment 379022] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Tadeu Zagallo <tzagallo at apple.com>'s
request for review:
Bug 201897: Syntax checker should report duplicate __proto__ properties
https://bugs.webkit.org/show_bug.cgi?id=201897

Attachment 379022: Patch

https://bugs.webkit.org/attachment.cgi?id=379022&action=review




--- Comment #3 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 379022
  --> https://bugs.webkit.org/attachment.cgi?id=379022
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=379022&action=review

r=me if test262 passes.

> Source/JavaScriptCore/ChangeLog:14
> +	   When syntax checking in sloppy mode, we call parseObjectLiteral,
which does not fill the
> +	   property names and therefore fails to detect the duplicate __proto__
properties. We replace
> +	   parseObjectLiteral with the strict version since:
> +	   - the only difference between parseObjectLiteral and
parseStrictObjectLiteral seems to be
> +	     whether we store the properties' names
> +	   - parseObjectLiteral has to fallback to parseStrictObjectLiteral in
the presence of getters/setters.

Ok, I think I understand the difference now.  parseObjectLiteral() passes
TreeBuilder::DontBuildStrings to the Lexer, but parseStrictObjectLiteral() does
not.  This appears to be an optimization that went wrong, because the parser
relies on getting the token strings in order to do the redefined __proto__
check.	But since it told the lexer not to build this token (which it wrongly
presumed is unnecessary work for the SyntaxChecker), the test fails.  It is
safe to always use parseStrictObjectLiteral() instead because there is actually
no semantic difference in terms of "strict" vs "sloppy":
parseStrictObjectLiteral() is also misnamed.  Can you please put these details
in the ChangeLog?

You should still run test262 to make sure there are no hidden surprises here.