<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#c13">Comment # 13</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:sbarati@apple.com" title="Saam Barati <sbarati@apple.com>"> <span class="fn">Saam Barati</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=267799&action=diff" name="attach_267799" title="Patch">attachment 267799</a> <a href="attachment.cgi?id=267799&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=267799&action=review">https://bugs.webkit.org/attachment.cgi?id=267799&action=review</a>
<span class="quote">> Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp:69
> + // this executable is part of the class</span >
comment isn't needed
<span class="quote">> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:575
> + if (SourceParseMode::ArrowFunctionMode == parseMode && (functionNode->usesThis() || isDerivedClassContext() || isDerivedConstructorContext()))</span >
Why are these new conditions needed?
Why do we need to load this if we're "SourceParseMode::ArrowFunctionMode == parseMode && (isDerivedClassContext() || isDerivedConstructorContext())"?
<span class="quote">> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:4050
> + if ((isConstructor() && constructorKind() == ConstructorKind::Derived) || (m_codeBlock->isClassContext())) {</span >
style: the parens around "m_codeBlock->isClassContext()" aren't needed.
<span class="quote">> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:813
> + bool newisDerivedConstructorContext = constructorKind() == ConstructorKind::Derived || (derivedContextType() == DerivedContextType::DerivedConstructorContext);
> +
> + if (newisDerivedConstructorContext)
> + newDerivedContextType = DerivedContextType::DerivedConstructorContext;
> + else {
> + bool isArrowFunctionInClassContext = m_codeBlock->isClassContext() || (m_codeBlock->isArrowFunction() && m_derivedContextType == DerivedContextType::DerivedMethodContext);
> + newDerivedContextType = isArrowFunctionInClassContext ? DerivedContextType::DerivedMethodContext : DerivedContextType::None;
> + }</span >
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"?
<span class="quote">> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:182
> + RegisterID* scope = generator.emitLoadDerivedConstructorFromArrowFunctionLexicalEnvironment(); </span >
This isn't a JSScope. Maybe a better variable name is "derivedConstructor" or something similar.
<span class="quote">> Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp:195
> + EvalExecutable* eval = EvalExecutable::create(callFrame, makeSource(script), codeBlock.isStrictMode(), thisTDZMode, codeBlock.unlinkedCodeBlock()->derivedContextType(), codeBlock.unlinkedCodeBlock()->isArrowFunction(), &variablesUnderTDZ);</span >
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?
<span class="quote">> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-superproperty.js:160
> + erorr = true;</span >
typo: "erorr" => "error"
<span class="quote">> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-superproperty.js:162
> + testCase(erorr, true, 'Error: using "super" should lead to error');</span >
ditto
<span class="quote">> LayoutTests/js/script-tests/arrowfunction-superproperty.js:78
> +// FIXME: Problem with access to the super before super in constructor</span >
nit: "super before super" => "super before super()"</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>