Skip to content

Commit 06f736e

Browse files
shvaikaleshshvaikalesh
committed
Object.prototype.toString is not spec-perfect
https://bugs.webkit.org/show_bug.cgi?id=199138 Reviewed by Darin Adler and Keith Miller. JSTests: * ChakraCore.yaml: Skip a test as `global` now has @@toStringTag. * ChakraCore/test/LetConst/delete.baseline: Removed. * stress/internal-promise-constructor-not-confusing.js: Use @ispromise. * stress/object-get-own-property-symbols.js: Adjust test as `global` now has @@toStringTag. * test262/expectations.yaml: Mark 6 test cases as passing. LayoutTests/imported/w3c: * web-platform-tests/WebIDL/ecmascript-binding/class-string-interface.any-expected.txt: * web-platform-tests/WebIDL/ecmascript-binding/class-string-interface.any.worker-expected.txt: * web-platform-tests/WebIDL/ecmascript-binding/class-string-iterator-prototype-object.any-expected.txt: * web-platform-tests/WebIDL/ecmascript-binding/class-string-iterator-prototype-object.any.worker-expected.txt: Source/JavaScriptCore: Before ES6, Object.prototype.toString relied only on internal [[Class]] slot. Starting with ES6, Object.prototype.toString checks for a handful of internal slots, mimicing [[Class]], to ensure backwards compatibility for pre-ES6 instances. Newly-added built-ins provide @@toStringTag for the method to use. Before this change, Object.prototype.toString in JSC relied on className() a.k.a [[Class]] for all instances. For (almost all) new built-ins, it was overriden by toStringName() returning "Object", while @@toStringTag was set to correct value. This is quite an error-prone approach and observable spec discrepancy if @@toStringTag is deleted or set to a non-string. This change eliminates the above-mentioned discrepancy and fixes Object.prototype.toString to return "[object Function]" for callable Proxy objects, aligning JSC with the spec [1], V8, and SpiderMonkey. For Object.prototype.toString to work through DebuggerScope and JSProxy, we perform all checks in JSObject::toStringName(). Given that isArray() may throw a TypeError [2], we invoke toStringName() before @@toStringTag lookup to accomodate revoked Proxy case. Also, this patch defines @@toStringTag for WebAssembly namespace object (to match Chrome), JSC shell, and ConsoleObject. [1]: https://tc39.es/ecma262/#sec-object.prototype.tostring [2]: https://tc39.es/ecma262/#sec-isarray (step 3.a) * jsc.cpp: * runtime/BigIntObject.cpp: (JSC::BigIntObject::toStringName): Deleted. * runtime/BigIntObject.h: * runtime/BooleanObject.cpp: (JSC::BooleanObject::toStringName): * runtime/BooleanObject.h: * runtime/ConsoleObject.cpp: (JSC::ConsoleObject::finishCreation): * runtime/DateInstance.cpp: (JSC::DateInstance::toStringName): * runtime/DateInstance.h: * runtime/ErrorInstance.cpp: (JSC::ErrorInstance::toStringName): * runtime/ErrorInstance.h: * runtime/JSArrayBufferView.cpp: (JSC::JSArrayBufferView::toStringName): Deleted. * runtime/JSArrayBufferView.h: * runtime/JSMap.cpp: (JSC::JSMap::toStringName): Deleted. * runtime/JSMap.h: * runtime/JSObject.cpp: (JSC::JSObject::toStringName): * runtime/JSSet.cpp: (JSC::JSSet::toStringName): Deleted. * runtime/JSSet.h: * runtime/JSWeakMap.cpp: (JSC::JSWeakMap::toStringName): Deleted. * runtime/JSWeakMap.h: * runtime/JSWeakObjectRef.cpp: (JSC::JSWeakObjectRef::toStringName): Deleted. * runtime/JSWeakObjectRef.h: * runtime/JSWeakSet.cpp: (JSC::JSWeakSet::toStringName): Deleted. * runtime/JSWeakSet.h: * runtime/NumberObject.cpp: (JSC::NumberObject::toStringName): * runtime/NumberObject.h: * runtime/ObjectPrototype.cpp: (JSC::objectProtoFuncToString): * runtime/ProxyObject.cpp: (JSC::ProxyObject::toStringName): Deleted. * runtime/ProxyObject.h: * runtime/RegExpObject.cpp: (JSC::RegExpObject::toStringName): * runtime/RegExpObject.h: * runtime/StringObject.cpp: (JSC::StringObject::toStringName): * runtime/StringObject.h: * runtime/SymbolObject.cpp: (JSC::SymbolObject::toStringName): Deleted. * runtime/SymbolObject.h: * wasm/js/JSWebAssembly.cpp: (JSC::JSWebAssembly::finishCreation): Source/WebCore: This patch defines @@toStringTag symbols for all WebIDL prototypes, including interfaces that are not exposed, as required by the spec [1]. With updated JSObject::toStringName() and @@toStringTag symbols added in r260992, className() and toStringName() methods of JSDOMConstructorBase can be safely removed. [1]: https://heycam.github.io/webidl/#dfn-class-string Tests: imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-interface.any.js imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-iterator-prototype-object.any.js * bindings/js/JSDOMConstructorBase.cpp: (WebCore::JSDOMConstructorBase::className): Deleted. (WebCore::JSDOMConstructorBase::toStringName): Deleted. * bindings/js/JSDOMConstructorBase.h: * bindings/scripts/CodeGeneratorJS.pm: (GenerateImplementation): (GeneratePrototypeDeclaration): * bindings/scripts/test/JS/JSTestGlobalObject.cpp: (WebCore::JSTestGlobalObjectPrototype::finishCreation): Tools: * TestWebKitAPI/Tests/JavaScriptCore/glib/TestJSC.cpp: Canonical link: https://commits.webkit.org/224332@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@261159 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 3af1927 commit 06f736e

54 files changed

Lines changed: 257 additions & 158 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎JSTests/ChakraCore.yaml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,8 @@
817817
- path: ChakraCore/test/LetConst/defer5.js
818818
cmd: runChakra :baseline, "ReferenceError", "defer5.baseline-jsc", []
819819
- path: ChakraCore/test/LetConst/delete.js
820-
cmd: runChakra :baseline, "NoException", "delete.baseline", []
820+
# The test should use Reflect.ownKeys as `this` may have symbol properties.
821+
cmd: runChakra :skipDueToOutdatedOrBadTest, "NoException", "", []
821822
- path: ChakraCore/test/LetConst/dop.js
822823
cmd: runChakra :baseline, "NoException", "dop.baseline-jsc", []
823824
- path: ChakraCore/test/LetConst/dop1.js

‎JSTests/ChakraCore/test/LetConst/delete.baseline‎

Lines changed: 0 additions & 4 deletions
This file was deleted.

‎JSTests/ChangeLog‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
2020-05-05 Alexey Shvayka <[email protected]>
2+
3+
Object.prototype.toString is not spec-perfect
4+
https://bugs.webkit.org/show_bug.cgi?id=199138
5+
6+
Reviewed by Darin Adler and Keith Miller.
7+
8+
* ChakraCore.yaml: Skip a test as `global` now has @@toStringTag.
9+
* ChakraCore/test/LetConst/delete.baseline: Removed.
10+
* stress/internal-promise-constructor-not-confusing.js: Use @isPromise.
11+
* stress/object-get-own-property-symbols.js: Adjust test as `global` now has @@toStringTag.
12+
* test262/expectations.yaml: Mark 6 test cases as passing.
13+
114
2020-05-04 Yusuke Suzuki <[email protected]>
215

316
[JSC] Implement BigInt.asIntN and BigInt.asUintN

‎JSTests/stress/internal-promise-constructor-not-confusing.js‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,26 @@ var InternalPromise = $vm.createBuiltin(`(function () {
77
return @InternalPromise;
88
})`)();
99

10+
var isPromise = $vm.createBuiltin(`(function (p) {
11+
return @isPromise(p);
12+
})`);
13+
1014
function DerivedPromise() { }
1115

1216
for (var i = 0; i < 1e4; ++i) {
1317
var promise = Reflect.construct(InternalPromise, [function (resolve) { resolve(42); }], DerivedPromise);
14-
shouldBe(promise.toString(), `[object InternalPromise]`);
18+
shouldBe(promise.toString(), `[object Object]`);
19+
shouldBe(isPromise(promise), true);
1520
}
1621
drainMicrotasks();
1722
for (var i = 0; i < 1e4; ++i) {
1823
var promise = Reflect.construct(Promise, [function (resolve) { resolve(42); }], DerivedPromise);
19-
shouldBe(promise.toString(), `[object Promise]`);
24+
shouldBe(promise.toString(), `[object Object]`);
25+
shouldBe(isPromise(promise), true);
2026
}
2127
drainMicrotasks();
2228
for (var i = 0; i < 1e4; ++i) {
2329
var promise = Reflect.construct(InternalPromise, [function (resolve) { resolve(42); }], DerivedPromise);
24-
shouldBe(promise.toString(), `[object InternalPromise]`);
30+
shouldBe(promise.toString(), `[object Object]`);
31+
shouldBe(isPromise(promise), true);
2532
}

‎JSTests/stress/object-get-own-property-symbols.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
var global = (Function("return this")());
44

55
// private names for privileged code should not be exposed.
6-
if (Object.getOwnPropertySymbols(global).length !== 0)
7-
throw "Error: bad value " + Object.getOwnPropertySymbols(global).length;
6+
var globalSymbols = Object.getOwnPropertySymbols(global).filter(s => s !== Symbol.toStringTag);
7+
if (globalSymbols.length !== 0)
8+
throw "Error: bad value " + globalSymbols.length;
89

910
var object = {};
1011
var symbol = Symbol("Cocoa");

‎JSTests/test262/expectations.yaml‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -970,15 +970,6 @@ test/built-ins/Object/internals/DefineOwnProperty/consistent-value-regexp-dollar
970970
test/built-ins/Object/keys/order-after-define-property.js:
971971
default: 'Test262Error: Expected [b, a] and [a, b] to have the same contents. '
972972
strict mode: 'Test262Error: Expected [b, a] and [a, b] to have the same contents. '
973-
test/built-ins/Object/prototype/toString/proxy-function.js:
974-
default: 'Test262Error: function proxy Expected SameValue(«[object Object]», «[object Function]») to be true'
975-
strict mode: 'Test262Error: function proxy Expected SameValue(«[object Object]», «[object Function]») to be true'
976-
test/built-ins/Object/prototype/toString/symbol-tag-non-str-builtin.js:
977-
default: 'Test262Error: Expected SameValue(«[object Math]», «[object Object]») to be true'
978-
strict mode: 'Test262Error: Expected SameValue(«[object Math]», «[object Object]») to be true'
979-
test/built-ins/Object/prototype/toString/symbol-tag-non-str-proxy-function.js:
980-
default: 'Test262Error: generator function proxy without Symbol.toStringTag Expected SameValue(«[object Object]», «[object Function]») to be true'
981-
strict mode: 'Test262Error: generator function proxy without Symbol.toStringTag Expected SameValue(«[object Object]», «[object Function]») to be true'
982973
test/built-ins/Object/values/order-after-define-property.js:
983974
default: 'Test262Error: Expected [b, a] and [a, b] to have the same contents. '
984975
strict mode: 'Test262Error: Expected [b, a] and [a, b] to have the same contents. '

‎LayoutTests/imported/w3c/ChangeLog‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
2020-05-05 Alexey Shvayka <[email protected]>
2+
3+
Object.prototype.toString is not spec-perfect
4+
https://bugs.webkit.org/show_bug.cgi?id=199138
5+
6+
Reviewed by Darin Adler and Keith Miller.
7+
8+
* web-platform-tests/WebIDL/ecmascript-binding/class-string-interface.any-expected.txt:
9+
* web-platform-tests/WebIDL/ecmascript-binding/class-string-interface.any.worker-expected.txt:
10+
* web-platform-tests/WebIDL/ecmascript-binding/class-string-iterator-prototype-object.any-expected.txt:
11+
* web-platform-tests/WebIDL/ecmascript-binding/class-string-iterator-prototype-object.any.worker-expected.txt:
12+
113
2020-05-04 Yusuke Suzuki <[email protected]>
214

315
[JSC] Add @@toStringTag to WebAssembly.Global

‎LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-interface.any-expected.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ PASS Object.prototype.toString applied to the prototype
55
PASS Object.prototype.toString applied to an instance
66
PASS Object.prototype.toString applied after modifying the prototype's @@toStringTag
77
PASS Object.prototype.toString applied to the instance after modifying the instance's @@toStringTag
8-
FAIL Object.prototype.toString applied to a null-prototype instance assert_equals: expected "[object Object]" but got "[object Blob]"
9-
FAIL Object.prototype.toString applied after deleting @@toStringTag assert_equals: prototype expected "[object Object]" but got "[object Blob]"
8+
PASS Object.prototype.toString applied to a null-prototype instance
9+
PASS Object.prototype.toString applied after deleting @@toStringTag
1010

‎LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-interface.any.worker-expected.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ PASS Object.prototype.toString applied to the prototype
55
PASS Object.prototype.toString applied to an instance
66
PASS Object.prototype.toString applied after modifying the prototype's @@toStringTag
77
PASS Object.prototype.toString applied to the instance after modifying the instance's @@toStringTag
8-
FAIL Object.prototype.toString applied to a null-prototype instance assert_equals: expected "[object Object]" but got "[object Blob]"
9-
FAIL Object.prototype.toString applied after deleting @@toStringTag assert_equals: prototype expected "[object Object]" but got "[object Blob]"
8+
PASS Object.prototype.toString applied to a null-prototype instance
9+
PASS Object.prototype.toString applied after deleting @@toStringTag
1010

‎LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/class-string-iterator-prototype-object.any-expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,5 @@ PASS @@toStringTag exists with the appropriate descriptor
33
PASS Object.prototype.toString
44
PASS Object.prototype.toString applied after modifying @@toStringTag
55
FAIL Object.prototype.toString applied after nulling the prototype assert_equals: expected "[object Object]" but got "[object URLSearchParams Iterator]"
6-
FAIL Object.prototype.toString applied after deleting @@toStringTag assert_equals: prototype expected "[object Object]" but got "[object URLSearchParams Iterator]"
6+
PASS Object.prototype.toString applied after deleting @@toStringTag
77

0 commit comments

Comments
 (0)