diff --git a/quickjs.c b/quickjs.c index bbac00c33..9d33bdfd3 100644 --- a/quickjs.c +++ b/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; diff --git a/test262_errors.txt b/test262_errors.txt index 082fb016a..06baa4371 100644 --- a/test262_errors.txt +++ b/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 diff --git a/tests.conf b/tests.conf index 192656e0b..1c6962bba 100644 --- a/tests.conf +++ b/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 diff --git a/tests/fixture_export_base.js b/tests/fixture_export_base.js new file mode 100644 index 000000000..d20098ac8 --- /dev/null +++ b/tests/fixture_export_base.js @@ -0,0 +1,3 @@ +export const foo = 2; +export let counter = 0; +export function bump() { counter++; } diff --git a/tests/fixture_export_empty.js b/tests/fixture_export_empty.js new file mode 100644 index 000000000..bd665be50 --- /dev/null +++ b/tests/fixture_export_empty.js @@ -0,0 +1 @@ +/* no exports; only ever used as a namespace */ diff --git a/tests/fixture_export_empty2.js b/tests/fixture_export_empty2.js new file mode 100644 index 000000000..891828b51 --- /dev/null +++ b/tests/fixture_export_empty2.js @@ -0,0 +1 @@ +/* a second, distinct empty module */ diff --git a/tests/fixture_export_from.js b/tests/fixture_export_from.js new file mode 100644 index 000000000..7a9333b59 --- /dev/null +++ b/tests/fixture_export_from.js @@ -0,0 +1 @@ +export { foo } from "./fixture_export_base.js"; diff --git a/tests/fixture_export_hub_ambiguous.js b/tests/fixture_export_hub_ambiguous.js new file mode 100644 index 000000000..64d012fea --- /dev/null +++ b/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"; diff --git a/tests/fixture_export_hub_ambiguous_use.js b/tests/fixture_export_hub_ambiguous_use.js new file mode 100644 index 000000000..b3742d3bb --- /dev/null +++ b/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 }; diff --git a/tests/fixture_export_hub_ns_ambiguous.js b/tests/fixture_export_hub_ns_ambiguous.js new file mode 100644 index 000000000..9cef2a24b --- /dev/null +++ b/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"; diff --git a/tests/fixture_export_hub_ns_ambiguous_use.js b/tests/fixture_export_hub_ns_ambiguous_use.js new file mode 100644 index 000000000..8e14b0a83 --- /dev/null +++ b/tests/fixture_export_hub_ns_ambiguous_use.js @@ -0,0 +1,2 @@ +import { foo } from "./fixture_export_hub_ns_ambiguous.js"; +export { foo }; diff --git a/tests/fixture_export_hub_ns_import_ok.js b/tests/fixture_export_hub_ns_import_ok.js new file mode 100644 index 000000000..88b17955c --- /dev/null +++ b/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"; diff --git a/tests/fixture_export_hub_ns_mixed_ok.js b/tests/fixture_export_hub_ns_mixed_ok.js new file mode 100644 index 000000000..aad47b00e --- /dev/null +++ b/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"; diff --git a/tests/fixture_export_hub_ns_ok.js b/tests/fixture_export_hub_ns_ok.js new file mode 100644 index 000000000..bdea36e07 --- /dev/null +++ b/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"; diff --git a/tests/fixture_export_hub_ok.js b/tests/fixture_export_hub_ok.js new file mode 100644 index 000000000..91f0b11a3 --- /dev/null +++ b/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"; diff --git a/tests/fixture_export_hub_partial.js b/tests/fixture_export_hub_partial.js new file mode 100644 index 000000000..61e9d972e --- /dev/null +++ b/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"; diff --git a/tests/fixture_export_import_and_export.js b/tests/fixture_export_import_and_export.js new file mode 100644 index 000000000..49077c2f1 --- /dev/null +++ b/tests/fixture_export_import_and_export.js @@ -0,0 +1,2 @@ +import { foo } from "./fixture_export_base.js"; +export { foo }; diff --git a/tests/fixture_export_ns_import_1.js b/tests/fixture_export_ns_import_1.js new file mode 100644 index 000000000..670b6f2bf --- /dev/null +++ b/tests/fixture_export_ns_import_1.js @@ -0,0 +1,2 @@ +import * as foo from "./fixture_export_empty.js"; +export { foo }; diff --git a/tests/fixture_export_ns_import_2.js b/tests/fixture_export_ns_import_2.js new file mode 100644 index 000000000..670b6f2bf --- /dev/null +++ b/tests/fixture_export_ns_import_2.js @@ -0,0 +1,2 @@ +import * as foo from "./fixture_export_empty.js"; +export { foo }; diff --git a/tests/fixture_export_ns_other.js b/tests/fixture_export_ns_other.js new file mode 100644 index 000000000..ece94132c --- /dev/null +++ b/tests/fixture_export_ns_other.js @@ -0,0 +1 @@ +export * as foo from "./fixture_export_empty2.js"; diff --git a/tests/fixture_export_ns_star_as_1.js b/tests/fixture_export_ns_star_as_1.js new file mode 100644 index 000000000..32a97d273 --- /dev/null +++ b/tests/fixture_export_ns_star_as_1.js @@ -0,0 +1 @@ +export * as foo from "./fixture_export_empty.js"; diff --git a/tests/fixture_export_ns_star_as_2.js b/tests/fixture_export_ns_star_as_2.js new file mode 100644 index 000000000..32a97d273 --- /dev/null +++ b/tests/fixture_export_ns_star_as_2.js @@ -0,0 +1 @@ +export * as foo from "./fixture_export_empty.js"; diff --git a/tests/fixture_export_only_second.js b/tests/fixture_export_only_second.js new file mode 100644 index 000000000..0dad49b1b --- /dev/null +++ b/tests/fixture_export_only_second.js @@ -0,0 +1 @@ +export const second = 5; diff --git a/tests/fixture_export_other.js b/tests/fixture_export_other.js new file mode 100644 index 000000000..56ce12a36 --- /dev/null +++ b/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; diff --git a/tests/fixture_export_renamed.js b/tests/fixture_export_renamed.js new file mode 100644 index 000000000..f72252f1a --- /dev/null +++ b/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; diff --git a/tests/module-export-binding-resolution.js b/tests/module-export-binding-resolution.js new file mode 100644 index 000000000..6b04da9bb --- /dev/null +++ b/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); +}