[webkit-reviews] review granted: [Bug 99557] Array and object allocations via 'new Object' or 'new Array' should be inlined in bytecode to allow allocation site profiling : [Attachment 169136] the patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 99557: Array and object allocations via 'new Object' or 'new Array' should
be inlined in bytecode to allow allocation site profiling
https://bugs.webkit.org/show_bug.cgi?id=99557

Attachment 169136: the patch
https://bugs.webkit.org/attachment.cgi?id=169136&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=169136&action=review


r=me

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:438
> +	   // This passes NoExpectedFunction because we expect that if the
function is in a
> +	   // local variable, then it's not one of our built-in constructors.

Slightly more accurate to say "then we aren't able to prove that it's one of
our built-in constructors".

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:456
> +    // FIXME: Reconsider passing expectedFunction instead of
NoExpectedFunction.

Are you saying we should consider *not* passing expectedFunction here? I'd
suggest saying something here about why we might want to make that change.
Otherwise, this comment doesn't give specific direction to another reader.