Bug 151369 - There is a bug when default parameter values are mixed with destructuring parameter values
Summary: There is a bug when default parameter values are mixed with destructuring par...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on: 151417 151419
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-17 16:35 PST by Saam Barati
Modified: 2015-11-18 17:28 PST (History)
4 users (show)

See Also:


Attachments
patch (12.84 KB, patch)
2015-11-18 12:04 PST, Saam Barati
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 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".
Comment 1 Saam Barati 2015-11-18 12:04:06 PST
Created attachment 265769 [details]
patch
Comment 2 Geoffrey Garen 2015-11-18 12:21:44 PST
Comment on attachment 265769 [details]
patch

r=me
Comment 3 Mark Lam 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.
Comment 4 Saam Barati 2015-11-18 14:01:39 PST
landed in:
http://trac.webkit.org/changeset/192586
Comment 5 WebKit Commit Bot 2015-11-18 15:51:51 PST
Re-opened since this is blocked by bug 151417
Comment 6 Saam Barati 2015-11-18 17:28:42 PST
landed again:
http://trac.webkit.org/changeset/192603