diff options
Diffstat (limited to 'js/src')
-rw-r--r-- | js/src/builtin/Module.js | 292 | ||||
-rw-r--r-- | js/src/builtin/ModuleObject.cpp | 38 | ||||
-rw-r--r-- | js/src/builtin/ModuleObject.h | 13 | ||||
-rw-r--r-- | js/src/builtin/SelfHostingDefines.h | 14 | ||||
-rw-r--r-- | js/src/builtin/TestingFunctions.cpp | 6 | ||||
-rw-r--r-- | js/src/jit-test/tests/modules/bug-1284486.js | 12 | ||||
-rw-r--r-- | js/src/jit-test/tests/modules/bug-1420420-2.js | 19 | ||||
-rw-r--r-- | js/src/jit-test/tests/modules/bug-1420420-3.js | 9 | ||||
-rw-r--r-- | js/src/jit-test/tests/modules/bug-1420420-4.js | 16 | ||||
-rw-r--r-- | js/src/jit-test/tests/modules/bug-1420420.js | 19 | ||||
-rw-r--r-- | js/src/js.msg | 1 | ||||
-rw-r--r-- | js/src/jsapi.cpp | 12 | ||||
-rw-r--r-- | js/src/jsapi.h | 6 | ||||
-rw-r--r-- | js/src/vm/EnvironmentObject.cpp | 2 | ||||
-rw-r--r-- | js/src/vm/SelfHosting.cpp | 1 |
15 files changed, 252 insertions, 208 deletions
diff --git a/js/src/builtin/Module.js b/js/src/builtin/Module.js index 64d62d216..86b44880d 100644 --- a/js/src/builtin/Module.js +++ b/js/src/builtin/Module.js @@ -69,8 +69,13 @@ function ModuleSetStatus(module, newStatus) { assert(newStatus >= MODULE_STATUS_ERRORED && newStatus <= MODULE_STATUS_EVALUATED, "Bad new module status in ModuleSetStatus"); - if (newStatus !== MODULE_STATUS_ERRORED) - assert(newStatus > module.status, "New module status inconsistent with current status"); + + // Note that under OOM conditions we can fail the module instantiation + // process even after modules have been marked as instantiated. + assert((module.status <= MODULE_STATUS_INSTANTIATED && + newStatus === MODULE_STATUS_UNINSTANTIATED) || + newStatus > module.status, + "New module status inconsistent with current status"); UnsafeSetReservedSlot(module, MODULE_OBJECT_STATUS_SLOT, newStatus); } @@ -80,20 +85,15 @@ function ModuleSetStatus(module, newStatus) // Returns an object describing the location of the resolved export or // indicating a failure. // -// On success this returns: { resolved: true, module, bindingName } -// -// There are three failure cases: +// On success this returns a resolved binding record: { module, bindingName } // -// - The resolution failure can be blamed on a particular module. -// Returns: { resolved: false, module, ambiguous: false } +// There are two failure cases: // -// - No culprit can be determined and the resolution failure was due to star -// export ambiguity. -// Returns: { resolved: false, module: null, ambiguous: true } +// - If no definition was found or the request is found to be circular, *null* +// is returned. // -// - No culprit can be determined and the resolution failure was not due to -// star export ambiguity. -// Returns: { resolved: false, module: null, ambiguous: false } +// - If the request is found to be ambiguous, the string `"ambiguous"` is +// returned. // function ModuleResolveExport(exportName, resolveSet = []) { @@ -106,53 +106,47 @@ function ModuleResolveExport(exportName, resolveSet = []) let module = this; // Step 2 - assert(module.status !== MODULE_STATUS_ERRORED, "Bad module status in ResolveExport"); - - // Step 3 for (let i = 0; i < resolveSet.length; i++) { let r = resolveSet[i]; if (r.module === module && r.exportName === exportName) { // This is a circular import request. - return {resolved: false, module: null, ambiguous: false}; + return null; } } - // Step 4 + // Step 3 _DefineDataProperty(resolveSet, resolveSet.length, {module: module, exportName: exportName}); - // Step 5 + // Step 4 let localExportEntries = module.localExportEntries; for (let i = 0; i < localExportEntries.length; i++) { let e = localExportEntries[i]; if (exportName === e.exportName) - return {resolved: true, module, bindingName: e.localName}; + return {module, bindingName: e.localName}; } - // Step 6 + // Step 5 let indirectExportEntries = module.indirectExportEntries; for (let i = 0; i < indirectExportEntries.length; i++) { let e = indirectExportEntries[i]; if (exportName === e.exportName) { let importedModule = CallModuleResolveHook(module, e.moduleRequest, MODULE_STATUS_UNINSTANTIATED); - let resolution = callFunction(importedModule.resolveExport, importedModule, e.importName, - resolveSet); - if (!resolution.resolved && !resolution.module) - resolution.module = module; - return resolution; + return callFunction(importedModule.resolveExport, importedModule, e.importName, + resolveSet); } } - // Step 7 + // Step 6 if (exportName === "default") { // A default export cannot be provided by an export *. - return {resolved: false, module: null, ambiguous: false}; + return null; } - // Step 8 + // Step 7 let starResolution = null; - // Step 9 + // Step 8 let starExportEntries = module.starExportEntries; for (let i = 0; i < starExportEntries.length; i++) { let e = starExportEntries[i]; @@ -160,27 +154,31 @@ function ModuleResolveExport(exportName, resolveSet = []) MODULE_STATUS_UNINSTANTIATED); let resolution = callFunction(importedModule.resolveExport, importedModule, exportName, resolveSet); - if (!resolution.resolved && (resolution.module || resolution.ambiguous)) + if (resolution === "ambiguous") return resolution; - if (resolution.resolved) { + if (resolution !== null) { if (starResolution === null) { starResolution = resolution; } else { if (resolution.module !== starResolution.module || resolution.bindingName !== starResolution.bindingName) { - return {resolved: false, module: null, ambiguous: true}; + return "ambiguous"; } } } } - // Step 10 - if (starResolution !== null) - return starResolution; + // Step 9 + return starResolution; +} - return {resolved: false, module: null, ambiguous: false}; +function IsResolvedBinding(resolution) +{ + assert(resolution === "ambiguous" || typeof resolution === "object", + "Bad module resolution result"); + return typeof resolution === "object" && resolution !== null; } // 15.2.1.18 GetModuleNamespace(module) @@ -189,12 +187,12 @@ function GetModuleNamespace(module) // Step 1 assert(IsModule(module), "GetModuleNamespace called with non-module"); - // Step 2 + // Steps 2-3 assert(module.status !== MODULE_STATUS_UNINSTANTIATED && - module.status !== MODULE_STATUS_ERRORED, + module.status !== MODULE_STATUS_EVALUATED_ERROR, "Bad module status in GetModuleNamespace"); - // Step 3 + // Step 4 let namespace = module.namespace; if (typeof namespace === "undefined") { @@ -203,7 +201,7 @@ function GetModuleNamespace(module) for (let i = 0; i < exportedNames.length; i++) { let name = exportedNames[i]; let resolution = callFunction(module.resolveExport, module, name); - if (resolution.resolved) + if (IsResolvedBinding(resolution)) _DefineDataProperty(unambiguousNames, unambiguousNames.length, name); } namespace = ModuleNamespaceCreate(module, unambiguousNames); @@ -225,7 +223,7 @@ function ModuleNamespaceCreate(module, exports) for (let i = 0; i < exports.length; i++) { let name = exports[i]; let binding = callFunction(module.resolveExport, module, name); - assert(binding.resolved, "Failed to resolve binding"); + assert(IsResolvedBinding(binding), "Failed to resolve binding"); AddModuleNamespaceBinding(ns, name, binding.module, binding.bindingName); } @@ -236,11 +234,6 @@ function GetModuleEnvironment(module) { assert(IsModule(module), "Non-module passed to GetModuleEnvironment"); - // Check for a previous failed attempt to instantiate this module. This can - // only happen due to a bug in the module loader. - if (module.status === MODULE_STATUS_ERRORED) - ThrowInternalError(JSMSG_MODULE_INSTANTIATE_FAILED, module.status); - let env = UnsafeGetReservedSlot(module, MODULE_OBJECT_ENVIRONMENT_SLOT); assert(env === undefined || IsModuleEnvironment(env), "Module environment slot contains unexpected value"); @@ -248,19 +241,6 @@ function GetModuleEnvironment(module) return env; } -function RecordModuleError(module, error) -{ - // Set the module's status to 'errored' to indicate a failed module - // instantiation and record the exception. The environment slot is also - // reset to 'undefined'. - - assert(IsObject(module) && IsModule(module), "Non-module passed to RecordModuleError"); - - ModuleSetStatus(module, MODULE_STATUS_ERRORED); - UnsafeSetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT, error); - UnsafeSetReservedSlot(module, MODULE_OBJECT_ENVIRONMENT_SLOT, undefined); -} - function CountArrayValues(array, value) { let count = 0; @@ -280,6 +260,16 @@ function ArrayContains(array, value) return false; } +function HandleModuleInstantiationFailure(module) +{ + // Reset the module to the "uninstantiated" state. Don't reset the + // environment slot as the environment object will be required by any + // possible future instantiation attempt. + ModuleSetStatus(module, MODULE_STATUS_UNINSTANTIATED); + UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_INDEX_SLOT, undefined); + UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT, undefined); +} + // 15.2.1.16.4 ModuleInstantiate() function ModuleInstantiate() { @@ -301,38 +291,30 @@ function ModuleInstantiate() // Steps 4-5 try { - InnerModuleDeclarationInstantiation(module, stack, 0); + InnerModuleInstantiation(module, stack, 0); } catch (error) { for (let i = 0; i < stack.length; i++) { let m = stack[i]; - assert(m.status === MODULE_STATUS_INSTANTIATING || - m.status === MODULE_STATUS_ERRORED, - "Bad module status after failed instantiation"); - - RecordModuleError(m, error); + assert(m.status === MODULE_STATUS_INSTANTIATING, + "Expected instantiating status during failed instantiation"); + HandleModuleInstantiationFailure(m); } - if (stack.length === 0 && - typeof(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT)) === "undefined") - { - // This can happen due to OOM when appending to the stack. - assert(error === "out of memory", - "Stack must contain module unless we hit OOM"); - RecordModuleError(module, error); - } + // Handle OOM when appending to the stack or over-recursion errors. + if (stack.length === 0) + HandleModuleInstantiationFailure(module); - assert(module.status === MODULE_STATUS_ERRORED, - "Bad module status after failed instantiation"); - assert(SameValue(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT), error), - "Module has different error set after failed instantiation"); + assert(module.status === MODULE_STATUS_UNINSTANTIATED, + "Expected uninstantiated status after failed instantiation"); throw error; } // Step 6 - assert(module.status == MODULE_STATUS_INSTANTIATED || - module.status == MODULE_STATUS_EVALUATED, + assert(module.status === MODULE_STATUS_INSTANTIATED || + module.status === MODULE_STATUS_EVALUATED || + module.status === MODULE_STATUS_EVALUATED_ERROR, "Bad module status after successful instantiation"); // Step 7 @@ -344,8 +326,8 @@ function ModuleInstantiate() } _SetCanonicalName(ModuleInstantiate, "ModuleInstantiate"); -// 15.2.1.16.4.1 InnerModuleDeclarationInstantiation(module, stack, index) -function InnerModuleDeclarationInstantiation(module, stack, index) +// 15.2.1.16.4.1 InnerModuleInstantiation(module, stack, index) +function InnerModuleInstantiation(module, stack, index) { // Step 1 // TODO: Support module records other than source text module records. @@ -353,42 +335,40 @@ function InnerModuleDeclarationInstantiation(module, stack, index) // Step 2 if (module.status === MODULE_STATUS_INSTANTIATING || module.status === MODULE_STATUS_INSTANTIATED || - module.status === MODULE_STATUS_EVALUATED) + module.status === MODULE_STATUS_EVALUATED || + module.status === MODULE_STATUS_EVALUATED_ERROR) { return index; } // Step 3 - if (module.status === MODULE_STATUS_ERRORED) - throw module.error; - - // Step 4 assert(module.status === MODULE_STATUS_UNINSTANTIATED, "Bad module status in ModuleDeclarationInstantiation"); - // Steps 5 + // Step 4 ModuleSetStatus(module, MODULE_STATUS_INSTANTIATING); - // Step 6-8 + // Steps 5-7 UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_INDEX_SLOT, index); UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT, index); index++; - // Step 9 + // Step 8 _DefineDataProperty(stack, stack.length, module); - // Step 10 + // Step 9 let requestedModules = module.requestedModules; for (let i = 0; i < requestedModules.length; i++) { let required = requestedModules[i]; - let requiredModule = CallModuleResolveHook(module, required, MODULE_STATUS_ERRORED); + let requiredModule = CallModuleResolveHook(module, required, MODULE_STATUS_UNINSTANTIATED); - index = InnerModuleDeclarationInstantiation(requiredModule, stack, index); + index = InnerModuleInstantiation(requiredModule, stack, index); assert(requiredModule.status === MODULE_STATUS_INSTANTIATING || requiredModule.status === MODULE_STATUS_INSTANTIATED || - requiredModule.status === MODULE_STATUS_EVALUATED, - "Bad required module status after InnerModuleDeclarationInstantiation"); + requiredModule.status === MODULE_STATUS_EVALUATED || + requiredModule.status === MODULE_STATUS_EVALUATED_ERROR, + "Bad required module status after InnerModuleInstantiation"); assert((requiredModule.status === MODULE_STATUS_INSTANTIATING) === ArrayContains(stack, requiredModule), @@ -404,16 +384,16 @@ function InnerModuleDeclarationInstantiation(module, stack, index) } } - // Step 11 + // Step 10 ModuleDeclarationEnvironmentSetup(module); - // Steps 12-13 + // Steps 11-12 assert(CountArrayValues(stack, module) === 1, "Current module should appear exactly once in the stack"); assert(module.dfsAncestorIndex <= module.dfsIndex, "Bad DFS ancestor index"); - // Step 14 + // Step 13 if (module.dfsAncestorIndex === module.dfsIndex) { let requiredModule; do { @@ -434,11 +414,9 @@ function ModuleDeclarationEnvironmentSetup(module) for (let i = 0; i < indirectExportEntries.length; i++) { let e = indirectExportEntries[i]; let resolution = callFunction(module.resolveExport, module, e.exportName); - assert(resolution.resolved || resolution.module, - "Unexpected failure to resolve export in ModuleDeclarationEnvironmentSetup"); - if (!resolution.resolved) { - return ResolutionError(resolution, "indirectExport", e.exportName, - e.lineNumber, e.columnNumber) + if (!IsResolvedBinding(resolution)) { + ThrowResolutionError(module, resolution, "indirectExport", e.exportName, + e.lineNumber, e.columnNumber); } } @@ -458,12 +436,9 @@ function ModuleDeclarationEnvironmentSetup(module) } else { let resolution = callFunction(importedModule.resolveExport, importedModule, imp.importName); - if (!resolution.resolved && !resolution.module) - resolution.module = module; - - if (!resolution.resolved) { - return ResolutionError(resolution, "import", imp.importName, - imp.lineNumber, imp.columnNumber); + if (!IsResolvedBinding(resolution)) { + ThrowResolutionError(module, resolution, "import", imp.importName, + imp.lineNumber, imp.columnNumber); } CreateImportBinding(env, imp.localName, resolution.module, resolution.bindingName); @@ -473,38 +448,58 @@ function ModuleDeclarationEnvironmentSetup(module) InstantiateModuleFunctionDeclarations(module); } -// 15.2.1.16.4.3 ResolutionError(module) -function ResolutionError(resolution, kind, name, line, column) +function ThrowResolutionError(module, resolution, kind, name, line, column) { - let module = resolution.module; - assert(module !== null, - "Null module passed to ResolutionError"); - - assert(module.status === MODULE_STATUS_UNINSTANTIATED || - module.status === MODULE_STATUS_INSTANTIATING, - "Unexpected module status in ResolutionError"); + assert(module.status === MODULE_STATUS_INSTANTIATING, + "Unexpected module status in ThrowResolutionError"); assert(kind === "import" || kind === "indirectExport", - "Unexpected kind in ResolutionError"); + "Unexpected kind in ThrowResolutionError"); assert(line > 0, "Line number should be present for all imports and indirect exports"); + let ambiguous = resolution === "ambiguous"; + let errorNumber; - if (kind === "import") { - errorNumber = resolution.ambiguous ? JSMSG_AMBIGUOUS_IMPORT - : JSMSG_MISSING_IMPORT; - } else { - errorNumber = resolution.ambiguous ? JSMSG_AMBIGUOUS_INDIRECT_EXPORT - : JSMSG_MISSING_INDIRECT_EXPORT; - } + if (kind === "import") + errorNumber = ambiguous ? JSMSG_AMBIGUOUS_IMPORT : JSMSG_MISSING_IMPORT; + else + errorNumber = ambiguous ? JSMSG_AMBIGUOUS_INDIRECT_EXPORT : JSMSG_MISSING_INDIRECT_EXPORT; let message = GetErrorMessage(errorNumber) + ": " + name; let error = CreateModuleSyntaxError(module, line, column, message); - RecordModuleError(module, error); throw error; } +function GetModuleEvaluationError(module) +{ + assert(IsObject(module) && IsModule(module), + "Non-module passed to GetModuleEvaluationError"); + assert(module.status === MODULE_STATUS_EVALUATED_ERROR, + "Bad module status in GetModuleEvaluationError"); + return UnsafeGetReservedSlot(module, MODULE_OBJECT_EVALUATION_ERROR_SLOT); +} + +function RecordModuleEvaluationError(module, error) +{ + // Set the module's EvaluationError slot to indicate a failed module + // evaluation. + + assert(IsObject(module) && IsModule(module), + "Non-module passed to RecordModuleEvaluationError"); + + if (module.status === MODULE_STATUS_EVALUATED_ERROR) { + // It would be nice to assert that |error| is the same as the one we + // previously recorded, but that's not always true in the case of out of + // memory and over-recursion errors. + return; + } + + ModuleSetStatus(module, MODULE_STATUS_EVALUATED_ERROR); + UnsafeSetReservedSlot(module, MODULE_OBJECT_EVALUATION_ERROR_SLOT, error); +} + // 15.2.1.16.5 ModuleEvaluate() function ModuleEvaluate() { @@ -515,9 +510,9 @@ function ModuleEvaluate() let module = this; // Step 2 - if (module.status !== MODULE_STATUS_ERRORED && - module.status !== MODULE_STATUS_INSTANTIATED && - module.status !== MODULE_STATUS_EVALUATED) + if (module.status !== MODULE_STATUS_INSTANTIATED && + module.status !== MODULE_STATUS_EVALUATED && + module.status !== MODULE_STATUS_EVALUATED_ERROR) { ThrowInternalError(JSMSG_BAD_MODULE_STATUS); } @@ -535,27 +530,20 @@ function ModuleEvaluate() assert(m.status === MODULE_STATUS_EVALUATING, "Bad module status after failed evaluation"); - RecordModuleError(m, error); + RecordModuleEvaluationError(m, error); } - if (stack.length === 0 && - typeof(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT)) === "undefined") - { - // This can happen due to OOM when appending to the stack. - assert(error === "out of memory", - "Stack must contain module unless we hit OOM"); - RecordModuleError(module, error); - } + // Handle OOM when appending to the stack or over-recursion errors. + if (stack.length === 0) + RecordModuleEvaluationError(module, error); - assert(module.status === MODULE_STATUS_ERRORED, + assert(module.status === MODULE_STATUS_EVALUATED_ERROR, "Bad module status after failed evaluation"); - assert(SameValue(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT), error), - "Module has different error set after failed evaluation"); throw error; } - assert(module.status == MODULE_STATUS_EVALUATED, + assert(module.status === MODULE_STATUS_EVALUATED, "Bad module status after successful evaluation"); assert(stack.length === 0, "Stack should be empty after successful evaluation"); @@ -571,19 +559,19 @@ function InnerModuleEvaluation(module, stack, index) // TODO: Support module records other than source text module records. // Step 2 - if (module.status === MODULE_STATUS_EVALUATING || - module.status === MODULE_STATUS_EVALUATED) - { + if (module.status === MODULE_STATUS_EVALUATED_ERROR) + throw GetModuleEvaluationError(module); + + if (module.status === MODULE_STATUS_EVALUATED) return index; - } // Step 3 - if (module.status === MODULE_STATUS_ERRORED) - throw module.error; + if (module.status === MODULE_STATUS_EVALUATING) + return index; // Step 4 assert(module.status === MODULE_STATUS_INSTANTIATED, - "Bad module status in ModuleEvaluation"); + "Bad module status in InnerModuleEvaluation"); // Step 5 ModuleSetStatus(module, MODULE_STATUS_EVALUATING); @@ -605,8 +593,8 @@ function InnerModuleEvaluation(module, stack, index) index = InnerModuleEvaluation(requiredModule, stack, index); - assert(requiredModule.status == MODULE_STATUS_EVALUATING || - requiredModule.status == MODULE_STATUS_EVALUATED, + assert(requiredModule.status === MODULE_STATUS_EVALUATING || + requiredModule.status === MODULE_STATUS_EVALUATED, "Bad module status after InnerModuleEvaluation"); assert((requiredModule.status === MODULE_STATUS_EVALUATING) === diff --git a/js/src/builtin/ModuleObject.cpp b/js/src/builtin/ModuleObject.cpp index 30e7120c0..a9a257178 100644 --- a/js/src/builtin/ModuleObject.cpp +++ b/js/src/builtin/ModuleObject.cpp @@ -18,10 +18,10 @@ using namespace js; using namespace js::frontend; -static_assert(MODULE_STATUS_ERRORED < MODULE_STATUS_UNINSTANTIATED && - MODULE_STATUS_UNINSTANTIATED < MODULE_STATUS_INSTANTIATING && +static_assert(MODULE_STATUS_UNINSTANTIATED < MODULE_STATUS_INSTANTIATING && MODULE_STATUS_INSTANTIATING < MODULE_STATUS_INSTANTIATED && - MODULE_STATUS_INSTANTIATED < MODULE_STATUS_EVALUATED, + MODULE_STATUS_INSTANTIATED < MODULE_STATUS_EVALUATED && + MODULE_STATUS_EVALUATED < MODULE_STATUS_EVALUATED_ERROR, "Module statuses are ordered incorrectly"); template<typename T, Value ValueGetter(const T* obj)> @@ -273,12 +273,12 @@ IndirectBindingMap::trace(JSTracer* trc) } bool -IndirectBindingMap::putNew(JSContext* cx, HandleId name, - HandleModuleEnvironmentObject environment, HandleId localName) +IndirectBindingMap::put(JSContext* cx, HandleId name, + HandleModuleEnvironmentObject environment, HandleId localName) { RootedShape shape(cx, environment->lookup(cx, localName)); MOZ_ASSERT(shape); - if (!map_.putNew(name, Binding(environment, shape))) { + if (!map_.put(name, Binding(environment, shape))) { ReportOutOfMemory(cx); return false; } @@ -359,7 +359,7 @@ ModuleNamespaceObject::addBinding(JSContext* cx, HandleAtom exportedName, RootedModuleEnvironmentObject environment(cx, &targetModule->initialEnvironment()); RootedId exportedNameId(cx, AtomToId(exportedName)); RootedId localNameId(cx, AtomToId(localName)); - return bindings->putNew(cx, exportedNameId, environment, localNameId); + return bindings->put(cx, exportedNameId, environment, localNameId); } const char ModuleNamespaceObject::ProxyHandler::family = 0; @@ -660,6 +660,9 @@ ModuleObject::finalize(js::FreeOp* fop, JSObject* obj) ModuleEnvironmentObject* ModuleObject::environment() const { +//- MOZ_ASSERT(status() != MODULE_STATUS_ERRORED); +//+ MOZ_ASSERT(!hadEvaluationError()); + Value value = getReservedSlot(EnvironmentSlot); if (value.isUndefined()) return nullptr; @@ -723,7 +726,7 @@ void ModuleObject::init(HandleScript script) { initReservedSlot(ScriptSlot, PrivateValue(script)); - initReservedSlot(StatusSlot, Int32Value(MODULE_STATUS_ERRORED)); + initReservedSlot(StatusSlot, Int32Value(MODULE_STATUS_UNINSTANTIATED)); } void @@ -827,7 +830,8 @@ ModuleObject::script() const static inline void AssertValidModuleStatus(ModuleStatus status) { - MOZ_ASSERT(status >= MODULE_STATUS_ERRORED && status <= MODULE_STATUS_EVALUATED); + MOZ_ASSERT(status >= MODULE_STATUS_UNINSTANTIATED && + status <= MODULE_STATUS_EVALUATED_ERROR); } ModuleStatus @@ -838,11 +842,17 @@ ModuleObject::status() const return status; } +bool +ModuleObject::hadEvaluationError() const +{ + return status() == MODULE_STATUS_EVALUATED_ERROR; +} + Value -ModuleObject::error() const +ModuleObject::evaluationError() const { - MOZ_ASSERT(status() == MODULE_STATUS_ERRORED); - return getReservedSlot(ErrorSlot); + MOZ_ASSERT(hadEvaluationError()); + return getReservedSlot(EvaluationErrorSlot); } Value @@ -1005,7 +1015,7 @@ ModuleObject::Evaluate(JSContext* cx, HandleModuleObject self) DEFINE_GETTER_FUNCTIONS(ModuleObject, namespace_, NamespaceSlot) DEFINE_GETTER_FUNCTIONS(ModuleObject, status, StatusSlot) -DEFINE_GETTER_FUNCTIONS(ModuleObject, error, ErrorSlot) +DEFINE_GETTER_FUNCTIONS(ModuleObject, evaluationError, EvaluationErrorSlot) DEFINE_GETTER_FUNCTIONS(ModuleObject, requestedModules, RequestedModulesSlot) DEFINE_GETTER_FUNCTIONS(ModuleObject, importEntries, ImportEntriesSlot) DEFINE_GETTER_FUNCTIONS(ModuleObject, localExportEntries, LocalExportEntriesSlot) @@ -1020,7 +1030,7 @@ GlobalObject::initModuleProto(JSContext* cx, Handle<GlobalObject*> global) static const JSPropertySpec protoAccessors[] = { JS_PSG("namespace", ModuleObject_namespace_Getter, 0), JS_PSG("status", ModuleObject_statusGetter, 0), - JS_PSG("error", ModuleObject_errorGetter, 0), + JS_PSG("evaluationError", ModuleObject_evaluationErrorGetter, 0), JS_PSG("requestedModules", ModuleObject_requestedModulesGetter, 0), JS_PSG("importEntries", ModuleObject_importEntriesGetter, 0), JS_PSG("localExportEntries", ModuleObject_localExportEntriesGetter, 0), diff --git a/js/src/builtin/ModuleObject.h b/js/src/builtin/ModuleObject.h index be0315215..5a809c865 100644 --- a/js/src/builtin/ModuleObject.h +++ b/js/src/builtin/ModuleObject.h @@ -107,8 +107,8 @@ class IndirectBindingMap void trace(JSTracer* trc); - bool putNew(JSContext* cx, HandleId name, - HandleModuleEnvironmentObject environment, HandleId localName); + bool put(JSContext* cx, HandleId name, + HandleModuleEnvironmentObject environment, HandleId localName); size_t count() const { return map_.count(); @@ -218,7 +218,7 @@ class ModuleObject : public NativeObject EnvironmentSlot, NamespaceSlot, StatusSlot, - ErrorSlot, + EvaluationErrorSlot, HostDefinedSlot, RequestedModulesSlot, ImportEntriesSlot, @@ -238,8 +238,8 @@ class ModuleObject : public NativeObject "EnvironmentSlot must match self-hosting define"); static_assert(StatusSlot == MODULE_OBJECT_STATUS_SLOT, "StatusSlot must match self-hosting define"); - static_assert(ErrorSlot == MODULE_OBJECT_ERROR_SLOT, - "ErrorSlot must match self-hosting define"); + static_assert(EvaluationErrorSlot == MODULE_OBJECT_EVALUATION_ERROR_SLOT, + "EvaluationErrorSlot must match self-hosting define"); static_assert(DFSIndexSlot == MODULE_OBJECT_DFS_INDEX_SLOT, "DFSIndexSlot must match self-hosting define"); static_assert(DFSAncestorIndexSlot == MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT, @@ -269,7 +269,8 @@ class ModuleObject : public NativeObject ModuleEnvironmentObject* environment() const; ModuleNamespaceObject* namespace_(); ModuleStatus status() const; - Value error() const; + bool hadEvaluationError() const; + Value evaluationError() const; Value hostDefinedField() const; ArrayObject& requestedModules() const; ArrayObject& importEntries() const; diff --git a/js/src/builtin/SelfHostingDefines.h b/js/src/builtin/SelfHostingDefines.h index a06c2aa62..6afa641b1 100644 --- a/js/src/builtin/SelfHostingDefines.h +++ b/js/src/builtin/SelfHostingDefines.h @@ -99,15 +99,15 @@ #define MODULE_OBJECT_ENVIRONMENT_SLOT 2 #define MODULE_OBJECT_STATUS_SLOT 4 -#define MODULE_OBJECT_ERROR_SLOT 5 +#define MODULE_OBJECT_EVALUATION_ERROR_SLOT 5 // 4 #define MODULE_OBJECT_DFS_INDEX_SLOT 16 #define MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT 17 -#define MODULE_STATUS_ERRORED 0 -#define MODULE_STATUS_UNINSTANTIATED 1 -#define MODULE_STATUS_INSTANTIATING 2 -#define MODULE_STATUS_INSTANTIATED 3 -#define MODULE_STATUS_EVALUATING 4 -#define MODULE_STATUS_EVALUATED 5 +#define MODULE_STATUS_UNINSTANTIATED 0 +#define MODULE_STATUS_INSTANTIATING 1 +#define MODULE_STATUS_INSTANTIATED 2 +#define MODULE_STATUS_EVALUATING 3 +#define MODULE_STATUS_EVALUATED 4 +#define MODULE_STATUS_EVALUATED_ERROR 5 #endif diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp index 025620766..e93ebeec3 100644 --- a/js/src/builtin/TestingFunctions.cpp +++ b/js/src/builtin/TestingFunctions.cpp @@ -3585,6 +3585,9 @@ GetModuleEnvironmentNames(JSContext* cx, unsigned argc, Value* vp) return false; } +//- if (module->status() == MODULE_STATUS_ERRORED) { +//+ if (module->hadEvaluationError()) { + RootedModuleEnvironmentObject env(cx, GetModuleEnvironment(cx, args[0])); Rooted<IdVector> ids(cx, IdVector(cx)); if (!JS_Enumerate(cx, env, &ids)) @@ -3622,6 +3625,9 @@ GetModuleEnvironmentValue(JSContext* cx, unsigned argc, Value* vp) return false; } +//- if (module->status() == MODULE_STATUS_ERRORED) { +//+ if (module->hadEvaluationError()) { + RootedModuleEnvironmentObject env(cx, GetModuleEnvironment(cx, args[0])); RootedString name(cx, args[1].toString()); RootedId id(cx); diff --git a/js/src/jit-test/tests/modules/bug-1284486.js b/js/src/jit-test/tests/modules/bug-1284486.js index 08286393a..6fbd67d9d 100644 --- a/js/src/jit-test/tests/modules/bug-1284486.js +++ b/js/src/jit-test/tests/modules/bug-1284486.js @@ -1,9 +1,8 @@ -// This tests that attempting to perform ModuleDeclarationInstantation a second -// time after a failure re-throws the same error. +// This tests that module instantiation can succeed when executed a second
+// time after a failure. // // The first attempt fails becuase module 'a' is not available. The second -// attempt fails because of the previous failure (it would otherwise succeed as -// 'a' is now available). +// attempt succeeds as 'a' is now available. load(libdir + "dummyModuleResolveHook.js"); @@ -25,12 +24,9 @@ let a = moduleRepo['a'] = parseModule("export var a = 1; export var b = 2;"); let d = moduleRepo['d'] = parseModule("import { a } from 'c'; a;"); threw = false; -let e2; try { d.declarationInstantiation(); } catch (exc) { threw = true; - e2 = exc; } -assertEq(threw, true); -assertEq(e1, e2); +assertEq(threw, false); diff --git a/js/src/jit-test/tests/modules/bug-1420420-2.js b/js/src/jit-test/tests/modules/bug-1420420-2.js new file mode 100644 index 000000000..0511e8126 --- /dev/null +++ b/js/src/jit-test/tests/modules/bug-1420420-2.js @@ -0,0 +1,19 @@ +// Test re-instantiation module after failure. + +load(libdir + "asserts.js"); +load(libdir + "dummyModuleResolveHook.js"); + +moduleRepo["good"] = parseModule(`export let x`); + +moduleRepo["y1"] = parseModule(`export let y`); +moduleRepo["y2"] = parseModule(`export let y`); +moduleRepo["bad"] = parseModule(`export* from "y1"; export* from "y2";`); + +moduleRepo["a"] = parseModule(`import* as ns from "good"; import {y} from "bad";`); + +let b = moduleRepo["b"] = parseModule(`import "a";`); +let c = moduleRepo["c"] = parseModule(`import "a";`); + +assertThrowsInstanceOf(() => b.declarationInstantiation(), SyntaxError); +assertThrowsInstanceOf(() => c.declarationInstantiation(), SyntaxError); + diff --git a/js/src/jit-test/tests/modules/bug-1420420-3.js b/js/src/jit-test/tests/modules/bug-1420420-3.js new file mode 100644 index 000000000..236c023b9 --- /dev/null +++ b/js/src/jit-test/tests/modules/bug-1420420-3.js @@ -0,0 +1,9 @@ +if (!('stackTest' in this)) + quit(); + +let a = parseModule(`throw new Error`); +a.declarationInstantiation(); +stackTest(function() { + a.evaluation(); +}); + diff --git a/js/src/jit-test/tests/modules/bug-1420420-4.js b/js/src/jit-test/tests/modules/bug-1420420-4.js new file mode 100644 index 000000000..721c770bc --- /dev/null +++ b/js/src/jit-test/tests/modules/bug-1420420-4.js @@ -0,0 +1,16 @@ +load(libdir + "asserts.js"); +load(libdir + "dummyModuleResolveHook.js"); + +moduleRepo["a"] = parseModule(`throw undefined`); + +let b = moduleRepo["b"] = parseModule(`import "a";`); +let c = moduleRepo["c"] = parseModule(`import "a";`); + +b.declarationInstantiation(); +c.declarationInstantiation(); + +let count = 0; +try { b.evaluation() } catch (e) { count++; } +try { c.evaluation() } catch (e) { count++; } +assertEq(count, 2); + diff --git a/js/src/jit-test/tests/modules/bug-1420420.js b/js/src/jit-test/tests/modules/bug-1420420.js new file mode 100644 index 000000000..cd1101c76 --- /dev/null +++ b/js/src/jit-test/tests/modules/bug-1420420.js @@ -0,0 +1,19 @@ +// Test re-instantiation module after failure. + +load(libdir + "asserts.js"); +load(libdir + "dummyModuleResolveHook.js"); + +moduleRepo["good"] = parseModule(`export let x`); + +moduleRepo["y1"] = parseModule(`export let y`); +moduleRepo["y2"] = parseModule(`export let y`); +moduleRepo["bad"] = parseModule(`export* from "y1"; export* from "y2";`); + +moduleRepo["a"] = parseModule(`import {x} from "good"; import {y} from "bad";`); + +let b = moduleRepo["b"] = parseModule(`import "a";`); +let c = moduleRepo["c"] = parseModule(`import "a";`); + +assertThrowsInstanceOf(() => b.declarationInstantiation(), SyntaxError); +assertThrowsInstanceOf(() => c.declarationInstantiation(), SyntaxError); + diff --git a/js/src/js.msg b/js/src/js.msg index 9c508ebbd..587481864 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -579,7 +579,6 @@ MSG_DEF(JSMSG_MISSING_IMPORT, 0, JSEXN_SYNTAXERR, "import not found") MSG_DEF(JSMSG_AMBIGUOUS_IMPORT, 0, JSEXN_SYNTAXERR, "ambiguous import") MSG_DEF(JSMSG_MISSING_NAMESPACE_EXPORT, 0, JSEXN_SYNTAXERR, "export not found for namespace") MSG_DEF(JSMSG_MISSING_EXPORT, 1, JSEXN_SYNTAXERR, "local binding for export '{0}' not found") -MSG_DEF(JSMSG_MODULE_INSTANTIATE_FAILED, 0, JSEXN_INTERNALERR, "attempt to re-instantiate module after failure") MSG_DEF(JSMSG_BAD_MODULE_STATUS, 0, JSEXN_INTERNALERR, "module record has unexpected status") // Promise diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index cf5880e03..3e0c63811 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4739,18 +4739,6 @@ JS::GetModuleScript(JSContext* cx, JS::HandleObject moduleArg) return moduleArg->as<ModuleObject>().script(); } -JS_PUBLIC_API(bool) -JS::IsModuleErrored(JSObject* moduleArg) -{ - return moduleArg->as<ModuleObject>().status() == MODULE_STATUS_ERRORED; -} - -JS_PUBLIC_API(JS::Value) -JS::GetModuleError(JSObject* moduleArg) -{ - return moduleArg->as<ModuleObject>().error(); -} - JS_PUBLIC_API(JSObject*) JS_New(JSContext* cx, HandleObject ctor, const JS::HandleValueArray& inputArgs) { diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 9c3bf8151..76781cf06 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4395,12 +4395,6 @@ GetRequestedModules(JSContext* cx, JS::HandleObject moduleRecord); extern JS_PUBLIC_API(JSScript*) GetModuleScript(JSContext* cx, JS::HandleObject moduleRecord); -extern JS_PUBLIC_API(bool) -IsModuleErrored(JSObject* moduleRecord); - -extern JS_PUBLIC_API(JS::Value) -GetModuleError(JSObject* moduleRecord); - } /* namespace JS */ extern JS_PUBLIC_API(bool) diff --git a/js/src/vm/EnvironmentObject.cpp b/js/src/vm/EnvironmentObject.cpp index 7834940f1..871ae3f19 100644 --- a/js/src/vm/EnvironmentObject.cpp +++ b/js/src/vm/EnvironmentObject.cpp @@ -491,7 +491,7 @@ ModuleEnvironmentObject::createImportBinding(JSContext* cx, HandleAtom importNam RootedId importNameId(cx, AtomToId(importName)); RootedId localNameId(cx, AtomToId(localName)); RootedModuleEnvironmentObject env(cx, &module->initialEnvironment()); - if (!importBindings().putNew(cx, importNameId, env, localNameId)) { + if (!importBindings().put(cx, importNameId, env, localNameId)) { ReportOutOfMemory(cx); return false; } diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp index 0dfeffc36..f98e968b8 100644 --- a/js/src/vm/SelfHosting.cpp +++ b/js/src/vm/SelfHosting.cpp @@ -2087,7 +2087,6 @@ intrinsic_CreateNamespaceBinding(JSContext* cx, unsigned argc, Value* vp) // the slot directly. RootedShape shape(cx, environment->lookup(cx, name)); MOZ_ASSERT(shape); - MOZ_ASSERT(environment->getSlot(shape->slot()).isMagic(JS_UNINITIALIZED_LEXICAL)); environment->setSlot(shape->slot(), args[2]); args.rval().setUndefined(); return true; |