[modules] AST and parser rework.

src/ast/modules.h

Index: src/ast/modules.h
diff --git a/src/ast/modules.h b/src/ast/modules.h
index 1fdf526cd136145bc38167fb72458adff7b40de8..d17ada2c1cdb88056875cedea181c3e795f7df7a 100644
--- a/src/ast/modules.h
+++ b/src/ast/modules.h
@@ -5,6 +5,8 @@
 #ifndef V8_AST_MODULES_H_
 #define V8_AST_MODULES_H_
 
+#include "src/parsing/scanner.h"

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

This is just for Scanner::Location, right? Can you add a comment to that effect?
I suspect we might want to move that somewhere else, having an AST -> Parser
dependency seems kinda backwards.

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

Yes. Done.

+#include "src/pending-compilation-error-handler.h"
 #include "src/zone.h"
 
 namespace v8 {
@@ -23,83 +25,84 @@ class ModuleDescriptor : public ZoneObject {
     return new (zone) ModuleDescriptor(zone);
   }
 
+
   // ---------------------------------------------------------------------------
   // Mutators.
 
-  // Add a name to the list of exports. If it already exists, that's an error.
-  void AddLocalExport(const AstRawString* export_name,
-                      const AstRawString* local_name, Zone* zone, bool* ok);
-
-  // Add module_specifier to the list of requested modules,
-  // if not already present.
-  void AddModuleRequest(const AstRawString* module_specifier, Zone* zone);
+  // import "foo.js";
+  // import {} from "foo.js";
+  // export {} from "foo.js";
+  void AddEmptyImport(
+      const AstRawString* module_request, const Scanner::Location loc,
+      Zone* zone);
+
+  // import x from "foo.js";
+  // import {x} from "foo.js";
+  // import {x as y} from "foo.js";
+  void AddNonStarImport(

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

This seems like the "common" case, how about "AddImport" for the name?

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

Done.

+    const AstRawString* import_name, const AstRawString* local_name,
+    const AstRawString* module_request, const Scanner::Location loc,
+    Zone* zone);
+
+  // import * as x from "foo.js";
+  void AddStarImport(
+    const AstRawString* local_name, const AstRawString* module_request,
+    const Scanner::Location loc, Zone* zone);
+
+  // export {x};
+  // export {x as y};
+  // export VariableStatement
+  // export Declaration
+  // export default ...
+  void AddNonStarExport(

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

Same here and below, just "AddExport" and let "AddStarExport" speak for itself?

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

Done.

+    const AstRawString* local_name, const AstRawString* export_name,
+    const Scanner::Location loc, Zone* zone);
+
+  // export {x} from "foo.js";
+  // export {x as y} from "foo.js";
+  void AddNonStarExport(
+    const AstRawString* export_name, const AstRawString* import_name,
+    const AstRawString* module_request, const Scanner::Location loc,
+    Zone* zone);
+
+  // export * from "foo.js";
+  void AddStarExport(
+    const AstRawString* module_request, const Scanner::Location loc,
+    Zone* zone);
 
-  // Assign an index.
-  void Allocate(int index) {
-    DCHECK_EQ(-1, index_);
-    index_ = index;
-  }
 
   // ---------------------------------------------------------------------------
   // Accessors.

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

Don't think this comment is terribly useful or accurate anymore.

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

Removed this and the others.

 
-  int Length() {
-    ZoneHashMap* exports = exports_;
-    return exports ? exports->occupancy() : 0;
-  }
-
-  // The context slot in the hosting script context pointing to this module.
-  int Index() {
-    return index_;
-  }
-
-  const AstRawString* LookupLocalExport(const AstRawString* export_name,
-                                        Zone* zone);
+  bool Validate(

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

Add a comment here too?

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

Done.

+      Scope* module_scope, PendingCompilationErrorHandler* error_handler,
+      Zone* zone) const;
 
-  const ZoneList<const AstRawString*>& requested_modules() const {
-    return requested_modules_;
-  }
-
-  // ---------------------------------------------------------------------------
-  // Iterators.
-
-  // Use like:
-  //   for (auto it = descriptor->iterator(); !it.done(); it.Advance()) {
-  //     ... it.name() ...
-  //   }
-  class Iterator {
-   public:
-    bool done() const { return entry_ == NULL; }
-    const AstRawString* export_name() const {
-      DCHECK(!done());
-      return static_cast<const AstRawString*>(entry_->key);
-    }
-    const AstRawString* local_name() const {
-      DCHECK(!done());
-      return static_cast<const AstRawString*>(entry_->value);
-    }
-    void Advance() { entry_ = exports_->Next(entry_); }
-
-   private:
-    friend class ModuleDescriptor;
-    explicit Iterator(const ZoneHashMap* exports)
-        : exports_(exports), entry_(exports ? exports->Start() : NULL) {}
-
-    const ZoneHashMap* exports_;
-    ZoneHashMap::Entry* entry_;
-  };
-
-  Iterator iterator() const { return Iterator(this->exports_); }
 
   // ---------------------------------------------------------------------------
   // Implementation.
+
  private:
   explicit ModuleDescriptor(Zone* zone)
-      : exports_(NULL), requested_modules_(1, zone), index_(-1) {}
+      : exports_(1, zone), imports_(1, zone) {}
+
+  struct ModuleEntry {

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

Any reason not to have this extend ZoneObject?

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

No, thans for the suggestion. Done.

+    const Scanner::Location location;
+    const AstRawString* export_name;
+    const AstRawString* local_name;
+    const AstRawString* import_name;
+    const AstRawString* module_request;
+
+    explicit ModuleEntry(const Scanner::Location loc)

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

No need for "const" here.

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

Done.

+        : location(loc),
+          export_name(nullptr),
+          local_name(nullptr),
+          import_name(nullptr),
+          module_request(nullptr) {}
+  };
 
-  ZoneHashMap* exports_;   // Module exports and their types (allocated lazily)
-  ZoneList<const AstRawString*> requested_modules_;
-  int index_;
+  ZoneList<const ModuleEntry*> exports_;
+  ZoneList<const ModuleEntry*> imports_;
 };
 
 }  // namespace internal