Diff
Modified: trunk/JSTests/ChangeLog (289165 => 289166)
--- trunk/JSTests/ChangeLog 2022-02-06 00:54:14 UTC (rev 289165)
+++ trunk/JSTests/ChangeLog 2022-02-06 12:34:45 UTC (rev 289166)
@@ -1,3 +1,13 @@
+2022-02-06 Alexey Shvayka <ashva...@apple.com>
+
+ Object literal doesn't properly resolve name clash between an accessor and a constant property
+ https://bugs.webkit.org/show_bug.cgi?id=220574
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/class-static-accessor-name-clash-with-field.js: Added.
+ * stress/object-literal-accessor-name-clash-with-constant.js: Added.
+
2022-02-05 Alexey Shvayka <ashva...@apple.com>
Attempting to [[Set]] JSArray's read-only "length" should throw even with current [[Value]]
Added: trunk/JSTests/stress/class-static-accessor-name-clash-with-field.js (0 => 289166)
--- trunk/JSTests/stress/class-static-accessor-name-clash-with-field.js (rev 0)
+++ trunk/JSTests/stress/class-static-accessor-name-clash-with-field.js 2022-02-06 12:34:45 UTC (rev 289166)
@@ -0,0 +1,76 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error(`Bad value: ${actual}!`);
+}
+
+let objFooSetterCalls = 0;
+let obj;
+
+obj = class {
+ static get foo() { return "fooGetter"; }
+ static foo = "fooValue"
+ static set foo(_val) { objFooSetterCalls++; }
+};
+
+shouldBe(obj.foo, "fooValue");
+obj.foo = 1;
+shouldBe(obj.foo, 1);
+shouldBe(objFooSetterCalls, 0);
+
+
+obj = class {
+ static set foo(_val) { objFooSetterCalls++; }
+ static foo = "fooValue"
+ static get foo() { return "fooGetter"; }
+};
+
+shouldBe(obj.foo, "fooValue");
+obj.foo = 1;
+shouldBe(obj.foo, 1);
+shouldBe(objFooSetterCalls, 0);
+
+
+obj = class {
+ static foo = "fooValue";
+ static get foo() { return "fooGetter"; }
+};
+
+shouldBe(obj.foo, "fooValue");
+obj.foo = 1;
+shouldBe(obj.foo, 1);
+shouldBe(objFooSetterCalls, 0);
+
+
+obj = class {
+ static foo = "fooValue"
+ static set foo(_val) { objFooSetterCalls++; }
+};
+
+shouldBe(obj.foo, "fooValue");
+obj.foo = 1;
+shouldBe(obj.foo, 1);
+shouldBe(objFooSetterCalls, 0);
+
+
+obj = class {
+ static get foo() { return "fooGetter"; }
+ static set foo(_val) { objFooSetterCalls++; }
+ static foo = "fooValue"
+};
+
+shouldBe(obj.foo, "fooValue");
+obj.foo = 1;
+shouldBe(obj.foo, 1);
+shouldBe(objFooSetterCalls, 0);
+
+
+obj = class {
+ static foo = "fooValue"
+ static get foo() { return "fooGetter"; }
+ static set foo(_val) { objFooSetterCalls++; }
+};
+
+shouldBe(obj.foo, "fooValue");
+obj.foo = 1;
+shouldBe(obj.foo, 1);
+shouldBe(objFooSetterCalls, 0);
Added: trunk/JSTests/stress/object-literal-accessor-name-clash-with-constant.js (0 => 289166)
--- trunk/JSTests/stress/object-literal-accessor-name-clash-with-constant.js (rev 0)
+++ trunk/JSTests/stress/object-literal-accessor-name-clash-with-constant.js 2022-02-06 12:34:45 UTC (rev 289166)
@@ -0,0 +1,93 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error(`Bad value: ${actual}!`);
+}
+
+function shouldThrow(func, expectedError) {
+ let errorThrown = false;
+ try {
+ func();
+ } catch (error) {
+ errorThrown = true;
+ if (error.toString() !== expectedError)
+ throw new Error(`Bad error: ${error}!`);
+ }
+ if (!errorThrown)
+ throw new Error(`Didn't throw!`);
+};
+
+let objFooSetterCalls = 0;
+let obj;
+
+const foo = "fooValue";
+
+obj = {
+ get foo() { return "fooGetter"; },
+ foo: "fooValue",
+ set foo(_val) { objFooSetterCalls++; },
+};
+
+shouldBe(obj.foo, undefined);
+obj.foo = 1;
+shouldBe(obj.foo, undefined);
+shouldBe(objFooSetterCalls, 1);
+
+
+obj = {
+ set foo(_val) { objFooSetterCalls++; },
+ foo,
+ get foo() { return "fooGetter"; },
+};
+
+shouldBe(obj.foo, "fooGetter");
+obj.foo = 1;
+shouldThrow(() => { "use strict"; obj.foo = 2; }, "TypeError: Attempted to assign to readonly property.");
+shouldBe(obj.foo, "fooGetter");
+shouldBe(objFooSetterCalls, 1);
+
+
+obj = {
+ foo: "fooValue",
+ get foo() { return "fooGetter"; },
+};
+
+shouldBe(obj.foo, "fooGetter");
+obj.foo = 1;
+shouldThrow(() => { "use strict"; obj.foo = 2; }, "TypeError: Attempted to assign to readonly property.");
+shouldBe(obj.foo, "fooGetter");
+shouldBe(objFooSetterCalls, 1);
+
+
+obj = {
+ foo,
+ set foo(_val) { objFooSetterCalls++; },
+};
+
+shouldBe(obj.foo, undefined);
+obj.foo = 1;
+shouldBe(obj.foo, undefined);
+shouldBe(objFooSetterCalls, 2);
+
+
+obj = {
+ foo: "fooValue",
+ get foo() { return "fooGetter"; },
+ set foo(_val) { objFooSetterCalls++; },
+};
+
+shouldBe(obj.foo, "fooGetter");
+obj.foo = 1;
+shouldBe(obj.foo, "fooGetter");
+shouldBe(objFooSetterCalls, 3);
+
+
+obj = {
+ get foo() { return "fooGetter"; },
+ set foo(_val) { objFooSetterCalls++; },
+ foo,
+};
+
+shouldBe(obj.foo, "fooValue");
+obj.foo = 1;
+shouldBe(obj.foo, 1);
+shouldBe(objFooSetterCalls, 3);
Modified: trunk/LayoutTests/ChangeLog (289165 => 289166)
--- trunk/LayoutTests/ChangeLog 2022-02-06 00:54:14 UTC (rev 289165)
+++ trunk/LayoutTests/ChangeLog 2022-02-06 12:34:45 UTC (rev 289166)
@@ -1,3 +1,15 @@
+2022-02-06 Alexey Shvayka <ashva...@apple.com>
+
+ Object literal doesn't properly resolve name clash between an accessor and a constant property
+ https://bugs.webkit.org/show_bug.cgi?id=220574
+
+ Reviewed by Yusuke Suzuki.
+
+ Adjusted test now passes on V8 and SpiderMonkey as well.
+
+ * js/class-syntax-method-names-expected.txt:
+ * js/script-tests/class-syntax-method-names.js:
+
2022-02-05 Chris Dumez <cdu...@apple.com>
Resync web-platform-tests/dom from upstream
Modified: trunk/LayoutTests/js/class-syntax-method-names-expected.txt (289165 => 289166)
--- trunk/LayoutTests/js/class-syntax-method-names-expected.txt 2022-02-06 00:54:14 UTC (rev 289165)
+++ trunk/LayoutTests/js/class-syntax-method-names-expected.txt 2022-02-06 12:34:45 UTC (rev 289166)
@@ -87,8 +87,8 @@
PASS setterValue = undefined; class A { a() { return 423 } set a(x) { setterValue = x } }; (new A).a = 424; setterValue is 424
PASS setterValue = undefined; class A { set a(x) { setterValue = x } a() { return 425 } }; (new A).a() is 425
PASS setterValue = undefined; class A { get foo() { return 426 } set foo(x) { setterValue = x; } }; a = new A; a.foo = a.foo; setterValue is 426
-PASS class A { get foo() { } foo() { } set foo(x) { } }; valueTypes((new A).__proto__, 'foo') is ['value']
-PASS class A { set foo(x) { } foo() { } get foo() { } }; valueTypes((new A).__proto__, 'foo') is ['value']
+PASS class A { get foo() { } foo() { } set foo(x) { } }; valueTypes((new A).__proto__, 'foo') is ['get', 'set']
+PASS class A { set foo(x) { } foo() { } get foo() { } }; valueTypes((new A).__proto__, 'foo') is ['get', 'set']
PASS class A { foo() { } get foo() { } set foo(x) { } }; valueTypes((new A).__proto__, 'foo') is ['get', 'set']
PASS class A { foo() { } set foo(x) { } get foo() { } }; valueTypes((new A).__proto__, 'foo') is ['get', 'set']
PASS class A { get foo() { } set foo(x) { } foo() { } }; valueTypes((new A).__proto__, 'foo') is ['value']
@@ -108,8 +108,8 @@
PASS setterValue = undefined; class A { static a() { return 523 } static set a(x) { setterValue = x } }; A.a = 524; setterValue is 524
PASS setterValue = undefined; class A { static set a(x) { setterValue = x } static a() { return 525 } }; A.a() is 525
PASS setterValue = undefined; class A { static get foo() { return 526 } static set foo(x) { setterValue = x; } }; A.foo = A.foo; setterValue is 526
-PASS class A { static get foo() { } static foo() { } static set foo(x) { } }; valueTypes(A, 'foo') is ['value']
-PASS class A { static set foo(x) { } static foo() { } static get foo() { } }; valueTypes(A, 'foo') is ['value']
+PASS class A { static get foo() { } static foo() { } static set foo(x) { } }; valueTypes(A, 'foo') is ['get', 'set']
+PASS class A { static set foo(x) { } static foo() { } static get foo() { } }; valueTypes(A, 'foo') is ['get', 'set']
PASS class A { static foo() { } static get foo() { } static set foo(x) { } }; valueTypes(A, 'foo') is ['get', 'set']
PASS class A { static foo() { } static set foo(x) { } static get foo() { } }; valueTypes(A, 'foo') is ['get', 'set']
PASS class A { static get foo() { } static set foo(x) { } static foo() { } }; valueTypes(A, 'foo') is ['value']
Modified: trunk/LayoutTests/js/script-tests/class-syntax-method-names.js (289165 => 289166)
--- trunk/LayoutTests/js/script-tests/class-syntax-method-names.js 2022-02-06 00:54:14 UTC (rev 289165)
+++ trunk/LayoutTests/js/script-tests/class-syntax-method-names.js 2022-02-06 12:34:45 UTC (rev 289166)
@@ -95,8 +95,8 @@
descriptor = Object.getOwnPropertyDescriptor(object, proeprtyName);
return ['value', 'get', 'set'].filter(function (name) { return name in descriptor; });
}
-shouldBe("class A { get foo() { } foo() { } set foo(x) { } }; valueTypes((new A).__proto__, 'foo')", "['value']");
-shouldBe("class A { set foo(x) { } foo() { } get foo() { } }; valueTypes((new A).__proto__, 'foo')", "['value']");
+shouldBe("class A { get foo() { } foo() { } set foo(x) { } }; valueTypes((new A).__proto__, 'foo')", "['get', 'set']");
+shouldBe("class A { set foo(x) { } foo() { } get foo() { } }; valueTypes((new A).__proto__, 'foo')", "['get', 'set']");
shouldBe("class A { foo() { } get foo() { } set foo(x) { } }; valueTypes((new A).__proto__, 'foo')", "['get', 'set']");
shouldBe("class A { foo() { } set foo(x) { } get foo() { } }; valueTypes((new A).__proto__, 'foo')", "['get', 'set']");
shouldBe("class A { get foo() { } set foo(x) { } foo() { } }; valueTypes((new A).__proto__, 'foo')", "['value']");
@@ -117,8 +117,8 @@
shouldBe("setterValue = undefined; class A { static a() { return 523 } static set a(x) { setterValue = x } }; A.a = 524; setterValue", "524");
shouldBe("setterValue = undefined; class A { static set a(x) { setterValue = x } static a() { return 525 } }; A.a()", "525");
shouldBe("setterValue = undefined; class A { static get foo() { return 526 } static set foo(x) { setterValue = x; } }; A.foo = A.foo; setterValue", "526");
-shouldBe("class A { static get foo() { } static foo() { } static set foo(x) { } }; valueTypes(A, 'foo')", "['value']");
-shouldBe("class A { static set foo(x) { } static foo() { } static get foo() { } }; valueTypes(A, 'foo')", "['value']");
+shouldBe("class A { static get foo() { } static foo() { } static set foo(x) { } }; valueTypes(A, 'foo')", "['get', 'set']");
+shouldBe("class A { static set foo(x) { } static foo() { } static get foo() { } }; valueTypes(A, 'foo')", "['get', 'set']");
shouldBe("class A { static foo() { } static get foo() { } static set foo(x) { } }; valueTypes(A, 'foo')", "['get', 'set']");
shouldBe("class A { static foo() { } static set foo(x) { } static get foo() { } }; valueTypes(A, 'foo')", "['get', 'set']");
shouldBe("class A { static get foo() { } static set foo(x) { } static foo() { } }; valueTypes(A, 'foo')", "['value']");
Modified: trunk/Source/_javascript_Core/ChangeLog (289165 => 289166)
--- trunk/Source/_javascript_Core/ChangeLog 2022-02-06 00:54:14 UTC (rev 289165)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-02-06 12:34:45 UTC (rev 289166)
@@ -1,3 +1,32 @@
+2022-02-06 Alexey Shvayka <ashva...@apple.com>
+
+ Object literal doesn't properly resolve name clash between an accessor and a constant property
+ https://bugs.webkit.org/show_bug.cgi?id=220574
+
+ Reviewed by Yusuke Suzuki.
+
+ The spec [1] calls [[DefineOwnProperty]] for every property node, whether it's a
+ getter, a setter, or a value. JSC attempts to reduce emitted bytecodes by setting
+ up a getter and a setter at once.
+
+ However, there is a slower path that exactly matches the spec, which was called only
+ if a spread syntax or a computed property was encountered. With this patch, the slower
+ path is also taken in case of a constant property (including a shorthand) with the
+ same name as an accessor.
+
+ That causes an incomplete accessor descriptor to correctly overwrite the existing
+ data one, which aligns JSC with V8 and SpiderMonkey.
+
+ This bug doesn't exist for static class fields and accessors because initialization
+ of class fields is deferred [2] and they always overwrite eponymous static methods /
+ accessors, no matter the order in source code. No reproduction for private elements either.
+
+ [1]: https://tc39.es/ecma262/#sec-runtime-semantics-methoddefinitionevaluation (step 11 of "get", step 10 of "set")
+ [2]: https://tc39.es/ecma262/#sec-runtime-semantics-classdefinitionevaluation (step 31.a)
+
+ * bytecompiler/NodesCodegen.cpp:
+ (JSC::PropertyListNode::emitBytecode):
+
2022-02-05 Alexey Shvayka <ashva...@apple.com>
Attempting to [[Set]] JSArray's read-only "length" should throw even with current [[Value]]
Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (289165 => 289166)
--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2022-02-06 00:54:14 UTC (rev 289165)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp 2022-02-06 12:34:45 UTC (rev 289166)
@@ -661,7 +661,8 @@
// Were there any get/set properties?
if (p) {
// Build a list of getter/setter pairs to try to put them at the same time. If we encounter
- // a computed property or a spread, just emit everything as that may override previous values.
+ // a constant property by the same name as accessor or a computed property or a spread,
+ // just emit everything as that may override previous values.
bool canOverrideProperties = false;
GetterSetterMap instanceMap;
@@ -675,12 +676,17 @@
break;
}
- if (node->m_type & PropertyNode::Constant)
+ GetterSetterMap& map = node->isStaticClassProperty() ? staticMap : instanceMap;
+ if (node->m_type & PropertyNode::Constant) {
+ if (map.contains(node->name()->impl())) {
+ canOverrideProperties = true;
+ break;
+ }
continue;
+ }
// Duplicates are possible.
GetterSetterPair pair(node, static_cast<PropertyNode*>(nullptr));
- GetterSetterMap& map = node->isStaticClassProperty() ? staticMap : instanceMap;
GetterSetterMap::AddResult result = map.add(node->name()->impl(), pair);
auto& resultPair = result.iterator->value;
if (!result.isNewEntry) {
