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

[bugzilla-daemon at webkit.org]

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

--- Comment #71 from Saam Barati <sbarati at apple.com> ---
(In reply to Caio Lima from comment #70)
> Comment on attachment 310597 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=310597&action=review
> 
> >> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:78
> >> +ENABLE_ESNEXT_OBJ_REST_SPREAD = ENABLE_ESNEXT_OBJ_REST_SPREAD;
> > 
> > Lets make it a runtime option, not a compile time one.
> 
> ok
> 
> >> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:4071
> >> +        generator.emitLoad(excludedIndex.get(), jsNumber(0));
> > 
> > Please don't do this. You should just build the Set in bytecode. There is no need to first build an array, and then have your builtin convert the array to a set.
> 
> I also think it's better use JSSet, but there is a thing that I'm worried:
> 
> If our implementation uses JSSet directly and then user overrides the
> Set.prototype.add, the rest semantics will be broken.
> 
This is an issue in all our builtins. They take care yo use private identifiers. You’ll need to do the same here, and make sure SetPrototye sets up your private identifier to the function you want. 
> like:
> 
> ```
> Set.prototype.add = () => false;
> 
> let {a, b, ...r} = {a: 1, b: 2, c: 3, d: 4};
> 
> // here "r" will contain "a" and "b", and it will be wrong.
> 
> ```
> 
> Do you think it is right?
> 
> >> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:-4076
> >> -            RefPtr<RegisterID> propertyName = generator.emitNode(target.propertyExpression);
> > 
> > Can you add an assertion here of what the bindingType is? Also, I think it's worth adding an assertion that this is the last thing in the list, otherwise this algorithm is broken.
> 
> ok
> 
> >> Source/JavaScriptCore/parser/Parser.cpp:980
> >> +        return 0;
> > 
> > bad indentation
> 
> Oops.
> 
> >> Source/JavaScriptCore/parser/Parser.cpp:1070
> >> +            if (UNLIKELY(match(DOTDOTDOT))) {
> > 
> > Is this syntactically required to be the last element in the destrucruting clause?
> > Also, no need for UNLIKELY, we're just going to negatively impact parsing code that uses this.
> 
> Yes. I will remove the UNLIKELY here.

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