1222446 - Add some module tests

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco) (PTO until 3rd April)]

This bug is to commit a bunch of module test cases I have sitting in my patch queue.

Comment 1

[Jon Coppeard (:jonco) (PTO until 3rd April)]

Attachment: test-ambiguous-exports

Ambiguous exports occur when we use |export *| to export the same name from more than one module.  Import or export of such a name is an error, but a module namespace ignores ambiguous exports.

Comment 2

[Jon Coppeard (:jonco) (PTO until 3rd April)]

Attachment: test-assign-import

Test that assigning to or deleting the various types of imports is an error.

Comment 3

[Jon Coppeard (:jonco) (PTO until 3rd April)]

Attachment: test-many-import-exports

Check that nothing goes awry when we have > 1000 imports or exports.

Comment 4

[Jon Coppeard (:jonco) (PTO until 3rd April)]

Attachment: test-cyclic-imports

> Just checking: imported bindings can never be uninitialized (thus triggering TDZ) in the target module at this point, right?

Unfortunately yes they can, and this is the case - cyclic imported variable dependencies.

Comment 5

[Shu-yu Guo [:shu]]

Comment on attachment 8684231 [details] [diff] [review]
test-ambiguous-exports

Review of attachment 8684231 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit-test/tests/modules/ambiguous-star-export.js
@@ +40,5 @@
> +// Check that namespace objects include only non-ambiguous names.
> +let m = parseModule("import * as ns from 'c'; ns;");
> +m.declarationInstantiation();
> +let ns = m.evaluation();
> +let names = Object.keys(ns).sort();

sort() doesn't seem important here since your tests below use 'in'

Comment 6

[Shu-yu Guo [:shu]]

(In reply to Jon Coppeard (:jonco) from comment #4)
> Created attachment 8684294 [details] [diff] [review]
> test-cyclic-imports
> 
> > Just checking: imported bindings can never be uninitialized (thus triggering TDZ) in the target module at this point, right?
> 

What patch review did I say that in? I made that comment because there wasn't code that correctly dealt with imported bindings being uninitialized.

Comment 7

[Shu-yu Guo [:shu]]

Comment on attachment 8684234 [details] [diff] [review]
test-many-import-exports

Review of attachment 8684234 [details] [diff] [review]:
-----------------------------------------------------------------

General note for future tests, using template strings over string concatenation would make the building up of the module source texts more readable.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8ff488cc4d44
https://hg.mozilla.org/integration/mozilla-inbound/rev/be11a4215c0b
https://hg.mozilla.org/integration/mozilla-inbound/rev/d92cfc42f114
https://hg.mozilla.org/integration/mozilla-inbound/rev/012f6c8917e5

Comment 9

[Jon Coppeard (:jonco) (PTO until 3rd April)]

(In reply to Shu-yu Guo [:shu] from comment #6)
> What patch review did I say that in? I made that comment because there
> wasn't code that correctly dealt with imported bindings being uninitialized.

Right, this was in bug 1219288.  But it's a situation that wasn't previously exercised by our tests so I thought it was worth adding a test case right away.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8ff488cc4d44
https://hg.mozilla.org/mozilla-central/rev/be11a4215c0b
https://hg.mozilla.org/mozilla-central/rev/d92cfc42f114
https://hg.mozilla.org/mozilla-central/rev/012f6c8917e5