[modules] Make exported variable indices stable

A module's exported variables are assigned a cell index based on their
order in the exported variable map. This map is keyed by the variable
name, an AstRawString*.

Unfortunately, these string pointers are not guaranteed to increase
monotonically as variables are encountered, which means that this map
isn't stable across parses. In particular, it can cause failures for
setVariableValue if the parser is unlucky.

This patch adds a custom comparator to these AstRawString* keyed maps,
which is stable across parses.

Change-Id: Ie6e88fc2d252d873de661d7fc5278feba3955727
Reviewed-on: https://chromium-review.googlesource.com/1131503Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54379}

src/ast/modules.cc

@@ -12,6 +12,21 @@
 namespace v8 {
 namespace internal {
 
+bool ModuleDescriptor::AstRawStringComparer::operator()(
+    const AstRawString* lhs, const AstRawString* rhs) const {
+  // Fast path for equal pointers: a pointer is not strictly less than itself.
+  if (lhs == rhs) return false;
+
+  // Order by contents (ordering by hash is unstable across runs).
+  if (lhs->is_one_byte() != rhs->is_one_byte()) {
+    return lhs->is_one_byte();
+  }
+  if (lhs->byte_length() != rhs->byte_length()) {
+    return lhs->byte_length() < rhs->byte_length();
+  }
+  return memcmp(lhs->raw_data(), rhs->raw_data(), lhs->byte_length()) < 0;
+}
+
 void ModuleDescriptor::AddImport(const AstRawString* import_name,
                                  const AstRawString* local_name,
                                  const AstRawString* module_request,

src/ast/modules.h

@@ -124,10 +124,21 @@ class ModuleDescriptor : public ZoneObject {
     ModuleRequest(int index, int position) : index(index), position(position) {}
   };
 
+  // Custom content-based comparer for the below maps, to keep them stable
+  // across parses.
+  struct AstRawStringComparer {
+    bool operator()(const AstRawString* lhs, const AstRawString* rhs) const;
+  };
+
+  typedef ZoneMap<const AstRawString*, ModuleRequest, AstRawStringComparer>
+      ModuleRequestMap;
+  typedef ZoneMultimap<const AstRawString*, Entry*, AstRawStringComparer>
+      RegularExportMap;
+  typedef ZoneMap<const AstRawString*, Entry*, AstRawStringComparer>
+      RegularImportMap;
+
   // Module requests.
-  const ZoneMap<const AstRawString*, ModuleRequest>& module_requests() const {
-    return module_requests_;
-  }
+  const ModuleRequestMap& module_requests() const { return module_requests_; }
 
   // Namespace imports.
   const ZoneVector<const Entry*>& namespace_imports() const {
@@ -135,9 +146,7 @@ class ModuleDescriptor : public ZoneObject {
   }
 
   // All the remaining imports, indexed by local name.
-  const ZoneMap<const AstRawString*, Entry*>& regular_imports() const {
-    return regular_imports_;
-  }
+  const RegularImportMap& regular_imports() const { return regular_imports_; }
 
   // Star exports and explicitly indirect exports.
   const ZoneVector<const Entry*>& special_exports() const {
@@ -146,9 +155,7 @@ class ModuleDescriptor : public ZoneObject {
 
   // All the remaining exports, indexed by local name.
   // After canonicalization (see Validate), these are exactly the local exports.
-  const ZoneMultimap<const AstRawString*, Entry*>& regular_exports() const {
-    return regular_exports_;
-  }
+  const RegularExportMap& regular_exports() const { return regular_exports_; }
 
   void AddRegularExport(Entry* entry) {
     DCHECK_NOT_NULL(entry->export_name);
@@ -188,11 +195,11 @@ class ModuleDescriptor : public ZoneObject {
                                  Handle<ModuleInfo> module_info);
 
  private:
-  ZoneMap<const AstRawString*, ModuleRequest> module_requests_;
+  ModuleRequestMap module_requests_;
   ZoneVector<const Entry*> special_exports_;
   ZoneVector<const Entry*> namespace_imports_;
-  ZoneMultimap<const AstRawString*, Entry*> regular_exports_;
-  ZoneMap<const AstRawString*, Entry*> regular_imports_;
+  RegularExportMap regular_exports_;
+  RegularImportMap regular_imports_;
 
   // If there are multiple export entries with the same export name, return the
   // last of them (in source order).  Otherwise return nullptr.