[webkit-reviews] review granted: [Bug 201857] [JSC] CheckArray+NonArray is not filtering out Array in AI : [Attachment 378928] 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 201857: [JSC] CheckArray+NonArray is not filtering out Array in AI
https://bugs.webkit.org/show_bug.cgi?id=201857

Attachment 378928: Patch

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




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

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

r=me with some comments.

> Source/JavaScriptCore/ChangeLog:10
> +	   While we are assuming that CheckArray+NonArray can ensure that it
only passes non-array inputs, DFG::ArrayMode::alreadyChecked

Nit: While we assume CheckArray+NonArray ensures it only...

> Source/JavaScriptCore/dfg/DFGArrayMode.cpp:441
> +    // But here, we already filtered TypedArrays. So, just handling it like
NonArray.

Nit: So, just handle it like a NonArray.

> Source/JavaScriptCore/dfg/DFGArrayMode.cpp:478
> +	   for (unsigned i = value.m_structure.size(); i--;) {
> +	       RegisteredStructure structure = value.m_structure[i];
> +	       if (structure.get() != originalStructure)

Isn't this a set so it should have size 1? How is it possible to have more than
one copy of the same structure?