[Webkit-unassigned] [Bug 144956] [ES6] Implement ES6 arrow function syntax. Arrow function specific features. Lexical bind of this

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=144956

--- Comment #89 from Basile Clement <basile_clement at apple.com> ---
Comment on attachment 257956
  --> https://bugs.webkit.org/attachment.cgi?id=257956
Patch

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

Saam asked me to comment on allocation sinking stuff, so here it is.

> Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:848
> +            target = &m_heap.newAllocation(node, Allocation::Kind::Function);

This is wrong. You need to add Allocation::Kind::ArrowFunction and use it here, otherwise you are transforming an arrow function into a regular function when sinking (not eliminating).
You should have isFunction() return true for Kind::ArrowFunction as well, and define a new isArrowFunction() helper in the Allocation class. Here is a test case for this, if my understanding of arrow functions is correct:

````
function f(p) {
  var arr = (x) => this;
  if (p) return arr;
}

for (var i = 0; i < 100000; ++i)
    f(i % 2);

if (f(true)() != f)
    throw new Error("Bad sinking.");
````

>> Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:997
>> +        case LoadArrowFunctionThis:
> 
> I don't think we need to worry about LoadArrowFunctionThis in the allocation
> sinking phase. This will just be loading from callee which should be materialized already at the top of a function.

We'd need to worry about it if we inlined arrow functions, which this patch does not handle by the look of it. If the plan is to implement inlining of arrow functions later, this should at least be a FIXME. Otherwise, this should check for target->isArrowFunctionAllocation() instead; cf above.

>> Source/JavaScriptCore/dfg/DFGPromoteHeapAccess.h:74
>> +    case LoadArrowFunctionThis:
> 
> I think this can be removed. It doesn't seem correct.
> We're using FunctionBoundThisPLoc when sinking the arrow function, not when
> actually executing an arrow function (LoadArrowFunctionThis is only used when Arrow function is the callee).

The whole file should be removed. It was used by the old object allocation sinking phase and is no longer included by anyone. I'll put up a patch to remove it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150801/ce643732/attachment.html>