[modules] AST and parser rework.

src/ast/modules.cc

Index: src/ast/modules.cc
diff --git a/src/ast/modules.cc b/src/ast/modules.cc
index f895756e4abc3339f97c95237761ce69b62bc1ca..7181b97f76ff6a7b5bbbb89e054d7ad8f884e478 100644
--- a/src/ast/modules.cc
+++ b/src/ast/modules.cc
@@ -3,56 +3,124 @@
 // found in the LICENSE file.
 
 #include "src/ast/modules.h"
-
 #include "src/ast/ast-value-factory.h"
+#include "src/ast/scopes.h"
 
 namespace v8 {
 namespace internal {
 
 
-void ModuleDescriptor::AddLocalExport(const AstRawString* export_name,
-                                      const AstRawString* local_name,
-                                      Zone* zone, bool* ok) {
-  void* key = const_cast<AstRawString*>(export_name);
+void ModuleDescriptor::AddEmptyImport(
+    const AstRawString* module_request, Scanner::Location loc, Zone* zone) {
+  DCHECK_NOT_NULL(module_request);
+  ModuleEntry* entry = new (zone->New(sizeof(ModuleEntry))) ModuleEntry(loc);

[adamk, 2016/07/13 18:38:21]

If you make ModuleEntry a ZoneObject this gets cleaner.

[neis, 2016/07/14 10:28:23]

Done.

+  entry->module_request = module_request;
+  imports_.Add(entry, zone);
+}
 
-  ZoneAllocationPolicy allocator(zone);
 
-  if (exports_ == nullptr) {
-    exports_ = new (zone->New(sizeof(ZoneHashMap)))
-        ZoneHashMap(ZoneHashMap::PointersMatch,
-                    ZoneHashMap::kDefaultHashMapCapacity, allocator);
-  }
+void ModuleDescriptor::AddNonStarImport(
+    const AstRawString* import_name, const AstRawString* local_name,
+    const AstRawString* module_request, Scanner::Location loc, Zone* zone) {
+  DCHECK_NOT_NULL(import_name);
+  DCHECK_NOT_NULL(local_name);
+  DCHECK_NOT_NULL(module_request);
+  ModuleEntry* entry = new (zone->New(sizeof(ModuleEntry))) ModuleEntry(loc);
+  entry->local_name = local_name;
+  entry->import_name = import_name;
+  entry->module_request = module_request;
+  imports_.Add(entry, zone);
+}
+
+
+void ModuleDescriptor::AddStarImport(
+    const AstRawString* local_name, const AstRawString* module_request,
+    Scanner::Location loc, Zone* zone) {
+  DCHECK_NOT_NULL(local_name);
+  DCHECK_NOT_NULL(module_request);
+  ModuleEntry* entry = new (zone->New(sizeof(ModuleEntry))) ModuleEntry(loc);
+  entry->local_name = local_name;
+  entry->module_request = module_request;
+  imports_.Add(entry, zone);
+}
 
-  ZoneHashMap::Entry* p =
-      exports_->LookupOrInsert(key, export_name->hash(), allocator);
-  DCHECK_NOT_NULL(p);
-  if (p->value != nullptr) {
-    // Duplicate export.
-    *ok = false;
-    return;
-  }
 
-  p->value = const_cast<AstRawString*>(local_name);
+void ModuleDescriptor::AddNonStarExport(
+    const AstRawString* local_name, const AstRawString* export_name,
+    Scanner::Location loc, Zone* zone) {
+  DCHECK_NOT_NULL(local_name);
+  DCHECK_NOT_NULL(export_name);
+  ModuleEntry* entry = new (zone->New(sizeof(ModuleEntry))) ModuleEntry(loc);
+  entry->export_name = export_name;
+  entry->local_name = local_name;
+  exports_.Add(entry, zone);
 }
 
 
-void ModuleDescriptor::AddModuleRequest(const AstRawString* module_specifier,
-                                        Zone* zone) {
-  // TODO(adamk): Avoid this O(N) operation on each insert by storing
-  // a HashMap, or by de-duping after parsing.
-  if (requested_modules_.Contains(module_specifier)) return;
-  requested_modules_.Add(module_specifier, zone);
+void ModuleDescriptor::AddNonStarExport(
+    const AstRawString* import_name, const AstRawString* export_name,
+    const AstRawString* module_request, Scanner::Location loc, Zone* zone) {
+  DCHECK_NOT_NULL(import_name);
+  DCHECK_NOT_NULL(export_name);
+  DCHECK_NOT_NULL(module_request);
+  ModuleEntry* entry = new (zone->New(sizeof(ModuleEntry))) ModuleEntry(loc);
+  entry->export_name = export_name;
+  entry->import_name = import_name;
+  entry->module_request = module_request;
+  exports_.Add(entry, zone);
 }
 
 
-const AstRawString* ModuleDescriptor::LookupLocalExport(
-    const AstRawString* export_name, Zone* zone) {
-  if (exports_ == nullptr) return nullptr;
-  ZoneHashMap::Entry* entry = exports_->Lookup(
-      const_cast<AstRawString*>(export_name), export_name->hash());
-  if (entry == nullptr) return nullptr;
-  DCHECK_NOT_NULL(entry->value);
-  return static_cast<const AstRawString*>(entry->value);
+void ModuleDescriptor::AddStarExport(
+    const AstRawString* module_request, Scanner::Location loc, Zone* zone) {
+  DCHECK_NOT_NULL(module_request);
+  ModuleEntry* entry = new (zone->New(sizeof(ModuleEntry))) ModuleEntry(loc);
+  entry->module_request = module_request;
+  exports_.Add(entry, zone);
+}
+
+
+bool ModuleDescriptor::Validate(
+    Scope* module_scope, PendingCompilationErrorHandler* error_handler,
+    Zone* zone) const {
+  DCHECK(module_scope->is_module_scope());
+  DCHECK_EQ(this, module_scope->module());
+  DCHECK_NOT_NULL(error_handler);
+
+  // 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& it : exports_) {

[adamk, 2016/07/13 18:38:21]

"it" seems an odd name for this, as it's not an iterator..."entry"?

Also, the non-const ref looks funny. I guess the type is actually const
ModuleEntry*, so that's fine?

[neis, 2016/07/14 10:28:23]

Thanks, I forgot to update this when the data structure changed. Done.

+      if (it->export_name == nullptr) continue;
+      AstRawString* key = const_cast<AstRawString*>(it->export_name);
+      ZoneHashMap::Entry* p =
+          export_names->LookupOrInsert(key, key->hash(), allocator);
+      DCHECK_NOT_NULL(p);
+      if (p->value != nullptr) {
+        error_handler->ReportMessageAt(
+            it->location.beg_pos, it->location.end_pos,
+            MessageTemplate::kDuplicateExport, it->export_name);
+        return false;
+      }
+      p->value = key;  // Anything but nullptr.
+    }
+  }
+
+  // Report error iff there are exports of non-existent local names.
+  for (auto& it : exports_) {
+    if (it->local_name == nullptr) continue;
+    if (module_scope->LookupLocal(it->local_name) == nullptr) {
+      error_handler->ReportMessageAt(
+          it->location.beg_pos, it->location.end_pos,
+          MessageTemplate::kModuleExportUndefined, it->local_name);
+      return false;
+    }
+  }
+
+  return true;
 }
 }  // namespace internal
 }  // namespace v8