[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

GSkachkov <gskachkov at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #256242|1                           |0
        is obsolete|                            |

--- Comment #43 from GSkachkov <gskachkov at gmail.com> ---
Comment on attachment 256242
  --> https://bugs.webkit.org/attachment.cgi?id=256242
Patch

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

>> Source/JavaScriptCore/ChangeLog:9
>> +        In patch were implemented the following cases cases:
> 
> "cases cases" => "cases"

Done

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1809
>> +void BytecodeGenerator::emitNewFunctionAbstract(RegisterID* dst, FuncExprNode* func, OpcodeID opcodeID)
> 
> Call this function "emitNewFunctionCommon"
> and also assert that opcodeID is what you expect.

Done

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1817
>>      instructions().append(index);
> 
> I would append thisRegister conditionally here:
> if (opcodeID == op_new_arrow_func_exp)
>     instruction().append(thisRegister()->index())

Done

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1829
>> +    emitTDZCheck(thisRegister());
> 
> Do we need a TDZ check here? If so, why?

See comment from Ryosuke Niwa https://bugs.webkit.org/show_bug.cgi?id=140855#c66
Without this TDZ checks following test will fail LayoutTests/js/script-tests/arrowfunction-tdz.js

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1831
>> +    instructions().append(thisRegister()->index());
> 
> Above code change will let you remove this.

Done

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4564
>> +    ? m_jit.graph().globalObjectFor(node->origin.semantic)->arrowFunctionStructure()
> 
> Indent four spaces (and to all below).

Done

>> Source/JavaScriptCore/jit/JITOpcodes.cpp:986
>> +void JIT::emit_op_new_abstract_func_exp(Instruction* currentInstruction, FunctionType functionType)
> 
> Nit: I would camel case this and call it:
> emitNewFuncExprCommon.

Done

>> Source/JavaScriptCore/jit/JITOperations.cpp:1024
>> +EncodedJSValue JIT_OPERATION operationNewArrowFunctionWithInvalidatedReallocationWatchpoint(ExecState* exec, JSScope* scope, JSCell* functionExecutable, EncodedJSValue thisValue)
> 
> Nit: This function and opeartionNewArrowFunction are nearly identical. There is probably an easy way
> to combine them.

Done

>> Source/JavaScriptCore/runtime/JSArrowFunction.cpp:99
>> +    // Fixme: m_arrowFunction already removed by GC, because it created in before run JSArrowFunction::create method
> 
> What's happening with this?

Actually I don't know. When JSArrowFunction::visitChildren is invoked the thisObject->m_arrowFunction property is already empty so it lead to error. I susspect that this property is already deleted by GC.

-- 
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/20150707/b40b30bb/attachment-0001.html>