[Webkit-unassigned] [Bug 167962] [ESnext] Implement Object Rest - Implementing Object Rest Destructuring

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 8 09:29:49 PST 2017


https://bugs.webkit.org/show_bug.cgi?id=167962

Keith Miller <keith_miller at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #303438|review?                     |review-
              Flags|                            |

--- Comment #37 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 303438
  --> https://bugs.webkit.org/attachment.cgi?id=303438
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303438&action=review

One more, small set of changes then the patch should be gtg. Thanks for following through on this!

> 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 :(.

> 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.

> 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.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170308/90239ad1/attachment-0001.html>


More information about the webkit-unassigned mailing list