[webkit-reviews] review granted: [Bug 182851] Fix bugs from r228411 : [Attachment 333974] patch

[bugzilla-daemon at webkit.org]

JF Bastien <jfbastien at apple.com> has granted Saam Barati <sbarati at apple.com>'s
request for review:
Bug 182851: Fix bugs from r228411
https://bugs.webkit.org/show_bug.cgi?id=182851

Attachment 333974: patch

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




--- Comment #5 from JF Bastien <jfbastien at apple.com> ---
Comment on attachment 333974
  --> https://bugs.webkit.org/attachment.cgi?id=333974
patch

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

r=me

> JSTests/stress/constant-folding-phase-insert-check-handle-varargs.js:7
> +    return properties[seed % properties.length];

I usually inline the object / properties because then the repro case isn't
random anymore.

> Source/JavaScriptCore/dfg/DFGGraph.h:529
> +    AdjacencyList copyVarargChildren(Node* node, WTF::Function<bool(Edge)>
filter = [] (Edge) { return true; })

I think it's better to pass the function as a template param here, so you never
have a WTF::Function ever.

> Source/JavaScriptCore/dfg/DFGPureValue.h:107
> +	       hash ^= m_graph->m_varArgChildren[m_children.firstChild() +
i].sanitized().hash();

Sanitized hash is important here? Why?