[webkit-reviews] review granted: [Bug 210023] [JSC] Implement JSMapIterator/JSSetIterator with JSInternalFieldObjectImpl : [Attachment 395553] Patch

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 210023: [JSC] Implement JSMapIterator/JSSetIterator with
JSInternalFieldObjectImpl
https://bugs.webkit.org/show_bug.cgi?id=210023

Attachment 395553: Patch

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




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

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

r=me.

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:3225
> +	       // Add the constant before exit becomes invalid because we may
want to insert (redundant) checks on it in Fixup.
> +	       Node* kindNode =
jsConstant(jsNumber(static_cast<uint32_t>(kind)));

Why does exit become invalid here? Seems like nothing here has effects should
prevent an exit?

> Source/JavaScriptCore/ftl/FTLOperations.cpp:131
> +		   target->internalField(static_cast<typename
JSCellType::Field>(property.location().info())).set(vm, target,
JSValue::decode(values[i]));

Maybe add an ASSERT(property.location().info() < typename
JSCellType::numberOfInternalFields);?