<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function syntax. Arrow function specific features. Lexical bind "super" property"
href="https://bugs.webkit.org/show_bug.cgi?id=149615#c8">Comment # 8</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function syntax. Arrow function specific features. Lexical bind "super" property"
href="https://bugs.webkit.org/show_bug.cgi?id=149615">bug 149615</a>
from <span class="vcard"><a class="email" href="mailto:gskachkov@gmail.com" title="GSkachkov <gskachkov@gmail.com>"> <span class="fn">GSkachkov</span></a>
</span></b>
<pre>Comment on <span class="bz_obsolete"><a href="attachment.cgi?id=267103&action=diff" name="attach_267103" title="Patch">attachment 267103</a> <a href="attachment.cgi?id=267103&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=267103&action=review">https://bugs.webkit.org/attachment.cgi?id=267103&action=review</a>
<span class="quote">>> 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.</span >
Done
<span class="quote">>> 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</span >
Done
<span class="quote">>> 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.</span >
Removed
<span class="quote">>> 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.</span >
Changed. Now I'm rely on executable->superBinding() property that has value SuperBinding::Needed when it part of the class
<span class="quote">>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:578
>> +
>
> style: revert newline.</span >
Done
<span class="quote">>> 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?</span >
Added additional condition
<span class="quote">>> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:189
>> +
>
> style: revert newline.</span >
Done
<span class="quote">>> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-superproperty.js:1
>> +var testCase = function (actual, expected, message) {
>
> Style: 4-space indent this file.</span >
Done
<span class="quote">>> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-superproperty.js:158
>> +// Fixme: should by check if e instanceof SyntaxError <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function syntax. Using 'super' in arrow function that declared out of the class should lead to Syntax error"
href="show_bug.cgi?id=150893">https://bugs.webkit.org/show_bug.cgi?id=150893</a>
>
> Style: "Fixme" => "FIXME" and also indented properly.</span >
Done
<span class="quote">>> 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.</span >
Done
<span class="quote">>> 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.</span >
Done
<span class="quote">>> LayoutTests/js/script-tests/arrowfunction-superproperty.js:80
>> +//shouldThrow('(new C(false))', 'ReferenceError');
>
> This should be "new C(true)"</span >
Yes, fixed</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>