Currently, `typeof XXX === "function"` is used (in Array.from) However, it's not accuate to the spec. According to the spec, this should be queried by `isConstructor` abstract operation. While `isConstructor` queries the given object has `[[Construct]]`, `typeof thisObj === "function"` queries the given object has `[[Call]]`. To fix that, we introduce a new bytecode op_is_constructor and use it with bytecode intrinsic @isConstructor.
ES6 Class is also require it. http://people.mozilla.org/~jorendorff/es6-draft.html#sec-runtime-semantics-classdefinitionevaluation
Created attachment 399023 [details] Patch
(In reply to Alexey Shvayka from comment #2) > Created attachment 399023 [details] > Patch With warm-up runs, --outer 16: r261438 patch array-from-arraylike 86.0394+-2.1520 ^ 53.7155+-1.7785 ^ definitely 1.6018x faster array-of 76.1386+-3.3354 ^ 45.5391+-1.6705 ^ definitely 1.6719x faster is-constructor 53.2389+-1.4687 ^ 38.8312+-1.3457 ^ definitely 1.3710x faster <geometric> 70.3023+-1.1954 ^ 45.5675+-0.8233 ^ definitely 1.5428x faster
(In reply to Alexey Shvayka from comment #3) > (In reply to Alexey Shvayka from comment #2) > > Created attachment 399023 [details] > > Patch > > With warm-up runs, --outer 16: > > r261438 patch > > > array-from-arraylike 86.0394+-2.1520 ^ 53.7155+-1.7785 > ^ definitely 1.6018x faster > array-of 76.1386+-3.3354 ^ 45.5391+-1.6705 > ^ definitely 1.6719x faster > is-constructor 53.2389+-1.4687 ^ 38.8312+-1.3457 > ^ definitely 1.3710x faster > > <geometric> 70.3023+-1.1954 ^ 45.5675+-0.8233 > ^ definitely 1.5428x faster Nice!
Comment on attachment 399023 [details] 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.
Can you ensure that total number of bytecodes does not exceed 255 yet?
*** Bug 193057 has been marked as a duplicate of this bug. ***
Created attachment 399164 [details] Patch Adjust WPT expectations, defer changes unrelated to test262 fix, make ChangeLog more clear, add FIXME.
(In reply to Yusuke Suzuki from comment #6) > Can you ensure that total number of bytecodes does not exceed 255 yet? As of r261556, we have 185 ops in Bytecode section of bytecode/BytecodeList.rb, 67 of which are in groups. Am I counting right?
Created attachment 399212 [details] Patch Update mac-wk1 expectations because the builder has ENABLE_DATALIST_ELEMENT off.
Created attachment 399234 [details] Patch Rework expectations for wpt/custom-elements/builtin-coverage.html.
Committed r261600: <https://trac.webkit.org/changeset/261600> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399234 [details].
<rdar://problem/63171871>
I suspect this may have caused Bug 212430.
Note: This change was rolled out due to web page loading problems: Committed r262231: <https://trac.webkit.org/changeset/262231>
(In reply to Brent Fulgham from comment #15) > Note: > > This change was rolled out due to web page loading problems: > > Committed r262231: <https://trac.webkit.org/changeset/262231> My understanding is that this was not a revert but rather a follow-up fix.
(In reply to Chris Dumez from comment #16) > (In reply to Brent Fulgham from comment #15) > > Note: > > > > This change was rolled out due to web page loading problems: > > > > Committed r262231: <https://trac.webkit.org/changeset/262231> > > My understanding is that this was not a revert but rather a follow-up fix. Right. I fixed the incorrect DFG AI rule included in this patch. The other part remains.