[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 #41 from Saam Barati <saambarati1 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

This is looking better. Some suggestions below. 
I think other people should also take a look because I'm not the best person to review all of this patch.

Will the DFG inline an ArrowFunctions or will it just never inline them?

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

"cases cases" => "cases"

> 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.

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

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1829
> +    emitTDZCheck(thisRegister());

Do we need a TDZ check here? If so, why?

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1831
> +    instructions().append(thisRegister()->index());

Above code change will let you remove this.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4564
> +    ? m_jit.graph().globalObjectFor(node->origin.semantic)->arrowFunctionStructure()

Indent four spaces (and to all below).

> 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.

> 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.

> 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?

-- 
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/aed72b79/attachment.html>