Changeset 206590 in webkit

Timestamp:

Sep 29, 2016, 11:03:23 AM (9 years ago)

Author:

sbarati@apple.com

Message:

We don't properly propagate non-simple-parameter-list when parsing a setter
​https://bugs.webkit.org/show_bug.cgi?id=160483

Reviewed by Joseph Pecoraro.

JSTests:

• test262.yaml:

Source/JavaScriptCore:

• parser/Parser.cpp:

(JSC::Parser<LexerType>::parseFunctionParameters):

LayoutTests:

• js/parser-syntax-check-expected.txt:
• js/script-tests/parser-syntax-check.js:

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/test262.yaml (modified) (1 diff)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/parser-syntax-check-expected.txt (modified) (1 diff)
• LayoutTests/js/script-tests/parser-syntax-check.js (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/parser/Parser.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2016-09-29  Saam Barati  <sbarati@apple.com>
+
+        We don't properly propagate non-simple-parameter-list when parsing a setter
+        https://bugs.webkit.org/show_bug.cgi?id=160483
+
+        Reviewed by Joseph Pecoraro.
+
+        * test262.yaml:
+
 2016-09-28  Saam Barati  <sbarati@apple.com>
 

trunk/JSTests/test262.yaml

@@ -66727 +66727 @@
   cmd: runTest262 :normal, "NoException", ["../../../../../harness/assert.js", "../../../../../harness/sta.js"], [:strict]
 - path: test262/test/language/expressions/object/method-definition/setter-use-strict-with-non-simple-param.js
-  cmd: runTest262 :fail, "SyntaxError", ["../../../../../harness/assert.js", "../../../../../harness/sta.js"], []
+  cmd: runTest262 :normal, "SyntaxError", ["../../../../../harness/assert.js", "../../../../../harness/sta.js"], []
 - path: test262/test/language/expressions/object/method-definition/setter-use-strict-with-non-simple-param.js
-  cmd: runTest262 :fail, "SyntaxError", ["../../../../../harness/assert.js", "../../../../../harness/sta.js"], [:strict]
+  cmd: runTest262 :normal, "SyntaxError", ["../../../../../harness/assert.js", "../../../../../harness/sta.js"], [:strict]
 - path: test262/test/language/expressions/object/method-definition/use-strict-with-non-simple-param.js
   cmd: runTest262 :normal, "SyntaxError", ["../../../../../harness/assert.js", "../../../../../harness/sta.js"], []

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-09-29  Saam Barati  <sbarati@apple.com>
+
+        We don't properly propagate non-simple-parameter-list when parsing a setter
+        https://bugs.webkit.org/show_bug.cgi?id=160483
+
+        Reviewed by Joseph Pecoraro.
+
+        * js/parser-syntax-check-expected.txt:
+        * js/script-tests/parser-syntax-check.js:
+
 2016-09-29  Nan Wang  <n_wang@apple.com>
 

trunk/LayoutTests/js/parser-syntax-check-expected.txt

@@ -1040 +1040 @@
 PASS Invalid: "({set [x (x){}})"
 PASS Invalid: "function f() { ({set [x (x){}}) }"
+PASS Valid:   "({set foo(x) { } })"
+PASS Valid:   "function f() { ({set foo(x) { } }) }"
+PASS Valid:   "({set foo(x) { 'use strict'; } })"
+PASS Valid:   "function f() { ({set foo(x) { 'use strict'; } }) }"
+PASS Invalid: "({set foo(x = 20) { 'use strict'; } })"
+PASS Invalid: "function f() { ({set foo(x = 20) { 'use strict'; } }) }"
+PASS Invalid: "({set foo({x}) { 'use strict'; } })"
+PASS Invalid: "function f() { ({set foo({x}) { 'use strict'; } }) }"
+PASS Invalid: "({set foo([x]) { 'use strict'; } })"
+PASS Invalid: "function f() { ({set foo([x]) { 'use strict'; } }) }"
+PASS Invalid: "({set foo(...x) {} })"
+PASS Invalid: "function f() { ({set foo(...x) {} }) }"
+PASS Valid:   "class Foo { set v(z) { } }"
+PASS Valid:   "function f() { class Foo { set v(z) { } } }"
+PASS Valid:   "class Foo { set v(z) { 'use strict'; } }"
+PASS Valid:   "function f() { class Foo { set v(z) { 'use strict'; } } }"
+PASS Invalid: "class Foo { set v(z = 50) { 'use strict'; } }"
+PASS Invalid: "function f() { class Foo { set v(z = 50) { 'use strict'; } } }"
+PASS Invalid: "class Foo { set v({z}) { 'use strict'; } }"
+PASS Invalid: "function f() { class Foo { set v({z}) { 'use strict'; } } }"
+PASS Invalid: "class Foo { set v([z]) { 'use strict'; } }"
+PASS Invalid: "function f() { class Foo { set v([z]) { 'use strict'; } } }"
+PASS Invalid: "class Foo { set v(...z) { } }"
+PASS Invalid: "function f() { class Foo { set v(...z) { } } }"
+PASS Invalid: "class foo { set y([x, y, x]) { } }"
+PASS Invalid: "function f() { class foo { set y([x, y, x]) { } } }"
+PASS Invalid: "class foo { set y([x, y, {x}]) { } }"
+PASS Invalid: "function f() { class foo { set y([x, y, {x}]) { } } }"
+PASS Invalid: "class foo { set y({x, x}) { } }"
+PASS Invalid: "function f() { class foo { set y({x, x}) { } } }"
+PASS Invalid: "class foo { set y({x, field: {x}}) { } }"
+PASS Invalid: "function f() { class foo { set y({x, field: {x}}) { } } }"
+PASS Valid:   "class foo { set y({x, field: {xx}}) { } }"
+PASS Valid:   "function f() { class foo { set y({x, field: {xx}}) { } } }"
 PASS Invalid: "({[...x]: 1})"
 PASS Invalid: "function f() { ({[...x]: 1}) }"

trunk/LayoutTests/js/script-tests/parser-syntax-check.js

@@ -630 +630 @@
 valid("({set [x](x){}})")
 invalid("({set [x (x){}})")
+valid("({set foo(x) { } })");
+valid("({set foo(x) { 'use strict'; } })");
+invalid("({set foo(x = 20) { 'use strict'; } })");
+invalid("({set foo({x}) { 'use strict'; } })");
+invalid("({set foo([x]) { 'use strict'; } })");
+invalid("({set foo(...x) {} })");
+valid("class Foo { set v(z) { } }");
+valid("class Foo { set v(z) { 'use strict'; } }");
+invalid("class Foo { set v(z = 50) { 'use strict'; } }");
+invalid("class Foo { set v({z}) { 'use strict'; } }");
+invalid("class Foo { set v([z]) { 'use strict'; } }");
+invalid("class Foo { set v(...z) { } }");
+invalid("class foo { set y([x, y, x]) { } }");
+invalid("class foo { set y([x, y, {x}]) { } }");
+invalid("class foo { set y({x, x}) { } }");
+invalid("class foo { set y({x, field: {x}}) { } }");
+valid("class foo { set y({x, field: {xx}}) { } }");
 invalid("({[...x]: 1})")
 invalid("function f({a, a}) {}");

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-09-29  Saam Barati  <sbarati@apple.com>
+
+        We don't properly propagate non-simple-parameter-list when parsing a setter
+        https://bugs.webkit.org/show_bug.cgi?id=160483
+
+        Reviewed by Joseph Pecoraro.
+
+        * parser/Parser.cpp:
+        (JSC::Parser<LexerType>::parseFunctionParameters):
+
 2016-09-28  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/parser/Parser.cpp

@@ -1995 +1995 @@
         failIfTrue(match(CLOSEPAREN), "setter functions must have one parameter");
         const Identifier* duplicateParameter = nullptr;
-        auto parameter = parseDestructuringPattern(context, DestructuringKind::DestructureToParameters, ExportType::NotExported, &duplicateParameter);
+        bool hasDestructuringPattern = false;
+        auto parameter = parseDestructuringPattern(context, DestructuringKind::DestructureToParameters, ExportType::NotExported, &duplicateParameter, &hasDestructuringPattern);
         failIfFalse(parameter, "setter functions must have one parameter");
         auto defaultValue = parseDefaultValueForDestructuringPattern(context);
         propagateError();
-        semanticFailIfTrue(duplicateParameter && defaultValue, "Duplicate parameter '", duplicateParameter->impl(), "' not allowed in function with default parameter values");
+        if (defaultValue || hasDestructuringPattern) {
+            semanticFailIfTrue(duplicateParameter, "Duplicate parameter '", duplicateParameter->impl(), "' not allowed in function with non-simple parameter list");
+            currentScope()->setHasNonSimpleParameterList();
+        }
         context.appendParameter(parameterList, parameter, defaultValue);
         functionInfo.parameterCount = 1;