[Webkit-unassigned] [Bug 149615] [ES6] Arrow function syntax. Arrow function specific features. Lexical bind "super" property

[bugzilla-daemon at webkit.org]

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

GSkachkov <gskachkov at gmail.com> changed:

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

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

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

>> Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp:69
>> +    // this executable is part of the class
> 
> comment isn't needed

Removed

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:575
>> +    if (SourceParseMode::ArrowFunctionMode == parseMode && (functionNode->usesThis() || isDerivedClassContext() || isDerivedConstructorContext()))
> 
> Why are these new conditions needed?
> Why do we need to load this if we're "SourceParseMode::ArrowFunctionMode == parseMode && (isDerivedClassContext() || isDerivedConstructorContext())"?

Without this condition following code will raise ReferenceError 'this' is undefined so to fix this error I've added this condition.

var A = class A {
    constructor() {
        this.value = 'testValue';
    }
    getValue () {
        return this.value;
    }

};

var B = class B extends A {
    getParentValue() {
        var arrow  = () => super.getValue();
        return arrow();
    }
};

var b = new B();
b. getParentValue()

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4050
>> +    if ((isConstructor() && constructorKind() == ConstructorKind::Derived) || (m_codeBlock->isClassContext())) {
> 
> style: the parens around "m_codeBlock->isClassContext()" aren't needed.

Done

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:813
>> +                }
> 
> Nit: I think this whole section of code would be easier to read like this:
> ```
> if (constructorKind() == ConstructorKind::Derived || isDerivedConstructorContext())
>     newDerivedContextType = DerivedContextType::DerivedConstructorContext;
> else if (m_codeBlock->isClassContext() || (m_codeBlock->isArrowFunction() && m_derivedContextType == DerivedContextType::DerivedMethodContext))
>     newDerivedContextType =  DerivedContextType::DerivedMethodContext
> ```
> 
> Also, why do we need to check "m_codeBlock->isArrowFunction()" when checking "m_derivedContextType == DerivedContextType::DerivedMethodContext"?
> When would we have "!m_codeBlock->isArrowFunction() && m_derivedContextType == DerivedContextType::DerivedMethodContext"?

Ohh, I see. We don't need to check m_codeBlock->isArrowFunction() because it already true because of this condition metadata->parseMode() == SourceParseMode::ArrowFunctionMode.

>> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:182
>> +        RegisterID* scope = generator.emitLoadDerivedConstructorFromArrowFunctionLexicalEnvironment();    
> 
> This isn't a JSScope. Maybe a better variable name is "derivedConstructor" or something similar.

Renamed

>> Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp:195
>> +    EvalExecutable* eval = EvalExecutable::create(callFrame, makeSource(script), codeBlock.isStrictMode(), thisTDZMode, codeBlock.unlinkedCodeBlock()->derivedContextType(), codeBlock.unlinkedCodeBlock()->isArrowFunction(), &variablesUnderTDZ);
> 
> have you tried this out inside the inspector to make sure it works?
> I.e, pausing inside an arrow function and typing in "super" into the console?

Good question. I vent done this. I've checked and it does not work. When I typing 'super' into console it raise exception 'SyntaxError: super is only valid inside functions.' The same behavior inside of the method of class and inside of the arrow function in class method. Looks like bug in class implementation.

>> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-superproperty.js:160
>> +      erorr = true;
> 
> typo: "erorr" => "error"

Fixed

>> LayoutTests/js/script-tests/arrowfunction-superproperty.js:78
>> +// FIXME: Problem with access to the super before super in constructor
> 
> nit: "super before super" => "super before super()"

Done

-- 
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/20151229/901d384c/attachment-0001.html>