[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

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

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

>> Source/JavaScriptCore/ChangeLog:9
>> +        inside of the arrow function in case if arrow function is nested in method, 
> 
> Might be worth explicitly adding constructor to this list of available contexts inside a class.

Done

>> Source/JavaScriptCore/ChangeLog:11
>> +        class, lead to wrong type of error, should be SyntaxError and this will be fixed in separete patch.
> 
> Do you have a bug open for this? If you don't, you should make one.
> You should link to it here

Done

>> Source/JavaScriptCore/bytecode/ExecutableInfo.h:39
>> +    ExecutableInfo(bool needsActivation, bool usesEval, bool isStrictMode, bool isConstructor, bool isBuiltinFunction, ConstructorKind constructorKind, GeneratorThisMode generatorThisMode, SuperBinding superBinding, SourceParseMode parseMode, DerivedContextType _derivedContextType, bool _isArrowFunctionContext, bool isClassContext)
> 
> style: Remove the "_" prefixes.

Removed

>> Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp:70
>> +    bool isClassContext = executable->parseMode() == SourceParseMode::MethodMode || executable->parseMode() == SourceParseMode::GetterMode || executable->parseMode() == SourceParseMode::SetterMode;
> 
> Is this always correct?
> We could have code like this:
> ```
> let x = {
>     get y() { return 20; }
> };
> x.y
> ```
> I'm not sure if this would count as SourceParseMode::GetterMode.

Changed. Now I'm rely on executable->superBinding() property that has value SuperBinding::Needed when it part of the class

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:578
>> +
> 
> style: revert newline.

Done

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:799
>> +            bool newisDerivedConstructorContext = constructorKind() == ConstructorKind::Derived || (derivedContextType() == DerivedContextType::DerivedConstructorContext && metadata->parseMode() == SourceParseMode::ArrowFunctionMode);
> 
> This calculation doesn't seem right to me. Shouldn't we be interested in the fact that we're creating an arrow function or not?

Added additional condition

>> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:189
>> +
> 
> style: revert newline.

Done

>> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-superproperty.js:1
>> +var testCase = function (actual, expected, message) {
> 
> Style: 4-space indent this file.

Done

>> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-superproperty.js:158
>> +// Fixme: should by check if e instanceof SyntaxError https://bugs.webkit.org/show_bug.cgi?id=150893
> 
> Style: "Fixme" => "FIXME" and also indented properly.

Done

>> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-superproperty.js:215
>> +//  testCase(j2._value, testValue, 'Error: Some problem with using "super" inside of the constructor');
> 
> Style: Indent this comment and all others to be at the level of block they're in.

Done

>> LayoutTests/js/script-tests/arrowfunction-superproperty.js:1
>> +description('Tests for ES6 arrow function, access to the super property in arrow function');
> 
> Style: 4-space indent this file.

Done

>> LayoutTests/js/script-tests/arrowfunction-superproperty.js:80
>> +//shouldThrow('(new C(false))', 'ReferenceError');
> 
> This should be "new C(true)"

Yes, fixed

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