[ESNext] Implement DynamicImportCall

src/d8.cc

Index: src/d8.cc
diff --git a/src/d8.cc b/src/d8.cc
index 8745d41027e61ead49916d78fc6cc0e8d99e3ae6..e92044e4d17c07940ed33c421ec2ae243a990f47 100644
--- a/src/d8.cc
+++ b/src/d8.cc
@@ -440,6 +440,8 @@ ScriptCompiler::CachedData* CompileForCachedData(
   }
   Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = Shell::array_buffer_allocator;
+  create_params.host_import_module_dynamically_callback_ =
+      Shell::HostImportModuleDynamically;
   Isolate* temp_isolate = Isolate::New(create_params);
   ScriptCompiler::CachedData* result = NULL;
   {
@@ -448,19 +450,21 @@ ScriptCompiler::CachedData* CompileForCachedData(
     Context::Scope context_scope(Context::New(temp_isolate));
     Local<String> source_copy =
         v8::String::NewFromTwoByte(temp_isolate, source_buffer,
-                                   v8::NewStringType::kNormal,
-                                   source_length).ToLocalChecked();
+                                   v8::NewStringType::kNormal, source_length)
+            .ToLocalChecked();
     Local<Value> name_copy;
     if (name_buffer) {
-      name_copy = v8::String::NewFromTwoByte(temp_isolate, name_buffer,
-                                             v8::NewStringType::kNormal,
-                                             name_length).ToLocalChecked();
+      name_copy =
+          v8::String::NewFromTwoByte(temp_isolate, name_buffer,
+                                     v8::NewStringType::kNormal, name_length)
+              .ToLocalChecked();
     } else {
       name_copy = v8::Undefined(temp_isolate);
     }
     ScriptCompiler::Source script_source(source_copy, ScriptOrigin(name_copy));
     if (!ScriptCompiler::CompileUnboundScript(temp_isolate, &script_source,
-                                              compile_options).IsEmpty() &&
+                                              compile_options)
+             .IsEmpty() &&
         script_source.GetCachedData()) {
       int length = script_source.GetCachedData()->length;
       uint8_t* cache = new uint8_t[length];
@@ -685,12 +689,10 @@ MaybeLocal<Module> Shell::FetchModuleTree(Local<Context> context,
                                           const std::string& file_name) {
   DCHECK(IsAbsolutePath(file_name));
   Isolate* isolate = context->GetIsolate();
-  TryCatch try_catch(isolate);
-  try_catch.SetVerbose(true);
   Local<String> source_text = ReadFile(isolate, file_name.c_str());
   if (source_text.IsEmpty()) {
     printf("Error reading '%s'\n", file_name.c_str());
-    Shell::Exit(1);
+    return MaybeLocal<Module>();
   }
   ScriptOrigin origin(
       String::NewFromUtf8(isolate, file_name.c_str(), NewStringType::kNormal)
@@ -700,7 +702,6 @@ MaybeLocal<Module> Shell::FetchModuleTree(Local<Context> context,
   ScriptCompiler::Source source(source_text, origin);
   Local<Module> module;
   if (!ScriptCompiler::CompileModule(isolate, &source).ToLocal(&module)) {
-    ReportException(isolate, &try_catch);
     return MaybeLocal<Module>();

[neis, 2017/03/17 10:36:31]

I'm confused by these changes. Don't we want to there to be an exception when
FetchModuleTree returns with nothing?

[gsathya, 2017/03/17 21:47:59]

The exception isn't caught here, but in the caller of FetchModuleTree. See
DoHostImportModuleDynamically and ExecuteModule

[neis, 2017/03/20 09:36:03]

Yes but there is no exception to catch! That's why d8 crashes, see the top of my
previous mail.

[gsathya, 2017/03/22 03:08:34]

The problem here is that throwing an exception here with isolate->ThrowException
will not be caught by the outer TryCatch.

[neis, 2017/03/22 10:08:20]

I made the following change:

--- a/src/d8.cc
+++ b/src/d8.cc
@@ -690,6 +690,7 @@ MaybeLocal<Module> Shell::FetchModuleTree(Local<Context>
context,
   Local<String> source_text = ReadFile(isolate, file_name.c_str());
   if (source_text.IsEmpty()) {
     printf("Error reading '%s'\n", file_name.c_str());
+    Throw(isolate, "Some error message");
     return MaybeLocal<Module>();
   }


And this seems to have the desired effect:

$ d8 Debug --harmony-dynamic-import -e 'import("ffsda").catch(print)'
Error reading '/usr/local/google/home/neis/v8/ffsda'
Some error message

Am I missing something?

   }
 
@@ -727,6 +728,87 @@ MaybeLocal<Module> Shell::FetchModuleTree(Local<Context> context,
   return module;
 }
 
+namespace {
+
+struct DynamicImportData {
+ public:

[neis, 2017/03/17 10:36:31]

No need for "public:" here.

[gsathya, 2017/03/17 21:47:59]

Done.

+  DynamicImportData(Isolate* isolate_, Local<String> referrer_,
+                    Local<String> specifier_, Local<Promise> promise_)
+      : isolate(isolate_) {
+    referrer.Reset(isolate, referrer_);
+    specifier.Reset(isolate, specifier_);
+    promise.Reset(isolate, promise_);
+  }
+
+  Isolate* isolate;
+  Global<String> referrer;
+  Global<String> specifier;
+  Global<Promise> promise;
+};
+
+}  // namespace
+void Shell::HostImportModuleDynamically(Isolate* isolate,
+                                        Local<String> referrer,
+                                        Local<String> specifier,
+                                        Local<Promise> promise) {
+  DynamicImportData* data =
+      new DynamicImportData(isolate, referrer, specifier, promise);
+  isolate->EnqueueMicrotask(Shell::DoHostImportModuleDynamically, data);
+}
+
+void Shell::DoHostImportModuleDynamically(void* import_data) {
+  std::unique_ptr<DynamicImportData> import_data_(
+      static_cast<DynamicImportData*>(import_data));
+  Isolate* isolate(import_data_->isolate);
+  HandleScope handle_scope(isolate);
+
+  Local<String> referrer(import_data_->referrer.Get(isolate));
+  Local<String> specifier(import_data_->specifier.Get(isolate));
+  Local<Promise> promise(import_data_->promise.Get(isolate));
+
+  PerIsolateData* data = PerIsolateData::Get(isolate);
+  Local<Context> realm = data->realms_[data->realm_current_].Get(isolate);
+  Context::Scope context_scope(realm);
+
+  std::string source_url = ToSTLString(referrer);
+  std::string dir_name =
+      IsAbsolutePath(source_url) ? DirName(source_url) : GetWorkingDirectory();
+  std::string file_name = ToSTLString(specifier);
+  std::string absolute_path = NormalizePath(file_name.c_str(), dir_name);
+
+  TryCatch try_catch(isolate);
+  try_catch.SetVerbose(true);
+
+  ModuleEmbedderData* d = GetModuleDataFromContext(realm);
+  Local<Module> root_module;
+  auto module_it = d->specifier_to_module_map.find(absolute_path);
+  if (module_it != d->specifier_to_module_map.end()) {
+    root_module = module_it->second.Get(isolate);
+  } else if (!FetchModuleTree(realm, absolute_path).ToLocal(&root_module)) {
+    CHECK(try_catch.HasCaught());
+    CHECK(Module::FinishDynamicImportFailure(realm, promise,
+                                             try_catch.Exception()));
+    return;
+  }
+
+  MaybeLocal<Value> maybe_result;
+  if (root_module->Instantiate(realm, ResolveModuleCallback)) {
+    maybe_result = root_module->Evaluate(realm);
+    EmptyMessageQueues(isolate);

[neis, 2017/03/17 12:43:07]

Can you explain why this call is needed? (I'm not saying it isn't.)

[gsathya, 2017/03/17 21:47:59]

This is from ExecuteModule (I tried to mirror the module loading behavior here).
Adam maybe knows better? 

+  }
+
+  Local<Value> result;
+  if (!maybe_result.ToLocal(&result)) {
+    DCHECK(try_catch.HasCaught());
+    CHECK(Module::FinishDynamicImportFailure(realm, promise,
+                                             try_catch.Exception()));
+    return;
+  }
+
+  DCHECK(!try_catch.HasCaught());
+  CHECK(Module::FinishDynamicImportSuccess(realm, promise, root_module));
+}
+
 bool Shell::ExecuteModule(Isolate* isolate, const char* file_name) {
   HandleScope handle_scope(isolate);
 
@@ -736,14 +818,18 @@ bool Shell::ExecuteModule(Isolate* isolate, const char* file_name) {
 
   std::string absolute_path = NormalizePath(file_name, GetWorkingDirectory());
 
+  TryCatch try_catch(isolate);
+  try_catch.SetVerbose(true);
+
   Local<Module> root_module;
+  MaybeLocal<Value> maybe_exception;
+
   if (!FetchModuleTree(realm, absolute_path).ToLocal(&root_module)) {
+    CHECK(try_catch.HasCaught());
+    ReportException(isolate, &try_catch);
     return false;
   }
 
-  TryCatch try_catch(isolate);
-  try_catch.SetVerbose(true);
-
   MaybeLocal<Value> maybe_result;
   if (root_module->Instantiate(realm, ResolveModuleCallback)) {
     maybe_result = root_module->Evaluate(realm);
@@ -2166,6 +2252,8 @@ base::Thread::Options SourceGroup::GetThreadOptions() {
 void SourceGroup::ExecuteInThread() {
   Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = Shell::array_buffer_allocator;
+  create_params.host_import_module_dynamically_callback_ =
+      Shell::HostImportModuleDynamically;
   Isolate* isolate = Isolate::New(create_params);
   for (int i = 0; i < Shell::options.stress_runs; ++i) {
     next_semaphore_.Wait();
@@ -2304,6 +2392,8 @@ void Worker::WaitForThread() {
 void Worker::ExecuteInThread() {
   Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = Shell::array_buffer_allocator;
+  create_params.host_import_module_dynamically_callback_ =
+      Shell::HostImportModuleDynamically;
   Isolate* isolate = Isolate::New(create_params);
   {
     Isolate::Scope iscope(isolate);
@@ -2965,6 +3055,9 @@ int Shell::Main(int argc, char* argv[]) {
     create_params.add_histogram_sample_callback = AddHistogramSample;
   }
 
+  create_params.host_import_module_dynamically_callback_ =
+      Shell::HostImportModuleDynamically;
+
   if (i::trap_handler::UseTrapHandler()) {
     if (!v8::V8::RegisterDefaultSignalHandler()) {
       fprintf(stderr, "Could not register signal handler");