[webkit-reviews] review granted: [Bug 38970] Non-enumerable property fails to shadow inherited enumerable property from for-in : [Attachment 415295] Patch

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Alexey Shvayka
<shvaikalesh at gmail.com>'s request for review:
Bug 38970: Non-enumerable property fails to shadow inherited enumerable
property from for-in
https://bugs.webkit.org/show_bug.cgi?id=38970

Attachment 415295: Patch

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




--- Comment #18 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 415295
  --> https://bugs.webkit.org/attachment.cgi?id=415295
Patch

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

r=me with some nits.

> Source/JavaScriptCore/ChangeLog:31
> +	   This patch introduces a new invariant: all properties
getOwn*PropertyNames() returns
> +	   in DontEnumPropertiesMode::Exclude should be reported as
[[Enumerable]] by
> +	   getOwnPropertySlot(). JSCallbackObject and RuntimeArray are fixed to
follow it.

Is there a way we can assert this? I think we could at least for the VMInquiry
flavor of getOwnPropertySlot()?

> Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:167
> +	       // FIXME: Set ReadOnly conditionally, based on setProperty
presence in class inheritance chain.

Can you open a bug for this and link to it here?

> Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:199
> +		       // FIXME: getStaticValue() performs the same loop &
checks just to acquire `entry`.

Ditto on bugzilla.