Bug 165598 – [JSC] Module namespace object behaves like immutable prototype exotic object

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

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

[Yusuke Suzuki]

Created attachment 296507 [details]
Patch

[Mark Lam]

Comment on attachment 296507 [details]
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.

[Yusuke Suzuki]

Comment on attachment 296507 [details]
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.

[Yusuke Suzuki]

Committed r209662: <http://trac.webkit.org/changeset/209662>