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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 11 09:47:09 PDT 2020

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

Attachment 399023: Patch


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

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) :

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.

More information about the webkit-reviews mailing list