<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 syntax. Arrow function specific features. Lexical bind "arguments""
href="https://bugs.webkit.org/show_bug.cgi?id=145132">bug 145132</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 #259366 Flags</td>
<td>review?, commit-queue?
</td>
<td>review-, commit-queue-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function syntax. Arrow function specific features. Lexical bind "arguments""
href="https://bugs.webkit.org/show_bug.cgi?id=145132#c22">Comment # 22</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES6] Arrow function syntax. Arrow function specific features. Lexical bind "arguments""
href="https://bugs.webkit.org/show_bug.cgi?id=145132">bug 145132</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=259366&action=diff" name="attach_259366" title="Patch">attachment 259366</a> <a href="attachment.cgi?id=259366&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=259366&action=review">https://bugs.webkit.org/attachment.cgi?id=259366&action=review</a>
The overall approach here is good. There needs to be some significant reworking in the bytecode generator
(and maybe the parser) here.
Also, we need many more tests.
<span class="quote">> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:70
> + if (m_needToInitializeArguments) {</span >
It doesn't seem like you're doing any prevention of allocating the different arguments object while inside
an arrow function. This is bad. We need to prevent those allocations.
<span class="quote">> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:73
> + initializeVariable(variable(propertyNames().arguments), m_arrowFunctionArguments.get());</span >
You can just use m_argumentsRegister here.
Also, we don't prefix local variables with "m_".
<span class="quote">> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2305
> + RegisterID* m_tmpArgumentsRegister = emitGetFromScope(newTemporary(), scope, var, DoNotThrowIfNotFound);</span >
This is pretty shady. We should just be using m_argumentsRegister here.
We shouldn't resolve anything. What this requires is that we now must note
that an arguments object must be created for functions that have arrow functions
nested inside them. We can get smart here, and we should if it's not too difficult, and only allocate these
arguments objects as being needed, i.e it the arrow function uses "arguments" or eval. Maybe a good first take before
optimizing is just report that any function with an arrow function nested inside of it needs to
allocate an arguments object (unless the parent function is an arrow function itself, in which
case it should get its arguments object through op_get_arrowfunction_arguments).
Once you get this working, you can optimize cases like this:
"function foo() { var x = () => 20; }". Obviously foo doesn't need an arguments object
"function foo() { var x = (p) => eval(p); }". foo does need arguments object
"function foo() { var x = () => { var x = () => arguments; } }". foo does need an arguments object if there is a chain from a non-arrow-function through only arrow functions that use arguments or eval.
"function foo() { var x = () => { var x = () { function bar() { var x = () => arguments; } } } }". bar function does need arguments. foo doesn't.
<span class="quote">> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-arguments.js:1
> +var testCase = function (actual, expected, message) {</span >
I don't think we're covering enough tests here.
JSC has three different arguments objects it allocates.
We should be writing tests to target each of these.
We also need to ensure different properties of the
arguments objects hold. For example, we shouldn't
be able to overwrite local variables for strict mode arguments
object.
Does lexical binding of arguments object impose any restrictions on the type
of arguments object? Or does it not matter. If it doesn't matter, we need
tests for sloppy mode argument assignment inside an arrow function
which change "local" arguments parameters.</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>