[modules] Split exports into regular and special, store regular ones in a multimap.

BUG=v8:1569

Review-Url: https://codereview.chromium.org/2273013002
Cr-Commit-Position: refs/heads/master@{#38889}

src/ast/modules.cc

@@ -54,7 +54,7 @@ void ModuleDescriptor::AddExport(
   ModuleEntry* entry = new (zone) ModuleEntry(loc);
   entry->export_name = export_name;
   entry->local_name = local_name;
-  exports_.Add(entry, zone);
+  regular_exports_.insert(std::make_pair(entry->local_name, entry));
 }
 
 
@@ -68,7 +68,7 @@ void ModuleDescriptor::AddExport(
   entry->export_name = export_name;
   entry->import_name = import_name;
   entry->module_request = module_request;
-  exports_.Add(entry, zone);
+  special_exports_.Add(entry, zone);
 }
 
 
@@ -77,26 +77,47 @@ void ModuleDescriptor::AddStarExport(
   DCHECK_NOT_NULL(module_request);
   ModuleEntry* entry = new (zone) ModuleEntry(loc);
   entry->module_request = module_request;
-  exports_.Add(entry, zone);
+  special_exports_.Add(entry, zone);
 }
 
-void ModuleDescriptor::MakeIndirectExportsExplicit() {
-  for (auto entry : exports_) {
-    if (entry->export_name == nullptr) continue;
-    if (entry->import_name != nullptr) continue;
+void ModuleDescriptor::MakeIndirectExportsExplicit(Zone* zone) {
+  for (auto it = regular_exports_.begin(); it != regular_exports_.end();) {
+    ModuleEntry* entry = it->second;
     DCHECK_NOT_NULL(entry->local_name);
-    auto it = regular_imports_.find(entry->local_name);
-    if (it != regular_imports_.end()) {
-      // Found an indirect export.
-      DCHECK_NOT_NULL(it->second->module_request);
-      DCHECK_NOT_NULL(it->second->import_name);
-      entry->import_name = it->second->import_name;
-      entry->module_request = it->second->module_request;
+    auto import = regular_imports_.find(entry->local_name);
+    if (import != regular_imports_.end()) {
+      // Found an indirect export.  Patch export entry and move it from regular
+      // to special.
+      DCHECK_NULL(entry->import_name);
+      DCHECK_NULL(entry->module_request);
+      DCHECK_NOT_NULL(import->second->import_name);
+      DCHECK_NOT_NULL(import->second->module_request);
+      entry->import_name = import->second->import_name;
+      entry->module_request = import->second->module_request;
       entry->local_name = nullptr;
+      special_exports_.Add(entry, zone);
+      it = regular_exports_.erase(it);
+    } else {
+      it++;
     }
   }
 }
 
+const ModuleDescriptor::ModuleEntry* ModuleDescriptor::FindDuplicateExport(
+    Zone* zone) const {
+  ZoneSet<const AstRawString*> export_names(zone);
+  for (const auto& it : regular_exports_) {
+    const ModuleEntry* entry = it.second;
+    DCHECK_NOT_NULL(entry->export_name);
+    if (!export_names.insert(entry->export_name).second) return entry;
+  }
+  for (auto entry : special_exports_) {
+    if (entry->export_name == nullptr) continue;  // Star export.
+    if (!export_names.insert(entry->export_name).second) return entry;
+  }
+  return nullptr;
+}
+
 bool ModuleDescriptor::Validate(ModuleScope* module_scope,
                                 PendingCompilationErrorHandler* error_handler,
                                 Zone* zone) {
@@ -105,29 +126,19 @@ bool ModuleDescriptor::Validate(ModuleScope* module_scope,
 
   // Report error iff there are duplicate exports.
   {
-    ZoneAllocationPolicy allocator(zone);
-    ZoneHashMap* export_names = new (zone->New(sizeof(ZoneHashMap)))
-        ZoneHashMap(ZoneHashMap::PointersMatch,
-                    ZoneHashMap::kDefaultHashMapCapacity, allocator);
-    for (auto entry : exports_) {
-      if (entry->export_name == nullptr) continue;
-      AstRawString* key = const_cast<AstRawString*>(entry->export_name);
-      ZoneHashMap::Entry* p =
-          export_names->LookupOrInsert(key, key->hash(), allocator);
-      DCHECK_NOT_NULL(p);
-      if (p->value != nullptr) {
-        error_handler->ReportMessageAt(
-            entry->location.beg_pos, entry->location.end_pos,
-            MessageTemplate::kDuplicateExport, entry->export_name);
-        return false;
-      }
-      p->value = key;  // Anything but nullptr.
+    const ModuleEntry* entry = FindDuplicateExport(zone);
+    if (entry != nullptr) {
+      error_handler->ReportMessageAt(
+          entry->location.beg_pos, entry->location.end_pos,
+          MessageTemplate::kDuplicateExport, entry->export_name);
+      return false;
     }
   }
 
   // Report error iff there are exports of non-existent local names.
-  for (auto entry : exports_) {
-    if (entry->local_name == nullptr) continue;
+  for (const auto& it : regular_exports_) {
+    const ModuleEntry* entry = it.second;
+    DCHECK_NOT_NULL(entry->local_name);
     if (module_scope->LookupLocal(entry->local_name) == nullptr) {
       error_handler->ReportMessageAt(
           entry->location.beg_pos, entry->location.end_pos,
@@ -136,7 +147,7 @@ bool ModuleDescriptor::Validate(ModuleScope* module_scope,
     }
   }
 
-  MakeIndirectExportsExplicit();
+  MakeIndirectExportsExplicit(zone);
   return true;
 }
 

src/ast/modules.h

@@ -19,7 +19,10 @@ class AstRawString;
 class ModuleDescriptor : public ZoneObject {
  public:
   explicit ModuleDescriptor(Zone* zone)
-      : exports_(1, zone), special_imports_(1, zone), regular_imports_(zone) {}
+      : special_exports_(1, zone),
+        special_imports_(1, zone),
+        regular_exports_(zone),
+        regular_imports_(zone) {}
 
   // import x from "foo.js";
   // import {x} from "foo.js";
@@ -82,8 +85,6 @@ class ModuleDescriptor : public ZoneObject {
           module_request(nullptr) {}
   };
 
-  const ZoneList<ModuleEntry*>& exports() const { return exports_; }
-
   // Empty imports and namespace imports.
   const ZoneList<const ModuleEntry*>& special_imports() const {
     return special_imports_;
@@ -95,11 +96,28 @@ class ModuleDescriptor : public ZoneObject {
     return regular_imports_;
   }
 
+  // Star exports and explicitly indirect exports.
+  const ZoneList<const ModuleEntry*>& special_exports() const {
+    return special_exports_;
+  }
+
+  // All the remaining exports, indexed by local name.
+  const ZoneMultimap<const AstRawString*, ModuleEntry*>& regular_exports()
+      const {
+    return regular_exports_;
+  }
+
  private:
-  ZoneList<ModuleEntry*> exports_;
+  // TODO(neis): Use STL datastructure instead of ZoneList?
+  ZoneList<const ModuleEntry*> special_exports_;
   ZoneList<const ModuleEntry*> special_imports_;
+  ZoneMultimap<const AstRawString*, ModuleEntry*> regular_exports_;
   ZoneMap<const AstRawString*, const ModuleEntry*> regular_imports_;
 
+  // If there are multiple export entries with the same export name, return one
+  // of them.  Otherwise return nullptr.
+  const ModuleEntry* FindDuplicateExport(Zone* zone) const;
+
   // Find any implicitly indirect exports and make them explicit.
   //
   // An explicitly indirect export is an export entry arising from an export
@@ -116,7 +134,7 @@ class ModuleDescriptor : public ZoneObject {
   // into:
   //   import {a as b} from "X"; export {a as c} from "X";
   // (The import entry is never deleted.)
-  void MakeIndirectExportsExplicit();
+  void MakeIndirectExportsExplicit(Zone* zone);
 };
 
 }  // namespace internal

src/ast/scopes.cc

@@ -1594,17 +1594,15 @@ void DeclarationScope::AllocateLocals() {
 }
 
 void ModuleScope::AllocateModuleVariables() {
-  for (auto it = module()->regular_imports().begin();
-       it != module()->regular_imports().end(); ++it) {
-    Variable* var = LookupLocal(it->second->local_name);
+  for (const auto& it : module()->regular_imports()) {
+    Variable* var = LookupLocal(it.first);
     // TODO(neis): Use a meaningful index.
     var->AllocateTo(VariableLocation::MODULE, 42);
   }
 
-  for (auto entry : module()->exports()) {
-    if (entry->local_name == nullptr) continue;
-    Variable* var = LookupLocal(entry->local_name);
-    var->AllocateTo(VariableLocation::MODULE, 42);
+  for (const auto& it : module()->regular_exports()) {
+    Variable* var = LookupLocal(it.first);
+    var->AllocateTo(VariableLocation::MODULE, 0);
   }
 }
 

src/zone-containers.h

@@ -122,6 +122,18 @@ class ZoneMap
             Compare(), zone_allocator<std::pair<const K, V>>(zone)) {}
 };
 
+// A wrapper subclass for std::multimap to make it easy to construct one that
+// uses a zone allocator.
+template <typename K, typename V, typename Compare = std::less<K>>
+class ZoneMultimap
+    : public std::multimap<K, V, Compare,
+                           zone_allocator<std::pair<const K, V>>> {
+ public:
+  // Constructs an empty multimap.
+  explicit ZoneMultimap(Zone* zone)
+      : std::multimap<K, V, Compare, zone_allocator<std::pair<const K, V>>>(
+            Compare(), zone_allocator<std::pair<const K, V>>(zone)) {}
+};
 
 // Typedefs to shorten commonly used vectors.
 typedef ZoneVector<bool> BoolVector;

test/cctest/test-parsing.cc

@@ -5852,22 +5852,22 @@ static void CheckModuleEntry(const i::ModuleDescriptor::ModuleEntry* entry,
   if (export_name == nullptr) {
     CHECK_NULL(entry->export_name);
   } else {
-    entry->export_name->IsOneByteEqualTo(export_name);
+    CHECK(entry->export_name->IsOneByteEqualTo(export_name));
   }
   if (local_name == nullptr) {
     CHECK_NULL(entry->local_name);
   } else {
-    entry->local_name->IsOneByteEqualTo(local_name);
+    CHECK(entry->local_name->IsOneByteEqualTo(local_name));
   }
   if (import_name == nullptr) {
     CHECK_NULL(entry->import_name);
   } else {
-    entry->import_name->IsOneByteEqualTo(import_name);
+    CHECK(entry->import_name->IsOneByteEqualTo(import_name));
   }
   if (module_request == nullptr) {
     CHECK_NULL(entry->module_request);
   } else {
-    entry->module_request->IsOneByteEqualTo(module_request);
+    CHECK(entry->module_request->IsOneByteEqualTo(module_request));
   }
 }
 
@@ -5914,6 +5914,7 @@ TEST(ModuleParsingInternals) {
   CHECK(outer_scope->is_script_scope());
   CHECK_NULL(outer_scope->outer_scope());
   CHECK(module_scope->is_module_scope());
+  const i::ModuleDescriptor::ModuleEntry* entry;
   i::ZoneList<i::Declaration*>* declarations = module_scope->declarations();
   CHECK_EQ(13, declarations->length());
 
@@ -5999,29 +6000,53 @@ TEST(ModuleParsingInternals) {
   i::ModuleDescriptor* descriptor = module_scope->module();
   CHECK_NOT_NULL(descriptor);
 
-  CHECK_EQ(11, descriptor->exports().length());
-  CheckModuleEntry(
-      descriptor->exports().at(0), "y", "x", nullptr, nullptr);
-  CheckModuleEntry(
-      descriptor->exports().at(1), "b", nullptr, "a", "m.js");
-  CheckModuleEntry(
-      descriptor->exports().at(2), nullptr, nullptr, nullptr, "p.js");
-  CheckModuleEntry(
-      descriptor->exports().at(3), "foo", "foo", nullptr, nullptr);
-  CheckModuleEntry(
-      descriptor->exports().at(4), "goo", "goo", nullptr, nullptr);
-  CheckModuleEntry(
-      descriptor->exports().at(5), "hoo", "hoo", nullptr, nullptr);
-  CheckModuleEntry(
-      descriptor->exports().at(6), "joo", "joo", nullptr, nullptr);
-  CheckModuleEntry(
-      descriptor->exports().at(7), "default", "*default*", nullptr, nullptr);
-  CheckModuleEntry(
-      descriptor->exports().at(8), "bb", nullptr, "aa", "m.js");  // !!!
-  CheckModuleEntry(
-      descriptor->exports().at(9), "x", "x", nullptr, nullptr);
-  CheckModuleEntry(
-      descriptor->exports().at(10), "foob", "foob", nullptr, nullptr);
+  CHECK_EQ(3, descriptor->special_exports().length());
+  CheckModuleEntry(descriptor->special_exports().at(0), "b", nullptr, "a",
+                   "m.js");
+  CheckModuleEntry(descriptor->special_exports().at(1), nullptr, nullptr,
+                   nullptr, "p.js");
+  CheckModuleEntry(descriptor->special_exports().at(2), "bb", nullptr, "aa",
+                   "m.js");  // !!!
+
+  CHECK_EQ(8, descriptor->regular_exports().size());
+  entry = descriptor->regular_exports()
+              .find(declarations->at(3)->proxy()->raw_name())
+              ->second;
+  CheckModuleEntry(entry, "foo", "foo", nullptr, nullptr);
+  entry = descriptor->regular_exports()
+              .find(declarations->at(4)->proxy()->raw_name())
+              ->second;
+  CheckModuleEntry(entry, "goo", "goo", nullptr, nullptr);
+  entry = descriptor->regular_exports()
+              .find(declarations->at(5)->proxy()->raw_name())
+              ->second;
+  CheckModuleEntry(entry, "hoo", "hoo", nullptr, nullptr);
+  entry = descriptor->regular_exports()
+              .find(declarations->at(6)->proxy()->raw_name())
+              ->second;
+  CheckModuleEntry(entry, "joo", "joo", nullptr, nullptr);
+  entry = descriptor->regular_exports()
+              .find(declarations->at(7)->proxy()->raw_name())
+              ->second;
+  CheckModuleEntry(entry, "default", "*default*", nullptr, nullptr);
+  entry = descriptor->regular_exports()
+              .find(declarations->at(12)->proxy()->raw_name())
+              ->second;
+  CheckModuleEntry(entry, "foob", "foob", nullptr, nullptr);
+  // TODO(neis): The next lines are terrible. Find a better way.
+  auto name_x = declarations->at(0)->proxy()->raw_name();
+  CHECK_EQ(2, descriptor->regular_exports().count(name_x));
+  auto it = descriptor->regular_exports().equal_range(name_x).first;
+  entry = it->second;
+  if (entry->export_name->IsOneByteEqualTo("y")) {
+    CheckModuleEntry(entry, "y", "x", nullptr, nullptr);
+    entry = (++it)->second;
+    CheckModuleEntry(entry, "x", "x", nullptr, nullptr);
+  } else {
+    CheckModuleEntry(entry, "x", "x", nullptr, nullptr);
+    entry = (++it)->second;
+    CheckModuleEntry(entry, "y", "x", nullptr, nullptr);
+  }
 
   CHECK_EQ(3, descriptor->special_imports().length());
   CheckModuleEntry(
@@ -6032,7 +6057,6 @@ TEST(ModuleParsingInternals) {
       descriptor->special_imports().at(2), nullptr, "foob", nullptr, "bar.js");
 
   CHECK_EQ(4, descriptor->regular_imports().size());
-  const i::ModuleDescriptor::ModuleEntry* entry;
   entry = descriptor->regular_imports().find(
       declarations->at(1)->proxy()->raw_name())->second;
   CheckModuleEntry(entry, nullptr, "z", "q", "m.js");