[runtime] Module code cleanup

- Make Module::RecordErrorUsingPendingException and
  Module::RecordError static (There is no need for them to be
  "fast" instance methods with raw pointers)
- Share various debug print snippets
- Share status change code in SetStatusInternal
- Simplify several casts

Change-Id: I159dc3dd9104bf76858a2d5ad142a72a75640716
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2416490
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Auto-Submit: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70094}

src/objects/module.cc

@@ -21,57 +21,74 @@
 namespace v8 {
 namespace internal {
 
+namespace {
 #ifdef DEBUG
-void Module::PrintStatusTransition(Status new_status) {
-  if (FLAG_trace_module_status) {
-    StdoutStream os;
-    os << "Changing module status from " << status() << " to " << new_status
-       << " for ";
-    if (this->IsSourceTextModule()) {
-      Handle<Script> script(SourceTextModule::cast(*this).script(),
-                            GetIsolate());
-      script->GetNameOrSourceURL().Print(os);
+void PrintModuleName(Module module, std::ostream& os) {
+  if (module.IsSourceTextModule()) {
+    SourceTextModule::cast(module).script().GetNameOrSourceURL().Print(os);
   } else {
-      SyntheticModule::cast(*this).name().Print(os);
+    SyntheticModule::cast(module).name().Print(os);
   }
 #ifndef OBJECT_PRINT
   os << "\n";
 #endif  // OBJECT_PRINT
-  }
 }
+
+void PrintStatusTransition(Module module, Module::Status new_status) {
+  if (!FLAG_trace_module_status) return;
+  StdoutStream os;
+  os << "Changing module status from " << module.status() << " to "
+     << new_status << " for ";
+  PrintModuleName(module, os);
+}
+
+void PrintStatusMessage(Module module, const char* message) {
+  if (!FLAG_trace_module_status) return;
+  StdoutStream os;
+  os << "Instantiating module ";
+  PrintModuleName(module, os);
+}
+#endif  // DEBUG
+
+void SetStatusInternal(Module module, Module::Status new_status) {
+  DisallowHeapAllocation no_alloc;
+#ifdef DEBUG
+  PrintStatusTransition(module, new_status);
 #endif  // DEBUG
+  module.set_status(new_status);
+}
+
+}  // end namespace
 
 void Module::SetStatus(Status new_status) {
   DisallowHeapAllocation no_alloc;
   DCHECK_LE(status(), new_status);
   DCHECK_NE(new_status, Module::kErrored);
-#ifdef DEBUG
-  PrintStatusTransition(new_status);
-#endif  // DEBUG
-  set_status(new_status);
+  SetStatusInternal(*this, new_status);
 }
 
-void Module::RecordErrorUsingPendingException(Isolate* isolate) {
+// static
+void Module::RecordErrorUsingPendingException(Isolate* isolate,
+                                              Handle<Module> module) {
   Handle<Object> the_exception(isolate->pending_exception(), isolate);
-  RecordError(isolate, the_exception);
+  RecordError(isolate, module, the_exception);
 }
 
-void Module::RecordError(Isolate* isolate, Handle<Object> error) {
-  DCHECK(exception().IsTheHole(isolate));
+// static
+void Module::RecordError(Isolate* isolate, Handle<Module> module,
+                         Handle<Object> error) {
+  DCHECK(module->exception().IsTheHole(isolate));
   DCHECK(!error->IsTheHole(isolate));
-  if (this->IsSourceTextModule()) {
-    Handle<SourceTextModule> self(SourceTextModule::cast(*this), GetIsolate());
+  if (module->IsSourceTextModule()) {
+    Handle<SourceTextModule> self(SourceTextModule::cast(*module), isolate);
     self->set_code(self->info());
   }
-#ifdef DEBUG
-  PrintStatusTransition(Module::kErrored);
-#endif  // DEBUG
-  set_status(Module::kErrored);
+  SetStatusInternal(*module, Module::kErrored);
   if (isolate->is_catchable_by_javascript(*error)) {
-    set_exception(*error);
+    module->set_exception(*error);
   } else {
     // v8::TryCatch uses `null` for termination exceptions.
-    set_exception(*isolate->factory()->null_value());
+    module->set_exception(*isolate->factory()->null_value());
   }
 }
 
@@ -85,11 +102,14 @@ void Module::ResetGraph(Isolate* isolate, Handle<Module> module) {
   Handle<FixedArray> requested_modules =
       module->IsSourceTextModule()
           ? Handle<FixedArray>(
-                Handle<SourceTextModule>::cast(module)->requested_modules(),
-                isolate)
+                SourceTextModule::cast(*module).requested_modules(), isolate)
           : Handle<FixedArray>();
   Reset(isolate, module);
-  if (module->IsSourceTextModule()) {
+
+  if (!module->IsSourceTextModule()) {
+    DCHECK(module->IsSyntheticModule());
+    return;
+  }
   for (int i = 0; i < requested_modules->length(); ++i) {
     Handle<Object> descendant(requested_modules->get(i), isolate);
     if (descendant->IsModule()) {
@@ -98,43 +118,31 @@ void Module::ResetGraph(Isolate* isolate, Handle<Module> module) {
       DCHECK(descendant->IsUndefined(isolate));
     }
   }
-  } else {
-    DCHECK(module->IsSyntheticModule());
-    // Nothing else to do here.
-  }
 }
 
 void Module::Reset(Isolate* isolate, Handle<Module> module) {
   DCHECK(module->status() == kPreInstantiating ||
          module->status() == kInstantiating);
-  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
   // succeeds instantiation.
   DCHECK(!module->module_namespace().IsJSModuleNamespace());
 
-#ifdef DEBUG
-  module->PrintStatusTransition(kUninstantiated);
-#endif  // DEBUG
-
-  const int export_count =
-      module->IsSourceTextModule()
-          ? Handle<SourceTextModule>::cast(module)->regular_exports().length()
-          : Handle<SyntheticModule>::cast(module)->export_names().length();
-  Handle<ObjectHashTable> exports = ObjectHashTable::New(isolate, export_count);
-
   if (module->IsSourceTextModule()) {
     SourceTextModule::Reset(isolate, Handle<SourceTextModule>::cast(module));
-  } else {
-    // Nothing to do here.
   }
 
+  const int export_count =
+      module->IsSourceTextModule()
+          ? SourceTextModule::cast(*module).regular_exports().length()
+          : SyntheticModule::cast(*module).export_names().length();
+  Handle<ObjectHashTable> exports = ObjectHashTable::New(isolate, export_count);
   module->set_exports(*exports);
-  module->set_status(kUninstantiated);
+  SetStatusInternal(*module, kUninstantiated);
 }
 
 Object Module::GetException() {
-  DisallowHeapAllocation no_alloc;
+  DisallowHeapAllocation no_gc;
   DCHECK_EQ(status(), Module::kErrored);
   DCHECK(!exception().IsTheHole());
   return exception();
@@ -163,21 +171,7 @@ bool Module::Instantiate(Isolate* isolate, Handle<Module> module,
                          v8::Local<v8::Context> context,
                          v8::Module::ResolveCallback callback) {
 #ifdef DEBUG
-  if (FLAG_trace_module_status) {
-    StdoutStream os;
-    os << "Instantiating module ";
-    if (module->IsSourceTextModule()) {
-      Handle<SourceTextModule>::cast(module)
-          ->script()
-          .GetNameOrSourceURL()
-          .Print(os);
-    } else {
-      Handle<SyntheticModule>::cast(module)->name().Print(os);
-    }
-#ifndef OBJECT_PRINT
-    os << "\n";
-#endif  // OBJECT_PRINT
-  }
+  PrintStatusMessage(*module, "Instantiating module ");
 #endif  // DEBUG
 
   if (!PrepareInstantiate(isolate, module, context, callback)) {
@@ -237,21 +231,7 @@ bool Module::FinishInstantiate(Isolate* isolate, Handle<Module> module,
 
 MaybeHandle<Object> Module::Evaluate(Isolate* isolate, Handle<Module> module) {
 #ifdef DEBUG
-  if (FLAG_trace_module_status) {
-    StdoutStream os;
-    os << "Evaluating module ";
-    if (module->IsSourceTextModule()) {
-      Handle<SourceTextModule>::cast(module)
-          ->script()
-          .GetNameOrSourceURL()
-          .Print(os);
-    } else {
-      Handle<SyntheticModule>::cast(module)->name().Print(os);
-    }
-#ifndef OBJECT_PRINT
-    os << "\n";
-#endif  // OBJECT_PRINT
-  }
+  PrintStatusMessage(*module, "Evaluating module ");
 #endif  // DEBUG
   STACK_CHECK(isolate, MaybeHandle<Object>());
   if (FLAG_harmony_top_level_await && module->IsSourceTextModule()) {
@@ -360,7 +340,7 @@ MaybeHandle<Object> JSModuleNamespace::GetExport(Isolate* isolate,
     return isolate->factory()->undefined_value();
   }
 
-  Handle<Object> value(Handle<Cell>::cast(object)->value(), isolate);
+  Handle<Object> value(Cell::cast(*object).value(), isolate);
   if (value->IsTheHole(isolate)) {
     THROW_NEW_ERROR(
         isolate, NewReferenceError(MessageTemplate::kNotDefined, name), Object);
@@ -378,9 +358,7 @@ Maybe<PropertyAttributes> JSModuleNamespace::GetPropertyAttributes(
   Isolate* isolate = it->isolate();
 
   Handle<Object> lookup(object->module().exports().Lookup(name), isolate);
-  if (lookup->IsTheHole(isolate)) {
-    return Just(ABSENT);
-  }
+  if (lookup->IsTheHole(isolate)) return Just(ABSENT);
 
   Handle<Object> value(Handle<Cell>::cast(lookup)->value(), isolate);
   if (value->IsTheHole(isolate)) {

src/objects/module.h

@@ -123,13 +123,10 @@ class Module : public HeapObject {
   // To set status to kErrored, RecordError or RecordErrorUsingPendingException
   // should be used.
   void SetStatus(Status status);
-  void RecordErrorUsingPendingException(Isolate* isolate);
-  void RecordError(Isolate* isolate, Handle<Object> error);
-
-#ifdef DEBUG
-  // For --trace-module-status.
-  void PrintStatusTransition(Status new_status);
-#endif  // DEBUG
+  static void RecordErrorUsingPendingException(Isolate* isolate,
+                                               Handle<Module>);
+  static void RecordError(Isolate* isolate, Handle<Module> module,
+                          Handle<Object> error);
 
   OBJECT_CONSTRUCTORS(Module, HeapObject);
 };

src/objects/source-text-module.cc

@@ -453,10 +453,9 @@ bool SourceTextModule::FinishInstantiate(
     if (requested_module->status() == kInstantiating) {
       // SyntheticModules go straight to kInstantiated so this must be a
       // SourceTextModule
-      module->set_dfs_ancestor_index(
-          std::min(module->dfs_ancestor_index(),
-                   Handle<SourceTextModule>::cast(requested_module)
-                       ->dfs_ancestor_index()));
+      module->set_dfs_ancestor_index(std::min(
+          module->dfs_ancestor_index(),
+          SourceTextModule::cast(*requested_module).dfs_ancestor_index()));
     }
   }
 
@@ -694,7 +693,7 @@ MaybeHandle<Object> SourceTextModule::Evaluate(
       CHECK_EQ(descendant->status(), kEvaluating);
       //  ii. Set m.[[Status]] to "evaluated".
       // iii. Set m.[[EvaluationError]] to result.
-      descendant->RecordErrorUsingPendingException(isolate);
+      Module::RecordErrorUsingPendingException(isolate, descendant);
     }
 
 #ifdef DEBUG
@@ -819,7 +818,7 @@ void SourceTextModule::AsyncModuleExecutionRejected(
   }
 
   // 4. Set module.[[EvaluationError]] to ThrowCompletion(error).
-  module->RecordError(isolate, exception);
+  Module::RecordError(isolate, module, exception);
 
   // 5. Set module.[[AsyncEvaluating]] to false.
   module->set_async_evaluating(false);

src/objects/synthetic-module.cc

@@ -30,9 +30,8 @@ Maybe<bool> SyntheticModule::SetExport(Isolate* isolate,
     return Nothing<bool>();
   }
 
-  Handle<Cell> export_cell(Handle<Cell>::cast(export_object));
   // Spec step 2: Set the mutable binding of export_name to export_value
-  export_cell->set_value(*export_value);
+  Cell::cast(*export_object).set_value(*export_value);
 
   return Just(true);
 }
@@ -56,18 +55,14 @@ MaybeHandle<Cell> SyntheticModule::ResolveExport(
     Handle<String> module_specifier, Handle<String> export_name,
     MessageLocation loc, bool must_resolve) {
   Handle<Object> object(module->exports().Lookup(export_name), isolate);
-  if (object->IsCell()) {
-    return Handle<Cell>::cast(object);
-  }
+  if (object->IsCell()) return Handle<Cell>::cast(object);
+
+  if (!must_resolve) return MaybeHandle<Cell>();
 
-  if (must_resolve) {
   return isolate->Throw<Cell>(
       isolate->factory()->NewSyntaxError(MessageTemplate::kUnresolvableExport,
                                          module_specifier, export_name),
       &loc);
-  }
-
-  return MaybeHandle<Cell>();
 }
 
 // Implements Synthetic Module Record's Instantiate concrete method :
@@ -116,7 +111,7 @@ MaybeHandle<Object> SyntheticModule::Evaluate(Isolate* isolate,
            Utils::ToLocal(Handle<Module>::cast(module)))
            .ToLocal(&result)) {
     isolate->PromoteScheduledException();
-    module->RecordErrorUsingPendingException(isolate);
+    Module::RecordErrorUsingPendingException(isolate, module);
     return MaybeHandle<Object>();
   }