[webkit-reviews] review denied: [Bug 236330] [JSC] OrdinarySet should invoke custom [[Set]] methods for indexed puts : [Attachment 451388] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has denied Alexey Shvayka
<ashvayka at apple.com>'s request for review:
Bug 236330: [JSC] OrdinarySet should invoke custom [[Set]] methods for indexed
puts
https://bugs.webkit.org/show_bug.cgi?id=236330

Attachment 451388: Patch

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




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

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

> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h:405
> +	   if (UNLIKELY(slot.thisValue() != thisObject)) {

can we do the "isThisValueAltered" thing?

> Source/JavaScriptCore/runtime/JSObject.cpp:2899
> +	   if
(current->structure(vm)->typeInfo().interceptsGetOwnPropertySlotByIndexEvenWhen
LengthIsNotZero()) {

this is bizarre that we're using this flag to indicate what to do for put. This
feels like an inconsistent fix to me.

> Source/JavaScriptCore/runtime/StructureInlines.h:93
> +    if
(typeInfo().interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero())
> +	   return true;

this is not the intention of this flag. Now, if you put a typed array in a
prototype chain, you're going to "have a bad time". Why is this needed?