Resolve export bindings to their canonical module

quickjs.c

@@ -30895,6 +30895,24 @@ typedef enum JSResolveResultEnum {
     JS_RESOLVE_RES_AMBIGUOUS,
 } JSResolveResultEnum;
 
+/* Canonical (Module Record, BindingName) of a resolved export entry. For
+   `export * as ns from` the binding is the imported module's namespace
+   (marked by JS_ATOM__star_) and does not depend on which module re-exported
+   it, so the same namespace reached through different modules compares
+   equal. */
+static void js_resolved_export_binding(JSModuleDef *m, JSExportEntry *me,
+                                       JSModuleDef **pbinding_m,
+                                       JSAtom *pbinding_name)
+{
+    if (me->local_name == JS_ATOM__star_) {
+        *pbinding_m = m->req_module_entries[me->u.req_module_idx].module;
+        *pbinding_name = JS_ATOM__star_;
+    } else {
+        *pbinding_m = m;
+        *pbinding_name = me->local_name;
+    }
+}
+
 static JSResolveResultEnum js_resolve_export1(JSContext *ctx,
                                               JSModuleDef **pmodule,
                                               JSExportEntry **pme,
@@ -30950,8 +30968,13 @@ static JSResolveResultEnum js_resolve_export1(JSContext *ctx,
                     return ret;
                 } else if (ret == JS_RESOLVE_RES_FOUND) {
                     if (*pme != NULL) {
-                        if (*pmodule != res_m ||
-                            res_me->local_name != (*pme)->local_name) {
+                        JSModuleDef *cur_m, *new_m;
+                        JSAtom cur_name, new_name;
+                        js_resolved_export_binding(*pmodule, *pme,
+                                                   &cur_m, &cur_name);
+                        js_resolved_export_binding(res_m, res_me,
+                                                   &new_m, &new_name);
+                        if (cur_m != new_m || cur_name != new_name) {
                             *pmodule = NULL;
                             *pme = NULL;
                             return JS_RESOLVE_RES_AMBIGUOUS;
@@ -36916,7 +36939,7 @@ static __exception int compute_stack_size(JSContext *ctx,
 
 static int add_module_variables(JSContext *ctx, JSFunctionDef *fd)
 {
-    int i, idx;
+    int i, j, idx;
     JSModuleDef *m = fd->module;
     JSExportEntry *me;
     JSGlobalVar *hf;
@@ -36943,7 +36966,27 @@ static int add_module_variables(JSContext *ctx, JSFunctionDef *fd)
                                         me->local_name);
                 return -1;
             }
-            me->u.local.var_idx = idx;
+            /* ParseModule: an entry of localExportEntries whose local name
+               is an imported bound name is not a binding of this module.
+               Rewrite it into an indirect export through the module the
+               name was imported from, exactly like
+               `export {ie.[[ImportName]]} from "m"` (or, for a namespace
+               import, like `export * as x from "m"`), so that ResolveExport
+               reaches the shared binding instead of a second, spuriously
+               distinct local one. */
+            for(j = 0; j < m->import_entries_count; j++) {
+                JSImportEntry *mi = &m->import_entries[j];
+                if (mi->var_idx == idx) {
+                    JSAtom import_name = JS_DupAtom(ctx, mi->import_name);
+                    JS_FreeAtom(ctx, me->local_name);
+                    me->local_name = import_name;
+                    me->export_type = JS_EXPORT_TYPE_INDIRECT;
+                    me->u.req_module_idx = mi->req_module_idx;
+                    break;
+                }
+            }
+            if (me->export_type == JS_EXPORT_TYPE_LOCAL)
+                me->u.local.var_idx = idx;
         }
     }
     return 0;

test262_errors.txt

@@ -31,10 +31,6 @@ test262/test/language/expressions/in/private-field-invalid-assignment-target.js:
 test262/test/language/expressions/in/private-field-invalid-assignment-target.js:23: strict mode: unexpected error type: Test262: This statement should not be evaluated.
 test262/test/language/expressions/object/computed-property-name-topropertykey-before-value-evaluation.js:31: Test262Error: Expected SameValue(«"bad"», «"ok"») to be true
 test262/test/language/expressions/object/computed-property-name-topropertykey-before-value-evaluation.js:31: strict mode: Test262Error: Expected SameValue(«"bad"», «"ok"») to be true
-test262/test/language/module-code/ambiguous-export-bindings/import-and-export-propagates-binding.js:75: SyntaxError: export 'foo' in module 'test262/test/language/module-code/ambiguous-export-bindings/imp' is ambiguous
-test262/test/language/module-code/ambiguous-export-bindings/namespace-unambiguous-if-export-star-as-from-and-import-star-as-and-export.js:74: SyntaxError: export 'foo' in module 'test262/test/language/module-code/ambiguous-export-bindings/nam' is ambiguous
-test262/test/language/module-code/ambiguous-export-bindings/namespace-unambiguous-if-export-star-as-from.js:75: SyntaxError: export 'foo' in module 'test262/test/language/module-code/ambiguous-export-bindings/nam' is ambiguous
-test262/test/language/module-code/ambiguous-export-bindings/namespace-unambiguous-if-import-star-as-and-export.js:74: SyntaxError: export 'foo' in module 'test262/test/language/module-code/ambiguous-export-bindings/nam' is ambiguous
 test262/test/language/module-code/top-level-await/module-graphs-does-not-hang.js:10: TypeError: $DONE() not called
 test262/test/language/module-code/top-level-await/rejection-order.js:20: TypeError: $DONE() not called
 test262/test/language/statements/await-using/initializer-Symbol.asyncDispose-disposed-at-end-of-imported-module.js:11: SyntaxError: exported variable 'resource' does not exist

tests.conf

@@ -16,3 +16,25 @@ tests/fixture_throwing_module.js
 tests/fixture_reexport_source.js
 tests/fixture_reexport_missing.js
 tests/fixture_reexport_direct.js
+tests/fixture_export_base.js
+tests/fixture_export_empty.js
+tests/fixture_export_empty2.js
+tests/fixture_export_from.js
+tests/fixture_export_hub_ambiguous.js
+tests/fixture_export_hub_ns_ambiguous.js
+tests/fixture_export_hub_ns_import_ok.js
+tests/fixture_export_hub_ns_mixed_ok.js
+tests/fixture_export_hub_ns_ok.js
+tests/fixture_export_hub_ok.js
+tests/fixture_export_import_and_export.js
+tests/fixture_export_ns_import_1.js
+tests/fixture_export_ns_import_2.js
+tests/fixture_export_ns_other.js
+tests/fixture_export_ns_star_as_1.js
+tests/fixture_export_ns_star_as_2.js
+tests/fixture_export_other.js
+tests/fixture_export_renamed.js
+tests/fixture_export_hub_ambiguous_use.js
+tests/fixture_export_hub_ns_ambiguous_use.js
+tests/fixture_export_hub_partial.js
+tests/fixture_export_only_second.js

tests/fixture_export_base.js

@@ -0,0 +1,3 @@
+export const foo = 2;
+export let counter = 0;
+export function bump() { counter++; }

tests/fixture_export_empty.js

@@ -0,0 +1 @@
+/* no exports; only ever used as a namespace */

tests/fixture_export_empty2.js

@@ -0,0 +1 @@
+/* a second, distinct empty module */

tests/fixture_export_from.js

@@ -0,0 +1 @@
+export { foo } from "./fixture_export_base.js";

tests/fixture_export_hub_ambiguous.js

@@ -0,0 +1,3 @@
+/* two genuinely different bindings named foo */
+export * from "./fixture_export_base.js";
+export * from "./fixture_export_other.js";

tests/fixture_export_hub_ambiguous_use.js

@@ -0,0 +1,3 @@
+/* importing the ambiguous name by name is the SyntaxError */
+import { foo } from "./fixture_export_hub_ambiguous.js";
+export { foo };

tests/fixture_export_hub_ns_ambiguous.js

@@ -0,0 +1,3 @@
+/* two namespaces of *different* modules */
+export * from "./fixture_export_ns_star_as_1.js";
+export * from "./fixture_export_ns_other.js";

tests/fixture_export_hub_ns_ambiguous_use.js

@@ -0,0 +1,2 @@
+import { foo } from "./fixture_export_hub_ns_ambiguous.js";
+export { foo };

tests/fixture_export_hub_ns_import_ok.js

@@ -0,0 +1,3 @@
+/* ... and through two `import * as` + `export` re-exports */
+export * from "./fixture_export_ns_import_1.js";
+export * from "./fixture_export_ns_import_2.js";

tests/fixture_export_hub_ns_mixed_ok.js

@@ -0,0 +1,3 @@
+/* ... and through one of each */
+export * from "./fixture_export_ns_star_as_1.js";
+export * from "./fixture_export_ns_import_1.js";

tests/fixture_export_hub_ns_ok.js

@@ -0,0 +1,3 @@
+/* the same namespace reached through two `export * as` re-exports */
+export * from "./fixture_export_ns_star_as_1.js";
+export * from "./fixture_export_ns_star_as_2.js";

tests/fixture_export_hub_ok.js

@@ -0,0 +1,3 @@
+/* the same base binding reached two ways: not ambiguous */
+export * from "./fixture_export_from.js";
+export * from "./fixture_export_import_and_export.js";

tests/fixture_export_hub_partial.js

@@ -0,0 +1,4 @@
+/* one ambiguous name next to unambiguous ones */
+export * from "./fixture_export_base.js";
+export * from "./fixture_export_other.js";
+export * from "./fixture_export_only_second.js";

tests/fixture_export_import_and_export.js

@@ -0,0 +1,2 @@
+import { foo } from "./fixture_export_base.js";
+export { foo };

tests/fixture_export_ns_import_1.js

@@ -0,0 +1,2 @@
+import * as foo from "./fixture_export_empty.js";
+export { foo };

tests/fixture_export_ns_import_2.js

@@ -0,0 +1,2 @@
+import * as foo from "./fixture_export_empty.js";
+export { foo };

tests/fixture_export_ns_other.js

@@ -0,0 +1 @@
+export * as foo from "./fixture_export_empty2.js";

tests/fixture_export_ns_star_as_1.js

@@ -0,0 +1 @@
+export * as foo from "./fixture_export_empty.js";

tests/fixture_export_ns_star_as_2.js

@@ -0,0 +1 @@
+export * as foo from "./fixture_export_empty.js";

tests/fixture_export_only_second.js

@@ -0,0 +1 @@
+export const second = 5;

tests/fixture_export_other.js

@@ -0,0 +1,2 @@
+/* a *different* module that also has a foo: re-exporting both is ambiguous */
+export const foo = 3;

tests/fixture_export_renamed.js

@@ -0,0 +1,3 @@
+import { foo as f, counter, bump } from "./fixture_export_base.js";
+export { f, f as alsoF, counter, bump };
+export const localOnly = 10;

tests/module-export-binding-resolution.js

@@ -0,0 +1,119 @@
+import { assert } from "./assert.js";
+
+/* ResolveExport only reports an ambiguity when two `export *` paths reach two
+   *different* bindings. Reaching the same one twice is fine, and there are two
+   ways to lose track of that:
+
+   - `import { foo } from "m"; export { foo }` is not a local binding of this
+     module. ParseModule turns it into an indirect export through "m", so it
+     resolves to the same binding as `export { foo } from "m"`.
+   - `export * as ns from "m"` resolves to m's namespace, which does not depend
+     on which module re-exported it, so two such re-exports of the same module
+     are the same binding. */
+
+async function importErr(spec) {
+    return import(spec).then(() => null, e => e);
+}
+
+/* a binding re-exported both ways is still one binding */
+{
+    const ns = await import("./fixture_export_hub_ok.js");
+    assert(ns.foo, 2);
+}
+
+/* the same, checked against the two halves separately */
+{
+    const viaFrom = await import("./fixture_export_from.js");
+    const viaImport = await import("./fixture_export_import_and_export.js");
+    assert(viaFrom.foo, 2);
+    assert(viaImport.foo, 2);
+}
+
+/* a module namespace re-exported twice is one binding, whichever form is
+   used and in whichever combination */
+{
+    for (const spec of ["./fixture_export_hub_ns_ok.js",
+                        "./fixture_export_hub_ns_import_ok.js",
+                        "./fixture_export_hub_ns_mixed_ok.js"]) {
+        const ns = await import(spec);
+        assert(typeof ns.foo, "object", spec);
+        assert(ns.foo[Symbol.toStringTag], "Module", spec);
+    }
+
+    /* and it is literally the same namespace object every time */
+    const a = await import("./fixture_export_hub_ns_ok.js");
+    const b = await import("./fixture_export_hub_ns_import_ok.js");
+    const c = await import("./fixture_export_hub_ns_mixed_ok.js");
+    const empty = await import("./fixture_export_empty.js");
+    assert(a.foo === empty, true);
+    assert(b.foo === empty, true);
+    assert(c.foo === empty, true);
+}
+
+/* genuinely different bindings are still ambiguous. An ambiguous name is
+   left out of the namespace object rather than throwing; importing it by
+   name is what fails. */
+{
+    const ns = await import("./fixture_export_hub_ambiguous.js");
+    assert("foo" in ns, false);
+    assert(Object.keys(ns).indexOf("foo"), -1);
+
+    const nsns = await import("./fixture_export_hub_ns_ambiguous.js");
+    assert("foo" in nsns, false);
+
+    /* the other names of the same modules are unaffected */
+    const partial = await import("./fixture_export_hub_partial.js");
+    assert("foo" in partial, false);
+    assert(partial.second, 5);
+    assert(partial.counter, 0);
+
+    for (const spec of ["./fixture_export_hub_ambiguous_use.js",
+                        "./fixture_export_hub_ns_ambiguous_use.js"]) {
+        const e = await importErr(spec);
+        assert(e instanceof SyntaxError, true, spec);
+        assert(e.message.indexOf("ambiguous") >= 0, true, e.message);
+    }
+}
+
+/* rewriting a re-exported import must not disturb the ordinary cases */
+{
+    const ns = await import("./fixture_export_renamed.js");
+
+    /* an import renamed on the way in, then exported under the local name */
+    assert(ns.f, 2);
+    assert(ns.alsoF, 2);
+    /* a plain local export alongside it */
+    assert(ns.localOnly, 10);
+
+    /* the export names are exactly the ones written, sorted */
+    assert(Object.keys(ns).join(","), "alsoF,bump,counter,f,localOnly");
+
+    /* the re-exported binding is live: mutating it in the defining module is
+       visible through the re-export */
+    const base = await import("./fixture_export_base.js");
+    assert(ns.counter, 0);
+    assert(base.counter, 0);
+    ns.bump();
+    assert(base.counter, 1);
+    assert(ns.counter, 1);
+    base.bump();
+    assert(ns.counter, 2);
+}
+
+/* namespace objects are read-only views, however the name got there */
+{
+    const ns = await import("./fixture_export_hub_ok.js");
+    assert(Reflect.set(ns, "foo", 99), false);
+    assert(ns.foo, 2);
+    assert(Reflect.defineProperty(ns, "bar", { value: 1 }), false);
+    assert(Object.getOwnPropertyDescriptor(ns, "foo").writable, true);
+    assert(Object.getOwnPropertyDescriptor(ns, "foo").configurable, false);
+    assert("nope" in ns, false);
+}
+
+/* re-importing resolves to the same module instance each time */
+{
+    const a = await import("./fixture_export_import_and_export.js");
+    const b = await import("./fixture_export_import_and_export.js");
+    assert(a === b, true);
+}