[modules] Include module specifier in instantiation error messages.

Bug: chromium:780819
Change-Id: I07c5ff3cf955edb087a175ea2d71a35e0f520ec3
Reviewed-on: https://chromium-review.googlesource.com/813839
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49953}

src/messages.h

@@ -308,7 +308,7 @@ class ErrorUtils : public AllStatic {
   T(ConstructorNotFunction, "Constructor % requires 'new'")                    \
   T(ConstructorNotReceiver, "The .constructor property is not an object")      \
   T(CurrencyCode, "Currency code is required with currency style.")            \
-  T(CyclicModuleDependency, "Detected cycle while resolving name '%'")         \
+  T(CyclicModuleDependency, "Detected cycle while resolving name '%' in '%'")  \
   T(DataViewNotArrayBuffer,                                                    \
     "First argument to DataView constructor must be an ArrayBuffer")           \
   T(DateType, "this is not a Date object.")                                    \
@@ -564,7 +564,7 @@ class ErrorUtils : public AllStatic {
   T(ValueOutOfRange, "Value % out of range for % options property %")          \
   /* SyntaxError */                                                            \
   T(AmbiguousExport,                                                           \
-    "The requested module contains conflicting star exports for name '%'")     \
+    "The requested module '%' contains conflicting star exports for name '%'") \
   T(BadGetterArity, "Getter must not have any formal parameters.")             \
   T(BadSetterArity, "Setter must have exactly one formal parameter.")          \
   T(BigIntInvalidString, "Invalid BigInt string")                              \
@@ -699,7 +699,7 @@ class ErrorUtils : public AllStatic {
     "Lexical declaration cannot appear in a single-statement context")         \
   T(UnknownLabel, "Undefined label '%'")                                       \
   T(UnresolvableExport,                                                        \
-    "The requested module does not provide an export named '%'")               \
+    "The requested module '%' does not provide an export named '%'")           \
   T(UnterminatedArgList, "missing ) after argument list")                      \
   T(UnterminatedRegExp, "Invalid regular expression: missing /")               \
   T(UnterminatedTemplate, "Unterminated template literal")                     \

src/objects/module.cc

@@ -244,8 +244,11 @@ MaybeHandle<Cell> Module::ResolveImport(Handle<Module> module,
   Isolate* isolate = module->GetIsolate();
   Handle<Module> requested_module(
       Module::cast(module->requested_modules()->get(module_request)), isolate);
-  MaybeHandle<Cell> result = Module::ResolveExport(requested_module, name, loc,
-                                                   must_resolve, resolve_set);
+  Handle<String> specifier(
+      String::cast(module->info()->module_requests()->get(module_request)),
+      isolate);
+  MaybeHandle<Cell> result = Module::ResolveExport(
+      requested_module, specifier, name, loc, must_resolve, resolve_set);
   if (isolate->has_pending_exception()) {
     DCHECK(result.is_null());
     if (must_resolve) module->RecordError();
@@ -258,7 +261,8 @@ MaybeHandle<Cell> Module::ResolveImport(Handle<Module> module,
 }
 
 MaybeHandle<Cell> Module::ResolveExport(Handle<Module> module,
-                                        Handle<String> name,
+                                        Handle<String> module_specifier,
+                                        Handle<String> export_name,
                                         MessageLocation loc, bool must_resolve,
                                         Module::ResolveSet* resolve_set) {
   DCHECK_NE(module->status(), kErrored);
@@ -266,7 +270,7 @@ MaybeHandle<Cell> Module::ResolveExport(Handle<Module> module,
   DCHECK_GE(module->status(), kPreInstantiating);
 
   Isolate* isolate = module->GetIsolate();
-  Handle<Object> object(module->exports()->Lookup(name), isolate);
+  Handle<Object> object(module->exports()->Lookup(export_name), isolate);
   if (object->IsCell()) {
     // Already resolved (e.g. because it's a local export).
     return Handle<Cell>::cast(object);
@@ -282,17 +286,18 @@ MaybeHandle<Cell> Module::ResolveExport(Handle<Module> module,
       Zone* zone = resolve_set->zone();
       name_set =
           new (zone->New(sizeof(UnorderedStringSet))) UnorderedStringSet(zone);
-    } else if (name_set->count(name)) {
+    } else if (name_set->count(export_name)) {
       // Cycle detected.
       if (must_resolve) {
         return isolate->Throw<Cell>(
             isolate->factory()->NewSyntaxError(
-                MessageTemplate::kCyclicModuleDependency, name),
+                MessageTemplate::kCyclicModuleDependency, export_name,
+                module_specifier),
             &loc);
       }
       return MaybeHandle<Cell>();
     }
-    name_set->insert(name);
+    name_set->insert(export_name);
   }
 
   if (object->IsModuleInfoEntry()) {
@@ -313,23 +318,24 @@ MaybeHandle<Cell> Module::ResolveExport(Handle<Module> module,
     // The export table may have changed but the entry in question should be
     // unchanged.
     Handle<ObjectHashTable> exports(module->exports(), isolate);
-    DCHECK(exports->Lookup(name)->IsModuleInfoEntry());
+    DCHECK(exports->Lookup(export_name)->IsModuleInfoEntry());
 
-    exports = ObjectHashTable::Put(exports, name, cell);
+    exports = ObjectHashTable::Put(exports, export_name, cell);
     module->set_exports(*exports);
     return cell;
   }
 
   DCHECK(object->IsTheHole(isolate));
-  return Module::ResolveExportUsingStarExports(module, name, loc, must_resolve,
-                                               resolve_set);
+  return Module::ResolveExportUsingStarExports(
+      module, module_specifier, export_name, loc, must_resolve, resolve_set);
 }
 
 MaybeHandle<Cell> Module::ResolveExportUsingStarExports(
-    Handle<Module> module, Handle<String> name, MessageLocation loc,
-    bool must_resolve, Module::ResolveSet* resolve_set) {
+    Handle<Module> module, Handle<String> module_specifier,
+    Handle<String> export_name, MessageLocation loc, bool must_resolve,
+    Module::ResolveSet* resolve_set) {
   Isolate* isolate = module->GetIsolate();
-  if (!name->Equals(isolate->heap()->default_string())) {
+  if (!export_name->Equals(isolate->heap()->default_string())) {
     // Go through all star exports looking for the given name.  If multiple star
     // exports provide the name, make sure they all map it to the same cell.
     Handle<Cell> unique_cell;
@@ -346,15 +352,15 @@ MaybeHandle<Cell> Module::ResolveExportUsingStarExports(
       MessageLocation new_loc(script, entry->beg_pos(), entry->end_pos());
 
       Handle<Cell> cell;
-      if (ResolveImport(module, name, entry->module_request(), new_loc, false,
-                        resolve_set)
+      if (ResolveImport(module, export_name, entry->module_request(), new_loc,
+                        false, resolve_set)
               .ToHandle(&cell)) {
         if (unique_cell.is_null()) unique_cell = cell;
         if (*unique_cell != *cell) {
-          return isolate->Throw<Cell>(
-              isolate->factory()->NewSyntaxError(
-                  MessageTemplate::kAmbiguousExport, name),
-              &loc);
+          return isolate->Throw<Cell>(isolate->factory()->NewSyntaxError(
+                                          MessageTemplate::kAmbiguousExport,
+                                          module_specifier, export_name),
+                                      &loc);
         }
       } else if (isolate->has_pending_exception()) {
         return MaybeHandle<Cell>();
@@ -364,8 +370,8 @@ MaybeHandle<Cell> Module::ResolveExportUsingStarExports(
     if (!unique_cell.is_null()) {
       // Found a unique star export for this name.
       Handle<ObjectHashTable> exports(module->exports(), isolate);
-      DCHECK(exports->Lookup(name)->IsTheHole(isolate));
-      exports = ObjectHashTable::Put(exports, name, unique_cell);
+      DCHECK(exports->Lookup(export_name)->IsTheHole(isolate));
+      exports = ObjectHashTable::Put(exports, export_name, unique_cell);
       module->set_exports(*exports);
       return unique_cell;
     }
@@ -373,9 +379,10 @@ MaybeHandle<Cell> Module::ResolveExportUsingStarExports(
 
   // Unresolvable.
   if (must_resolve) {
-    return isolate->Throw<Cell>(isolate->factory()->NewSyntaxError(
-                                    MessageTemplate::kUnresolvableExport, name),
-                                &loc);
+    return isolate->Throw<Cell>(
+        isolate->factory()->NewSyntaxError(MessageTemplate::kUnresolvableExport,
+                                           module_specifier, export_name),
+        &loc);
   }
   return MaybeHandle<Cell>();
 }
@@ -606,8 +613,8 @@ bool Module::FinishInstantiate(Handle<Module> module,
     if (name->IsUndefined(isolate)) continue;  // Star export.
     MessageLocation loc(script, entry->beg_pos(), entry->end_pos());
     ResolveSet resolve_set(zone);
-    if (ResolveExport(module, Handle<String>::cast(name), loc, true,
-                      &resolve_set)
+    if (ResolveExport(module, Handle<String>(), Handle<String>::cast(name), loc,
+                      true, &resolve_set)
             .is_null()) {
       return false;
     }

src/objects/module.h

@@ -154,15 +154,17 @@ class Module : public Struct {
   // exception (so check manually!).
   class ResolveSet;
   static MUST_USE_RESULT MaybeHandle<Cell> ResolveExport(
-      Handle<Module> module, Handle<String> name, MessageLocation loc,
-      bool must_resolve, ResolveSet* resolve_set);
+      Handle<Module> module, Handle<String> module_specifier,
+      Handle<String> export_name, MessageLocation loc, bool must_resolve,
+      ResolveSet* resolve_set);
   static MUST_USE_RESULT MaybeHandle<Cell> ResolveImport(
       Handle<Module> module, Handle<String> name, int module_request,
       MessageLocation loc, bool must_resolve, ResolveSet* resolve_set);
 
   static MUST_USE_RESULT MaybeHandle<Cell> ResolveExportUsingStarExports(
-      Handle<Module> module, Handle<String> name, MessageLocation loc,
-      bool must_resolve, ResolveSet* resolve_set);
+      Handle<Module> module, Handle<String> module_specifier,
+      Handle<String> export_name, MessageLocation loc, bool must_resolve,
+      ResolveSet* resolve_set);
 
   static MUST_USE_RESULT bool PrepareInstantiate(
       Handle<Module> module, v8::Local<v8::Context> context,

test/message/fail/modules-cycle1.out

-*%(basename)s:7: SyntaxError: Detected cycle while resolving name 'a'
+*%(basename)s:7: SyntaxError: Detected cycle while resolving name 'a' in 'modules-cycle1.js'
 import {a} from "modules-cycle1.js";
         ^
-SyntaxError: Detected cycle while resolving name 'a'
+SyntaxError: Detected cycle while resolving name 'a' in 'modules-cycle1.js'
 

test/message/fail/modules-cycle2.out

-*%(basename)s:7: SyntaxError: Detected cycle while resolving name 'a'
+*%(basename)s:7: SyntaxError: Detected cycle while resolving name 'a' in 'modules-skip-cycle2.js'
 import {a} from "modules-skip-cycle2.js";
         ^
-SyntaxError: Detected cycle while resolving name 'a'
+SyntaxError: Detected cycle while resolving name 'a' in 'modules-skip-cycle2.js'
 

test/message/fail/modules-cycle3.out

-*%(basename)s:7: SyntaxError: Detected cycle while resolving name 'a'
+*%(basename)s:7: SyntaxError: Detected cycle while resolving name 'a' in 'modules-skip-cycle3.js'
 export {a as x} from "modules-skip-cycle3.js";
         ^^^^^^
-SyntaxError: Detected cycle while resolving name 'a'
+SyntaxError: Detected cycle while resolving name 'a' in 'modules-skip-cycle3.js'
 

test/message/fail/modules-cycle4.out

-*modules-cycle3.js:7: SyntaxError: Detected cycle while resolving name 'a'
+*modules-cycle3.js:7: SyntaxError: Detected cycle while resolving name 'a' in 'modules-skip-cycle3.js'
 export {a as x} from "modules-skip-cycle3.js";
         ^^^^^^
-SyntaxError: Detected cycle while resolving name 'a'
+SyntaxError: Detected cycle while resolving name 'a' in 'modules-skip-cycle3.js'
 

test/message/fail/modules-cycle5.out

-*%(basename)s:8: SyntaxError: Detected cycle while resolving name 'foo'
+*%(basename)s:8: SyntaxError: Detected cycle while resolving name 'foo' in 'modules-cycle5.js'
 export {foo} from "modules-cycle5.js";
         ^^^
-SyntaxError: Detected cycle while resolving name 'foo'
+SyntaxError: Detected cycle while resolving name 'foo' in 'modules-cycle5.js'
 

test/message/fail/modules-cycle6.out

-*modules-skip-cycle6.js:5: SyntaxError: The requested module does not provide an export named 'foo'
+*modules-skip-cycle6.js:5: SyntaxError: The requested module 'modules-cycle6.js' does not provide an export named 'foo'
 export {foo} from "modules-cycle6.js";
         ^^^
-SyntaxError: The requested module does not provide an export named 'foo'
+SyntaxError: The requested module 'modules-cycle6.js' does not provide an export named 'foo'
 

test/message/fail/modules-import1.out

-*%(basename)s:7: SyntaxError: The requested module does not provide an export named 'a'
+*%(basename)s:7: SyntaxError: The requested module 'modules-import1.js' does not provide an export named 'a'
 import {a} from "modules-import1.js";
         ^
-SyntaxError: The requested module does not provide an export named 'a'
+SyntaxError: The requested module 'modules-import1.js' does not provide an export named 'a'
 

test/message/fail/modules-import2.out

-*%(basename)s:7: SyntaxError: The requested module does not provide an export named 'a'
+*%(basename)s:7: SyntaxError: The requested module 'modules-import2.js' does not provide an export named 'a'
 import {a as b} from "modules-import2.js";
         ^
-SyntaxError: The requested module does not provide an export named 'a'
+SyntaxError: The requested module 'modules-import2.js' does not provide an export named 'a'
 

test/message/fail/modules-import3.out

-*%(basename)s:7: SyntaxError: The requested module does not provide an export named 'default'
+*%(basename)s:7: SyntaxError: The requested module 'modules-import3.js' does not provide an export named 'default'
 import foo from "modules-import3.js";
        ^^^
-SyntaxError: The requested module does not provide an export named 'default'
+SyntaxError: The requested module 'modules-import3.js' does not provide an export named 'default'
 

test/message/fail/modules-import4.out

-*%(basename)s:8: SyntaxError: The requested module does not provide an export named 'c'
+*%(basename)s:8: SyntaxError: The requested module 'modules-import4.js' does not provide an export named 'c'
 export {c as a} from "modules-import4.js";
         ^^^^^^
-SyntaxError: The requested module does not provide an export named 'c'
+SyntaxError: The requested module 'modules-import4.js' does not provide an export named 'c'
 

test/message/fail/modules-import5.out

-*%(basename)s:8: SyntaxError: The requested module does not provide an export named 'c'
+*%(basename)s:8: SyntaxError: The requested module 'modules-import5.js' does not provide an export named 'c'
 export {c as a} from "modules-import5.js";
         ^^^^^^
-SyntaxError: The requested module does not provide an export named 'c'
+SyntaxError: The requested module 'modules-import5.js' does not provide an export named 'c'
 

test/message/fail/modules-import6.out

-*%(basename)s:9: SyntaxError: The requested module does not provide an export named 'c'
+*%(basename)s:9: SyntaxError: The requested module 'modules-import6.js' does not provide an export named 'c'
 import {c} from "modules-import6.js";
         ^
-SyntaxError: The requested module does not provide an export named 'c'
+SyntaxError: The requested module 'modules-import6.js' does not provide an export named 'c'
 

test/message/fail/modules-star-conflict1.out

-*%(basename)s:7: SyntaxError: The requested module contains conflicting star exports for name 'a'
+*%(basename)s:7: SyntaxError: The requested module '../../mjsunit/modules-skip-7.js' contains conflicting star exports for name 'a'
 import {a} from "../../mjsunit/modules-skip-7.js";
         ^
-SyntaxError: The requested module contains conflicting star exports for name 'a'
+SyntaxError: The requested module '../../mjsunit/modules-skip-7.js' contains conflicting star exports for name 'a'
 

test/message/fail/modules-star-conflict2.out

-*%(basename)s:7: SyntaxError: The requested module contains conflicting star exports for name 'a'
+*%(basename)s:7: SyntaxError: The requested module '../../mjsunit/modules-skip-star-exports-conflict.js' contains conflicting star exports for name 'a'
 export * from "../../mjsunit/modules-skip-star-exports-conflict.js";
        ^
-SyntaxError: The requested module contains conflicting star exports for name 'a'
+SyntaxError: The requested module '../../mjsunit/modules-skip-star-exports-conflict.js' contains conflicting star exports for name 'a'
 

test/message/fail/modules-star-default.out

-*modules-import4.js:8: SyntaxError: The requested module does not provide an export named 'c'
+*modules-import4.js:8: SyntaxError: The requested module 'modules-import4.js' does not provide an export named 'c'
 export {c as a} from "modules-import4.js";
         ^^^^^^
-SyntaxError: The requested module does not provide an export named 'c'
+SyntaxError: The requested module 'modules-import4.js' does not provide an export named 'c'
 

test/mjsunit/harmony/modules-import-15.js

@@ -29,7 +29,8 @@ async function test2() {
   } catch(e) {
     assertInstanceof(e, SyntaxError);
     assertEquals(
-      "The requested module does not provide an export named 'default'",
+      "The requested module 'modules-skip-empty.js' does not provide an " +
+      "export named 'default'",
       e.message);
     ran = true;
   }