165598 2016-12-08 02:24:13 -0800 [JSC] Module namespace object behaves like immutable prototype exotic object 2016-12-10 03:57:08 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 ysuzuki ysuzuki commit-queue keith_miller mark.lam msaboff saam oldest_to_newest 1257674 0 ysuzuki 2016-12-08 02:24:13 -0800 https://github.com/tc39/ecma262/commit/13906140a Immutable prototype exotic object https://tc39.github.io/ecma262/#sec-immutable-prototype-exotic-objects-setprototypeof-v Module namespace object https://tc39.github.io/ecma262/#sec-module-namespace-exotic-objects-setprototypeof-v 1257675 1 296507 ysuzuki 2016-12-08 02:32:47 -0800 Created attachment 296507 Patch 1257744 2 296507 mark.lam 2016-12-08 10:14:54 -0800 Comment on attachment 296507 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296507&action=review r=me > Source/JavaScriptCore/runtime/JSModuleNamespaceObject.h:38 > + // https://tc39.github.io/ecma262/#sec-module-namespace-exotic-objects-setprototypeof-v I think we can do without this comment here. It's easy enough to trace the addition of this attribute back to the ChangeLog which will give us the url. > JSTests/modules/namespace-set-prototype-of.js:10 > +import * as namespace from "./namespace-set-prototype-of.js" > +import { shouldBe, shouldThrow } from "./resources/assert.js"; > + > +shouldThrow(() => { > + Object.setPrototypeOf(namespace, {}); > +}, `TypeError: Cannot set prototype of immutable prototype object`); > + > +shouldBe(Reflect.setPrototypeOf(namespace, {}), false); > +shouldBe(Reflect.setPrototypeOf(namespace, null), true); > +shouldBe(Object.setPrototypeOf(namespace, null), namespace); You need also test namespace.__proto__. Alternatively, you can add namespace as one of the object types to test in LayoutTests/js/script-tests/prototype-assignment.js. Search for "this.testObject" which tests "{}.__proto__". You can add a case for "thos.testNamespace" and test a namespace object. The benefit of going with prototype-assignment.js is that it is more exhaustive in its testing against what is specified in the spec. 1258676 3 296507 ysuzuki 2016-12-10 03:49:49 -0800 Comment on attachment 296507 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296507&action=review Thanks. >> Source/JavaScriptCore/runtime/JSModuleNamespaceObject.h:38 >> + // https://tc39.github.io/ecma262/#sec-module-namespace-exotic-objects-setprototypeof-v > > I think we can do without this comment here. It's easy enough to trace the addition of this attribute back to the ChangeLog which will give us the url. Dropped. >> JSTests/modules/namespace-set-prototype-of.js:10 >> +shouldBe(Object.setPrototypeOf(namespace, null), namespace); > > You need also test namespace.__proto__. > > Alternatively, you can add namespace as one of the object types to test in LayoutTests/js/script-tests/prototype-assignment.js. Search for "this.testObject" which tests "{}.__proto__". You can add a case for "thos.testNamespace" and test a namespace object. The benefit of going with prototype-assignment.js is that it is more exhaustive in its testing against what is specified in the spec. If we would like to test module namespace object, we need to extend the prototype-assignment.js to <script type="module">. And it prevents us from executing this layout test directly in the JSC shell. So instead, I'll copy the tests from that and put them in JSTests/modules for module namespace object. 1258677 4 ysuzuki 2016-12-10 03:57:08 -0800 Committed r209662: <http://trac.webkit.org/changeset/209662> 296507 2016-12-08 02:32:47 -0800 2016-12-08 10:14:54 -0800 Patch bug-165598-20161208193206.patch text/plain 3590 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjA5NTMzCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBh OGZhYjk4MmZjMzY1ZjA0ZDU0ZmVjMzMwZTRkNWRhNWQ4NjQ4N2RiLi40MTQ3MWEyMWRjY2YwYTYy YWZlYzg0OWExNDEwN2IzMTQyNDBkMmYyIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNSBAQAorMjAxNi0xMi0wOCAgWXVzdWtlIFN1enVraSAgPHV0YXRhbmUudGVhQGdtYWls LmNvbT4KKworICAgICAgICBbSlNDXSBNb2R1bGUgbmFtZXNwYWNlIG9iamVjdCBiZWhhdmVzIGxp a2UgaW1tdXRhYmxlIHByb3RvdHlwZSBleG90aWMgb2JqZWN0CisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjU1OTgKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJbiB0aGUgbGF0ZXN0IEVDTUEyNjIgZHJhZnQs IHRoZSBtb2R1bGUgbmFtZXNwYWNlIG9iamVjdCBiZWhhdmVzIGxpa2UgaW1tdXRhYmxlIHByb3Rv dHlwZSBleG90aWMgb2JqZWN0LgorICAgICAgICBodHRwczovL3RjMzkuZ2l0aHViLmlvL2VjbWEy NjIvI3NlYy1tb2R1bGUtbmFtZXNwYWNlLWV4b3RpYy1vYmplY3RzLXNldHByb3RvdHlwZW9mLXYK KworICAgICAgICAqIHJ1bnRpbWUvSlNNb2R1bGVOYW1lc3BhY2VPYmplY3QuaDoKKwogMjAxNi0x Mi0wNyAgSkYgQmFzdGllbiAgPGpmYmFzdGllbkBhcHBsZS5jb20+CiAKICAgICAgICAgQ2xlYW51 cCBXZWJBc3NlbWJseSdzIFJFVFVSTl9JRl9FWENFUFRJT04KZGlmZiAtLWdpdCBhL1NvdXJjZS9K YXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTTW9kdWxlTmFtZXNwYWNlT2JqZWN0LmggYi9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvcnVudGltZS9KU01vZHVsZU5hbWVzcGFjZU9iamVjdC5oCmluZGV4IDQw NjBiMzk3OWM3MWRjZTcxNzZmZWVjZGJjYTYyMGU0MjFiYjYyYWQuLjI4MDdmZGU1YTRjMzcyOTI1 ZTQ0MmU0ZjJiNTk2ODdkMDMyYmY3MWYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9ydW50aW1lL0pTTW9kdWxlTmFtZXNwYWNlT2JqZWN0LmgKKysrIGIvU291cmNlL0phdmFTY3Jp cHRDb3JlL3J1bnRpbWUvSlNNb2R1bGVOYW1lc3BhY2VPYmplY3QuaApAQCAtMzUsNyArMzUsOCBA QCBjbGFzcyBBYnN0cmFjdE1vZHVsZVJlY29yZDsKIGNsYXNzIEpTTW9kdWxlTmFtZXNwYWNlT2Jq ZWN0IDogcHVibGljIEpTRGVzdHJ1Y3RpYmxlT2JqZWN0IHsKIHB1YmxpYzoKICAgICB0eXBlZGVm IEpTRGVzdHJ1Y3RpYmxlT2JqZWN0IEJhc2U7Ci0gICAgc3RhdGljIGNvbnN0IHVuc2lnbmVkIFN0 cnVjdHVyZUZsYWdzID0gQmFzZTo6U3RydWN0dXJlRmxhZ3MgfCBPdmVycmlkZXNHZXRPd25Qcm9w ZXJ0eVNsb3QgfCBJbnRlcmNlcHRzR2V0T3duUHJvcGVydHlTbG90QnlJbmRleEV2ZW5XaGVuTGVu Z3RoSXNOb3RaZXJvIHwgT3ZlcnJpZGVzR2V0UHJvcGVydHlOYW1lcyB8IEdldE93blByb3BlcnR5 U2xvdElzSW1wdXJlRm9yUHJvcGVydHlBYnNlbmNlOworICAgIC8vIGh0dHBzOi8vdGMzOS5naXRo dWIuaW8vZWNtYTI2Mi8jc2VjLW1vZHVsZS1uYW1lc3BhY2UtZXhvdGljLW9iamVjdHMtc2V0cHJv dG90eXBlb2YtdgorICAgIHN0YXRpYyBjb25zdCB1bnNpZ25lZCBTdHJ1Y3R1cmVGbGFncyA9IEJh c2U6OlN0cnVjdHVyZUZsYWdzIHwgT3ZlcnJpZGVzR2V0T3duUHJvcGVydHlTbG90IHwgSW50ZXJj ZXB0c0dldE93blByb3BlcnR5U2xvdEJ5SW5kZXhFdmVuV2hlbkxlbmd0aElzTm90WmVybyB8IE92 ZXJyaWRlc0dldFByb3BlcnR5TmFtZXMgfCBHZXRPd25Qcm9wZXJ0eVNsb3RJc0ltcHVyZUZvclBy b3BlcnR5QWJzZW5jZSB8IElzSW1tdXRhYmxlUHJvdG90eXBlRXhvdGljT2JqZWN0OwogCiAgICAg c3RhdGljIEpTTW9kdWxlTmFtZXNwYWNlT2JqZWN0KiBjcmVhdGUoRXhlY1N0YXRlKiBleGVjLCBK U0dsb2JhbE9iamVjdCogZ2xvYmFsT2JqZWN0LCBTdHJ1Y3R1cmUqIHN0cnVjdHVyZSwgQWJzdHJh Y3RNb2R1bGVSZWNvcmQqIG1vZHVsZVJlY29yZCwgY29uc3QgSWRlbnRpZmllclNldCYgZXhwb3J0 cykKICAgICB7CmRpZmYgLS1naXQgYS9KU1Rlc3RzL0NoYW5nZUxvZyBiL0pTVGVzdHMvQ2hhbmdl TG9nCmluZGV4IDgzOTIyZWZhOTk1YjZkMWQ0ZjcwMjJmZTdlYzJiZWUxMmRkNDk4NTEuLjg2ODg5 NmI4NGFhNzRiOGI1OGRjMTEyMDQ0ZjA4NGRjOWNjNWRhYWUgMTAwNjQ0Ci0tLSBhL0pTVGVzdHMv Q2hhbmdlTG9nCisrKyBiL0pTVGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTYt MTItMDggIFl1c3VrZSBTdXp1a2kgIDx1dGF0YW5lLnRlYUBnbWFpbC5jb20+CisKKyAgICAgICAg W0pTQ10gTW9kdWxlIG5hbWVzcGFjZSBvYmplY3QgYmVoYXZlcyBsaWtlIGltbXV0YWJsZSBwcm90 b3R5cGUgZXhvdGljIG9iamVjdAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTY1NTk4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgKiBtb2R1bGVzL25hbWVzcGFjZS1zZXQtcHJvdG90eXBlLW9mLmpzOiBBZGRl ZC4KKyAgICAgICAgKHNob3VsZFRocm93KToKKyAgICAgICAgKFR5cGVFcnJvci5DYW5ub3Quc2V0 IHByb3RvdHlwZSk6CisKIDIwMTYtMTItMDcgIEtlaXRoIE1pbGxlciAgPGtlaXRoX21pbGxlckBh cHBsZS5jb20+CiAKICAgICAgICAgQWRkIG1vcmUgbWlzc2luZyB0cml2aWFsIHdhc20gb3BzLgpk aWZmIC0tZ2l0IGEvSlNUZXN0cy9tb2R1bGVzL25hbWVzcGFjZS1zZXQtcHJvdG90eXBlLW9mLmpz IGIvSlNUZXN0cy9tb2R1bGVzL25hbWVzcGFjZS1zZXQtcHJvdG90eXBlLW9mLmpzCm5ldyBmaWxl IG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAuLjJjMWVjZTM3OTkyNDJmODgzNzg0YzRjM2QwZTRmZDg1MzQ3YWQ0MzYKLS0tIC9kZXYvbnVs bAorKysgYi9KU1Rlc3RzL21vZHVsZXMvbmFtZXNwYWNlLXNldC1wcm90b3R5cGUtb2YuanMKQEAg LTAsMCArMSwxMCBAQAoraW1wb3J0ICogYXMgbmFtZXNwYWNlIGZyb20gIi4vbmFtZXNwYWNlLXNl dC1wcm90b3R5cGUtb2YuanMiCitpbXBvcnQgeyBzaG91bGRCZSwgc2hvdWxkVGhyb3cgfSBmcm9t ICIuL3Jlc291cmNlcy9hc3NlcnQuanMiOworCitzaG91bGRUaHJvdygoKSA9PiB7CisgICAgT2Jq ZWN0LnNldFByb3RvdHlwZU9mKG5hbWVzcGFjZSwge30pOworfSwgYFR5cGVFcnJvcjogQ2Fubm90 IHNldCBwcm90b3R5cGUgb2YgaW1tdXRhYmxlIHByb3RvdHlwZSBvYmplY3RgKTsKKworc2hvdWxk QmUoUmVmbGVjdC5zZXRQcm90b3R5cGVPZihuYW1lc3BhY2UsIHt9KSwgZmFsc2UpOworc2hvdWxk QmUoUmVmbGVjdC5zZXRQcm90b3R5cGVPZihuYW1lc3BhY2UsIG51bGwpLCB0cnVlKTsKK3Nob3Vs ZEJlKE9iamVjdC5zZXRQcm90b3R5cGVPZihuYW1lc3BhY2UsIG51bGwpLCBuYW1lc3BhY2UpOwo=