From 26285e2fa39184be28b356a262fcff1c8ff0a8fe Mon Sep 17 00:00:00 2001 From: Camillo Bruni Date: Thu, 5 Aug 2021 18:00:00 +0200 Subject: [PATCH] [modules] Update Module::Status enum to match spec Change-Id: Ia324f486f138757017951c0d2b83502937b950d9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3075362 Auto-Submit: Camillo Bruni Commit-Queue: Omer Katz Reviewed-by: Omer Katz Reviewed-by: Shu-yu Guo Cr-Commit-Position: refs/heads/master@{#76158} --- src/api/api.cc | 13 ++++++----- src/diagnostics/objects-debug.cc | 10 ++++---- src/heap/factory.cc | 4 ++-- src/objects/module.cc | 32 ++++++++++++------------- src/objects/module.h | 11 +++++---- src/objects/source-text-module.cc | 39 ++++++++++++++++--------------- src/objects/synthetic-module.cc | 2 +- test/cctest/test-api.cc | 2 +- 8 files changed, 57 insertions(+), 56 deletions(-) diff --git a/src/api/api.cc b/src/api/api.cc index b045ff8ca5..27dff9ac6a 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -2105,14 +2105,15 @@ Local ModuleRequest::GetImportAssertions() const { Module::Status Module::GetStatus() const { i::Handle self = Utils::OpenHandle(this); switch (self->status()) { - case i::Module::kUninstantiated: - case i::Module::kPreInstantiating: + case i::Module::kUnlinked: + case i::Module::kPreLinking: return kUninstantiated; - case i::Module::kInstantiating: + case i::Module::kLinking: return kInstantiating; - case i::Module::kInstantiated: + case i::Module::kLinked: return kInstantiated; case i::Module::kEvaluating: + case i::Module::kEvaluatingAsync: return kEvaluating; case i::Module::kEvaluated: return kEvaluated; @@ -2298,8 +2299,8 @@ MaybeLocal Module::Evaluate(Local context) { i::TimerEventScope timer_scope(isolate); i::Handle self = Utils::OpenHandle(this); - Utils::ApiCheck(self->status() >= i::Module::kInstantiated, - "Module::Evaluate", "Expected instantiated module"); + Utils::ApiCheck(self->status() >= i::Module::kLinked, "Module::Evaluate", + "Expected instantiated module"); Local result; has_pending_exception = !ToLocal(i::Module::Evaluate(isolate, self), &result); diff --git a/src/diagnostics/objects-debug.cc b/src/diagnostics/objects-debug.cc index 1b66be85ad..9254313802 100644 --- a/src/diagnostics/objects-debug.cc +++ b/src/diagnostics/objects-debug.cc @@ -1507,7 +1507,7 @@ void Module::ModuleVerify(Isolate* isolate) { CHECK(module_namespace().IsUndefined(isolate) || module_namespace().IsJSModuleNamespace()); if (module_namespace().IsJSModuleNamespace()) { - CHECK_LE(Module::kInstantiating, status()); + CHECK_LE(Module::kLinking, status()); CHECK_EQ(JSModuleNamespace::cast(module_namespace()).module(), *this); } @@ -1540,13 +1540,13 @@ void SourceTextModule::SourceTextModuleVerify(Isolate* isolate) { } else if (status() == kEvaluating || status() == kEvaluated) { CHECK(code().IsJSGeneratorObject()); } else { - if (status() == kInstantiated) { + if (status() == kLinked) { CHECK(code().IsJSGeneratorObject()); - } else if (status() == kInstantiating) { + } else if (status() == kLinking) { CHECK(code().IsJSFunction()); - } else if (status() == kPreInstantiating) { + } else if (status() == kPreLinking) { CHECK(code().IsSharedFunctionInfo()); - } else if (status() == kUninstantiated) { + } else if (status() == kUnlinked) { CHECK(code().IsSharedFunctionInfo()); } CHECK(!AsyncParentModuleCount()); diff --git a/src/heap/factory.cc b/src/heap/factory.cc index 33ba4e1739..d86a51b6db 100644 --- a/src/heap/factory.cc +++ b/src/heap/factory.cc @@ -2655,7 +2655,7 @@ Handle Factory::NewSourceTextModule( module.set_hash(isolate()->GenerateIdentityHash(Smi::kMaxValue)); module.set_module_namespace(roots.undefined_value(), SKIP_WRITE_BARRIER); module.set_requested_modules(*requested_modules); - module.set_status(Module::kUninstantiated); + module.set_status(Module::kUnlinked); module.set_exception(roots.the_hole_value(), SKIP_WRITE_BARRIER); module.set_top_level_capability(roots.undefined_value(), SKIP_WRITE_BARRIER); module.set_import_meta(roots.the_hole_value(), kReleaseStore, @@ -2686,7 +2686,7 @@ Handle Factory::NewSyntheticModule( DisallowGarbageCollection no_gc; module.set_hash(isolate()->GenerateIdentityHash(Smi::kMaxValue)); module.set_module_namespace(roots.undefined_value(), SKIP_WRITE_BARRIER); - module.set_status(Module::kUninstantiated); + module.set_status(Module::kUnlinked); module.set_exception(roots.the_hole_value(), SKIP_WRITE_BARRIER); module.set_top_level_capability(roots.undefined_value(), SKIP_WRITE_BARRIER); module.set_name(*module_name); diff --git a/src/objects/module.cc b/src/objects/module.cc index 95aefdf7cf..2945f36a14 100644 --- a/src/objects/module.cc +++ b/src/objects/module.cc @@ -103,8 +103,7 @@ void Module::RecordError(Isolate* isolate, Handle module, void Module::ResetGraph(Isolate* isolate, Handle module) { DCHECK_NE(module->status(), kEvaluating); - if (module->status() != kPreInstantiating && - module->status() != kInstantiating) { + if (module->status() != kPreLinking && module->status() != kLinking) { return; } @@ -130,8 +129,7 @@ void Module::ResetGraph(Isolate* isolate, Handle module) { } void Module::Reset(Isolate* isolate, Handle module) { - DCHECK(module->status() == kPreInstantiating || - module->status() == kInstantiating); + DCHECK(module->status() == kPreLinking || module->status() == kLinking); DCHECK(module->exception().IsTheHole(isolate)); // The namespace object cannot exist, because it would have been created // by RunInitializationCode, which is called only after this module's SCC @@ -148,7 +146,7 @@ void Module::Reset(Isolate* isolate, Handle module) { } module->set_exports(*exports); - SetStatusInternal(*module, kUninstantiated); + SetStatusInternal(*module, kUnlinked); } Object Module::GetException() { @@ -163,7 +161,7 @@ MaybeHandle Module::ResolveExport(Isolate* isolate, Handle module, Handle export_name, MessageLocation loc, bool must_resolve, Module::ResolveSet* resolve_set) { - DCHECK_GE(module->status(), kPreInstantiating); + DCHECK_GE(module->status(), kPreLinking); DCHECK_NE(module->status(), kEvaluating); if (module->IsSourceTextModule()) { @@ -188,7 +186,7 @@ bool Module::Instantiate( if (!PrepareInstantiate(isolate, module, context, callback, callback_without_import_assertions)) { ResetGraph(isolate, module); - DCHECK_EQ(module->status(), kUninstantiated); + DCHECK_EQ(module->status(), kUnlinked); return false; } Zone zone(isolate->allocator(), ZONE_NAME); @@ -196,10 +194,10 @@ bool Module::Instantiate( unsigned dfs_index = 0; if (!FinishInstantiate(isolate, module, &stack, &dfs_index, &zone)) { ResetGraph(isolate, module); - DCHECK_EQ(module->status(), kUninstantiated); + DCHECK_EQ(module->status(), kUnlinked); return false; } - DCHECK(module->status() == kInstantiated || module->status() == kEvaluated || + DCHECK(module->status() == kLinked || module->status() == kEvaluated || module->status() == kErrored); DCHECK(stack.empty()); return true; @@ -210,9 +208,9 @@ bool Module::PrepareInstantiate( v8::Module::ResolveModuleCallback callback, DeprecatedResolveCallback callback_without_import_assertions) { DCHECK_NE(module->status(), kEvaluating); - DCHECK_NE(module->status(), kInstantiating); - if (module->status() >= kPreInstantiating) return true; - module->SetStatus(kPreInstantiating); + DCHECK_NE(module->status(), kLinking); + if (module->status() >= kPreLinking) return true; + module->SetStatus(kPreLinking); STACK_CHECK(isolate, false); if (module->IsSourceTextModule()) { @@ -229,8 +227,8 @@ bool Module::FinishInstantiate(Isolate* isolate, Handle module, ZoneForwardList>* stack, unsigned* dfs_index, Zone* zone) { DCHECK_NE(module->status(), kEvaluating); - if (module->status() >= kInstantiating) return true; - DCHECK_EQ(module->status(), kPreInstantiating); + if (module->status() >= kLinking) return true; + DCHECK_EQ(module->status(), kPreLinking); STACK_CHECK(isolate, false); if (module->IsSourceTextModule()) { @@ -276,7 +274,7 @@ MaybeHandle Module::EvaluateMaybeAsync(Isolate* isolate, // Start of Evaluate () Concrete Method // 2. Assert: module.[[Status]] is "linked" or "evaluated". - CHECK(module->status() == kInstantiated || module->status() == kEvaluated); + CHECK(module->status() == kLinked || module->status() == kEvaluated); // 3. If module.[[Status]] is "evaluated", set module to // module.[[CycleRoot]]. @@ -316,7 +314,7 @@ MaybeHandle Module::InnerEvaluate(Isolate* isolate, // // However, SyntheticModules transition directly to 'Evaluated,' so we should // never see an 'Evaluating' module at this point. - CHECK_EQ(module->status(), kInstantiated); + CHECK_EQ(module->status(), kLinked); if (module->IsSourceTextModule()) { return SourceTextModule::Evaluate(isolate, @@ -446,7 +444,7 @@ bool Module::IsGraphAsync(Isolate* isolate) const { do { SourceTextModule current = worklist.back(); worklist.pop_back(); - DCHECK_GE(current.status(), kInstantiated); + DCHECK_GE(current.status(), kLinked); if (current.async()) return true; FixedArray requested_modules = current.requested_modules(); diff --git a/src/objects/module.h b/src/objects/module.h index e92c5ff40f..05ea04ccd9 100644 --- a/src/objects/module.h +++ b/src/objects/module.h @@ -40,11 +40,12 @@ class Module : public TorqueGeneratedModule { enum Status { // Order matters! - kUninstantiated, - kPreInstantiating, - kInstantiating, - kInstantiated, + kUnlinked, + kPreLinking, + kLinking, + kLinked, kEvaluating, + kEvaluatingAsync, kEvaluated, kErrored }; @@ -121,7 +122,7 @@ class Module : public TorqueGeneratedModule { static V8_WARN_UNUSED_RESULT MaybeHandle InnerEvaluate( Isolate* isolate, Handle module); - // Set module's status back to kUninstantiated and reset other internal state. + // Set module's status back to kUnlinked and reset other internal state. // This is used when instantiation fails. static void Reset(Isolate* isolate, Handle module); static void ResetGraph(Isolate* isolate, Handle module); diff --git a/src/objects/source-text-module.cc b/src/objects/source-text-module.cc index 957d36ba1f..3bc6807e49 100644 --- a/src/objects/source-text-module.cc +++ b/src/objects/source-text-module.cc @@ -89,13 +89,14 @@ struct SourceTextModule::AsyncEvaluatingOrdinalCompare { SharedFunctionInfo SourceTextModule::GetSharedFunctionInfo() const { DisallowGarbageCollection no_gc; switch (status()) { - case kUninstantiated: - case kPreInstantiating: + case kUnlinked: + case kPreLinking: return SharedFunctionInfo::cast(code()); - case kInstantiating: + case kLinking: return JSFunction::cast(code()).shared(); - case kInstantiated: + case kLinked: case kEvaluating: + case kEvaluatingAsync: case kEvaluated: return JSGeneratorObject::cast(code()).function().shared(); case kErrored: @@ -390,13 +391,13 @@ bool SourceTextModule::PrepareInstantiate( entry); } - DCHECK_EQ(module->status(), kPreInstantiating); + DCHECK_EQ(module->status(), kPreLinking); return true; } bool SourceTextModule::RunInitializationCode(Isolate* isolate, Handle module) { - DCHECK_EQ(module->status(), kInstantiating); + DCHECK_EQ(module->status(), kLinking); Handle function(JSFunction::cast(module->code()), isolate); DCHECK_EQ(MODULE_SCOPE, function->shared().scope_info().scope_type()); Handle receiver = isolate->factory()->undefined_value(); @@ -421,7 +422,7 @@ bool SourceTextModule::RunInitializationCode(Isolate* isolate, bool SourceTextModule::MaybeTransitionComponent( Isolate* isolate, Handle module, ZoneForwardList>* stack, Status new_status) { - DCHECK(new_status == kInstantiated || new_status == kEvaluated); + DCHECK(new_status == kLinked || new_status == kEvaluated); SLOW_DCHECK( // {module} is on the {stack}. std::count_if(stack->begin(), stack->end(), @@ -435,8 +436,8 @@ bool SourceTextModule::MaybeTransitionComponent( ancestor = stack->front(); stack->pop_front(); DCHECK_EQ(ancestor->status(), - new_status == kInstantiated ? kInstantiating : kEvaluating); - if (new_status == kInstantiated) { + new_status == kLinked ? kLinking : kEvaluating); + if (new_status == kLinked) { if (!SourceTextModule::RunInitializationCode(isolate, ancestor)) return false; } else if (new_status == kEvaluated) { @@ -461,7 +462,7 @@ bool SourceTextModule::FinishInstantiate( Factory::JSFunctionBuilder{isolate, shared, isolate->native_context()} .Build(); module->set_code(*function); - module->SetStatus(kInstantiating); + module->SetStatus(kLinking); module->set_dfs_index(*dfs_index); module->set_dfs_ancestor_index(*dfs_index); stack->push_front(module); @@ -478,16 +479,16 @@ bool SourceTextModule::FinishInstantiate( } DCHECK_NE(requested_module->status(), kEvaluating); - DCHECK_GE(requested_module->status(), kInstantiating); + DCHECK_GE(requested_module->status(), kLinking); SLOW_DCHECK( // {requested_module} is instantiating iff it's on the {stack}. - (requested_module->status() == kInstantiating) == + (requested_module->status() == kLinking) == std::count_if(stack->begin(), stack->end(), [&](Handle m) { return *m == *requested_module; })); - if (requested_module->status() == kInstantiating) { - // SyntheticModules go straight to kInstantiated so this must be a + if (requested_module->status() == kLinking) { + // SyntheticModules go straight to kLinked so this must be a // SourceTextModule module->set_dfs_ancestor_index(std::min( module->dfs_ancestor_index(), @@ -531,14 +532,14 @@ bool SourceTextModule::FinishInstantiate( } } - return MaybeTransitionComponent(isolate, module, stack, kInstantiated); + return MaybeTransitionComponent(isolate, module, stack, kLinked); } void SourceTextModule::FetchStarExports(Isolate* isolate, Handle module, Zone* zone, UnorderedModuleSet* visited) { - DCHECK_GE(module->status(), Module::kInstantiating); + DCHECK_GE(module->status(), Module::kLinking); if (module->module_namespace().IsJSModuleNamespace()) return; // Shortcut. @@ -729,7 +730,7 @@ MaybeHandle SourceTextModule::EvaluateMaybeAsync( MaybeHandle SourceTextModule::Evaluate( Isolate* isolate, Handle module) { // Evaluate () Concrete Method continued from EvaluateMaybeAsync. - CHECK(module->status() == kInstantiated || module->status() == kEvaluated); + CHECK(module->status() == kLinked || module->status() == kEvaluated); // 5. Let stack be a new empty List. Zone zone(isolate->allocator(), ZONE_NAME); @@ -1040,7 +1041,7 @@ MaybeHandle SourceTextModule::InnerModuleEvaluation( } // 4. Assert: module.[[Status]] is "linked". - CHECK_EQ(module->status(), kInstantiated); + CHECK_EQ(module->status(), kLinked); // 5. Set module.[[Status]] to "evaluating". module->SetStatus(kEvaluating); @@ -1189,7 +1190,7 @@ void SourceTextModule::Reset(Isolate* isolate, Handle requested_modules = factory->NewFixedArray(module->requested_modules().length()); - if (module->status() == kInstantiating) { + if (module->status() == kLinking) { module->set_code(JSFunction::cast(module->code()).shared()); } module->set_regular_exports(*regular_exports); diff --git a/src/objects/synthetic-module.cc b/src/objects/synthetic-module.cc index 451dcc5160..0322ca9b8a 100644 --- a/src/objects/synthetic-module.cc +++ b/src/objects/synthetic-module.cc @@ -92,7 +92,7 @@ bool SyntheticModule::PrepareInstantiate(Isolate* isolate, // just update status. bool SyntheticModule::FinishInstantiate(Isolate* isolate, Handle module) { - module->SetStatus(kInstantiated); + module->SetStatus(kLinked); return true; } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 67c7236e1d..e3bacd504f 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -24378,7 +24378,7 @@ TEST(CreateSyntheticModule) { .IsUndefined()); CHECK_EQ(i_module->export_names().length(), 1); CHECK(i::String::cast(i_module->export_names().get(0)).Equals(*default_name)); - CHECK_EQ(i_module->status(), i::Module::kInstantiated); + CHECK_EQ(i_module->status(), i::Module::kLinked); CHECK(module->IsSyntheticModule()); CHECK(!module->IsSourceTextModule()); CHECK_EQ(module->GetModuleRequests()->Length(), 0);