Bug 151369

Summary: There is a bug when default parameter values are mixed with destructuring parameter values
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, keith_miller, mark.lam, msaboff
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 151417, 151419    
Bug Blocks:    
Attachments:
Description Flags
patch ggaren: review+

Saam Barati
Reported 2015-11-17 16:35:59 PST
``` function foo(a = function() { return b; }, {b}) { b = 50; return a(); } print(foo(undefined, {b: 34})); ``` this prints 34 instead of 50. Basically, the problem is that we mark destructuring parameters as "var"s in the function. This causes us to create an extra lexical environment register on top of the one used for default parameter values. We essentially create two "b"s in the above function. The body of the function only has access to the top-most "b" and the default parameter value function only has access to the bottom-most "b".
Attachments
patch (12.84 KB, patch)
2015-11-18 12:04 PST, Saam Barati 		ggaren: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Saam Barati
Comment 1 2015-11-18 12:04:06 PST
Created attachment 265769 [details] patch
Geoffrey Garen
Comment 2 2015-11-18 12:21:44 PST
Comment on attachment 265769 [details] patch r=me
Mark Lam
Comment 3 2015-11-18 12:22:39 PST
Comment on attachment 265769 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=265769&action=review > Source/JavaScriptCore/ChangeLog:12 > + incorrectly transofrm this program: typo: transofrm ==> transform. > Source/JavaScriptCore/ChangeLog:15 > + ```function foo(a = function() { b = 40; }, {b}) { a(); return b; }``` > + into > + ```function foo(a = function() { b = 40; }, {b}) { var b; a(); return b; }``` nit: It would be nice if you can provide some example values and show how that propagates through to produce a different and wrong result. Perhaps this is already straightforward to anyone who already is well versed in ES6, but it would certainly highlight the nuance of the bug a little clearer for anyone else who is not. On second thought, maybe the test case you added should be sufficient to illustrate that. I'll leave it up to your discretion. > Source/JavaScriptCore/ChangeLog:17 > + there whould only be one. typo: whould ==> should.
Saam Barati
Comment 4 2015-11-18 14:01:39 PST
landed in: http://trac.webkit.org/changeset/192586
WebKit Commit Bot
Comment 5 2015-11-18 15:51:51 PST
Re-opened since this is blocked by bug 151417
Saam Barati
Comment 6 2015-11-18 17:28:42 PST
landed again: http://trac.webkit.org/changeset/192603
Note You need to log in before you can comment on or make changes to this bug.