<rdar://problem/53201788>
Created attachment 379022 [details] Patch
Comment on attachment 379022 [details] Patch For a change like this, can you run test262 to make sure there are no regressions? I wonder what was the rationale for having a different parseStrictObjectLiteral() in the first place.
Comment on attachment 379022 [details] 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.
Thanks for the review. I checked test262 and JetStream2 and it seems all good. I'll update the ChangeLog before landing it.
Created attachment 379148 [details] Patch for landing
Comment on attachment 379148 [details] Patch for landing Clearing flags on attachment: 379148 Committed r250098: <https://trac.webkit.org/changeset/250098>
All reviewed patches have been landed. Closing bug.