<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<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#c38">Comment # 38</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:ticaiolima@gmail.com" title="Caio Lima <ticaiolima@gmail.com>"> <span class="fn">Caio Lima</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=303438&action=diff" name="attach_303438" title="Patch">attachment 303438</a> <a href="attachment.cgi?id=303438&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=303438&action=review">https://bugs.webkit.org/attachment.cgi?id=303438&action=review</a>
Thank you very much for the reviews
<span class="quote">>> Source/JavaScriptCore/builtins/GlobalOperations.js:94
>> + for (let nextKey of keys) {
>
> I don't think this can be a for of loop since it uses the iterator protocol, which is, sadly, observable. Instead you should do an indexed loop. Getting the length property of keys should be fine since it's an array.
>
> For some reason I thought I made this comment last time but I guess I forgot :(.</span >
You are right. Thanks catching it. Changed.
<span class="quote">>> Source/JavaScriptCore/bytecode/CodeBlock.cpp:2410
>> + for (size_t i = 0; i < count; i++) {
>
> I think you could do:
> for (const auto& entry : constants)
>
> Your current code will use the copy constructor of HashSet, which we can avoid.</span >
Agreed and changed.
<span class="quote">>> Source/JavaScriptCore/bytecode/CodeBlock.cpp:2418
>> + HashSet<UniquedStringImpl*> set = entry.first;
>
> you can make this:
> const HashSet<UniquedStringImpl*>& set = entry.first;
>
> for the same reason as above.</span >
Ditto.</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>