[webkit-reviews] review denied: [Bug 197631] Don't cache HasOwnProperty on 'arguments' : [Attachment 369178] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has denied Robin Morisset
<rmorisset at apple.com>'s request for review:
Bug 197631: Don't cache HasOwnProperty on 'arguments'
https://bugs.webkit.org/show_bug.cgi?id=197631

Attachment 369178: Patch

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




--- Comment #4 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 369178
  --> https://bugs.webkit.org/attachment.cgi?id=369178
Patch

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

> JSTests/stress/has-own-property-arguments.js:3
> +class A extends Function {}
> +new A("'use strict';").hasOwnProperty('arguments');
> +new A().hasOwnProperty('arguments');

I'm super confused what this is doing. This is just calling the constructor
with a string as the first argument.

Also, classes are always in strict mode. Can you explain what's going on? Can
this also test what it expects the results to be?

> Source/JavaScriptCore/runtime/HasOwnPropertyCache.h:95
> +	   // We don't cache accesses to arguments, because it depends on
whether we are in strict or sloppy mode.
> +	   if (propName == "arguments")
> +	       return;

This seems wrong. We should be reflecting this in Structure. It's unlikely the
bug is just here and not also in Get