<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:keith_miller@apple.com" title="Keith Miller <keith_miller@apple.com>"> <span class="fn">Keith Miller</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [ESnext] Implement Object Rest - Implementing Object Rest Destructuring"
href="https://bugs.webkit.org/show_bug.cgi?id=167962">bug 167962</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 #301324 Flags</td>
<td>review?
</td>
<td>review-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [ESnext] Implement Object Rest - Implementing Object Rest Destructuring"
href="https://bugs.webkit.org/show_bug.cgi?id=167962#c15">Comment # 15</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ESnext] Implement Object Rest - Implementing Object Rest Destructuring"
href="https://bugs.webkit.org/show_bug.cgi?id=167962">bug 167962</a>
from <span class="vcard"><a class="email" href="mailto:keith_miller@apple.com" title="Keith Miller <keith_miller@apple.com>"> <span class="fn">Keith Miller</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=301324&action=diff" name="attach_301324" title="Patch">attachment 301324</a> <a href="attachment.cgi?id=301324&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=301324&action=review">https://bugs.webkit.org/attachment.cgi?id=301324&action=review</a>
I think it looks pretty good. I have a few comments, however.
<span class="quote">> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:4051
> + if (UNLIKELY(m_containsRestElement)) {
> + RefPtr<RegisterID> propertyName = generator.emitLoad(nullptr, target.propertyName);
> + generator.emitDirectPutByVal(excludedList.get(), excludedIndex.get(), propertyName.get());
> + generator.emitInc(excludedIndex.get());</span >
Don't we statically know what we are going to put in the excludedList at this point? Can we just add this array to the constant pool? If we do, we should probably freeze so the array can't be modified.
<span class="quote">> Source/JavaScriptCore/parser/Parser.cpp:1019
> + if (kind == DestructuringKind::DestructureToExpressions && !innerPattern)
> + return 0;
> + failIfFalse(innerPattern, "Cannot parse this destructuring pattern");</span >
I think you just want to do propageError(); here
<span class="quote">> Source/JavaScriptCore/runtime/JSGlobalObject.h:117
> + macro(Set, set, set, JSSet, Set, object) \</span >
Why did you move this?</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>