[webkit-reviews] review granted: [Bug 144093] Implement @isConstructor bytecode intrinsic and bytecode for that : [Attachment 399023] Patch

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Alexey Shvayka
<shvaikalesh at gmail.com>'s request for review:
Bug 144093: Implement @isConstructor bytecode intrinsic and bytecode for that
https://bugs.webkit.org/show_bug.cgi?id=144093

Attachment 399023: Patch

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




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

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

r=me but what fixed the test262 test? It's not obvious from the patch.

> Source/JavaScriptCore/ChangeLog:9
> +	   This change replaces @isConstructor link-time-constant with bytecode
intrinsic and utilizes it
> +	   in ClassExprNode::emitBytecode() according to the spec [1], aligning
JSC with V8 and SpiderMonkey.

Can you elaborate on what semantically changed here? From the rest of the
ChangeLog this seems like a perf-only change.

> Source/JavaScriptCore/builtins/ArrayConstructor.js:31
> +    var array = this !== @Array && @isConstructor(this) ? new this(length) :
@newArrayWithSize(length);

Is this just for perf?

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1737
> +	       if (!(child.m_type & (SpecFunction | SpecProxyObject))) {

Can you add a FIXME + bugzilla to someday iterate all Structures in the
abstract values structure set and see if we know they are all constructible?
Right now that's not possible because we don't have the object but for many
(most?) structures we shouldn't need the object itself to answer this question.