[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 #111 from GSkachkov <gskachkov at gmail.com> ---
Comment on attachment 258572
  --> https://bugs.webkit.org/attachment.cgi?id=258572
Patch

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

>>> Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:-4557
>>> -</Project>
>> 
>> Is this intended?
> 
> Is this intended?

Reverted

>>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:729
>>> +            return UnlinkedFunctionExecutable::create(m_vm, m_scopeNode->source(), body, isBuiltinFunction() ? UnlinkedBuiltinFunction : UnlinkedNormalFunction, constructAbility, variablesUnderTDZ, nullptr);
>> 
>> Not: if nullptr is already the default parameter here you can revert this line.
> 
> Not: if nullptr is already the default parameter here you can revert this line.

Done

>>> Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:852
>>> +        case NewArrowFunction: {
>> 
>> Nit:
>> This code is mostly the same as the NewFunction case. 
>> I think it's better to have these cases be the same and then
>> have them differ where they need to based on op == NewArrowFunction. 
>> I know this is a bit pedantic it's better for future proofing code.
> 
> Nit:
> This code is mostly the same as the NewFunction case. 
> I think it's better to have these cases be the same and then
> have them differ where they need to based on op == NewArrowFunction. 
> I know this is a bit pedantic it's better for future proofing code.

Refactored

>>> Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp:1446
>>> +        case Allocation::Kind::NewArrowFunction: {
>> 
>> Ditto here.
> 
> Ditto here.

Done

>>> Source/JavaScriptCore/jit/JITOperations.cpp:945
>>> +EncodedJSValue static operationNewArrowFunctionCommon(ExecState* exec, JSScope* scope, JSCell* functionExecutable, EncodedJSValue thisValue, bool isInvalidated)
>> 
>> Nit: call this operationNewFunctionCommon
> 
> Nit: call this operationNewFunctionCommon

Renamed

>>> Source/JavaScriptCore/jit/JITOperations.cpp:947
>>> +    UNUSED_PARAM(thisValue);
>> 
>> This seems wrong. You do use "thisValue"
> 
> This seems wrong. You do use "thisValue"

Fixed

>>> Source/JavaScriptCore/parser/Nodes.h:1880
>>> +    class ArrowFuncExprNode : public FuncExprNode {
>> 
>> I would either have his inherit from ExpressionNode or create a new
>> parent class that both FuncExprNode and ArrowFuncExprNode inherit from. 
>> This inheritance chain seems a bit weird.
> 
> I would either have his inherit from ExpressionNode or create a new
> parent class that both FuncExprNode and ArrowFuncExprNode inherit from. 
> This inheritance chain seems a bit weird.

Done. Added new class BaseFuncExprNode

-- 
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/20150809/18a21062/attachment.html>