<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><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> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function created before super() causes TDZ, should it?"
href="https://bugs.webkit.org/show_bug.cgi?id=149338">bug 149338</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #264078 Flags</td>
<td>review?
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function created before super() causes TDZ, should it?"
href="https://bugs.webkit.org/show_bug.cgi?id=149338#c50">Comment # 50</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function created before super() causes TDZ, should it?"
href="https://bugs.webkit.org/show_bug.cgi?id=149338">bug 149338</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=264078&action=diff" name="attach_264078" title="Patch">attachment 264078</a> <a href="attachment.cgi?id=264078&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=264078&action=review">https://bugs.webkit.org/attachment.cgi?id=264078&action=review</a>
Patch looks mostly good. Just have some comments regarding some tricky problems with the implementation for some edge cases.
<span class="quote">> Source/JavaScriptCore/ChangeLog:9
> + 'this' is stored inside of the lexical environment of the function. To store and load are used </span >
nit: "are used" => "we use"
<span class="quote">> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:849
> + m_arrowFunctionContextLexicalEnvironmentRegister = emitMove(newBlockScopeVariable(), m_scopeRegister);</span >
I was wrong about advising you on this earlier.
I forgot that pushLexicalScopeInternal already creates a variable for us.
Because you have captured variables, we will create a block scoped variable for
the scope.
So all you have to do is this:
m_arrowFunctionContextLexcialEnvironmentRegister = m_symbolTableStack.last().m_scope.
Feel free to add asserts around the pushLexicalScopeInternal call to make sure it actually adds exactly one item to the symbol table stack.
<span class="quote">> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2989
> + if (m_codeType == FunctionCode) {</span >
I realized we don't need this.
We should only need to do this once per function.
If you look how new_target is materialized now, it happens
before we create a "this". So I think it'd be wrong to update
this value over time. It should just happen once at the very beginning of the program.
<span class="quote">> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:3881
> +{</span >
There are a few interesting things to consider here that seem like they are problems.
1) consider the problem of using the literal string "__proto__":
class A extends B {
let arr1 = () => {
let __proto__ = "blah";
let arr2 = () => super();
arr2();
}
}
This strategy worked with "this" because no variable can be named "this". But variables can be named "__proto__"
2) what happens when __proto__ is overwritten (I'm not really sure if this is a problem or what's supposed to happen with respect to the specification, but it's probably worth looking into).
class A extends B {
let arr1 = () => { ... }
A.__proto__ = null;
arr1();
}
I wonder what happens just in normal classes with this.
3) If we're storing values in m_calleeRegister, maybe we're doing this wrong. I think this might break stack traces because it will look like an arrow functions parent frame is displayed twice on a stack trace if an error is thrown.
It's worth considering this problem.
For all problems above, if they really are problems, we should have tests for them, too.</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>