[module] Track script "module code" status

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index a5da88a0b3b2e0b5e44b17e0918d17655ea38e09..e1f076601348dc5550c459f2a89c9416ae5de12d 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -15742,11 +15742,16 @@ void Symbol::SymbolShortPrint(std::ostream& os) {
 class StringSharedKey : public HashTableKey {
  public:
   StringSharedKey(Handle<String> source, Handle<SharedFunctionInfo> shared,
-                  LanguageMode language_mode, int scope_position)
+                  LanguageMode language_mode, bool is_module,
+                  int scope_position)
       : source_(source),
         shared_(shared),
         language_mode_(language_mode),
+        is_module_(is_module),
         scope_position_(scope_position) {}
+  StringSharedKey(Handle<String> source, Handle<SharedFunctionInfo> shared,
+                  LanguageMode language_mode, int scope_position)
+      : StringSharedKey(source, shared, language_mode, false, scope_position) {}

[vogelheim, 2016/06/23 14:53:19]

I'd prefer if this constructor was removed... It makes it fairly easy to forget
about the flag and hence introduce bugs. I suspect line 17106 in this file is
one example...

[mike3, 2016/06/25 19:31:45]

Acknowledged.

 
   bool IsMatch(Object* other) override {
     DisallowHeapAllocation no_allocation;
@@ -15762,7 +15767,10 @@ class StringSharedKey : public HashTableKey {
     DCHECK(is_valid_language_mode(language_unchecked));
     LanguageMode language_mode = static_cast<LanguageMode>(language_unchecked);
     if (language_mode != language_mode_) return false;
-    int scope_position = Smi::cast(other_array->get(3))->value();
+    bool is_module =
+        Smi::cast(other_array->get(3))->value() == 1 ? true : false;

[vogelheim, 2016/06/23 14:53:19]

nit: drop "? true : false". The result of == is already a bool.

[mike3, 2016/06/25 19:31:45]

Acknowledged.

+    if (is_module != is_module_) return false;
+    int scope_position = Smi::cast(other_array->get(4))->value();
     if (scope_position != scope_position_) return false;
     String* source = String::cast(other_array->get(1));
     return source->Equals(*source_);
@@ -15771,7 +15779,7 @@ class StringSharedKey : public HashTableKey {
   static uint32_t StringSharedHashHelper(String* source,
                                          SharedFunctionInfo* shared,
                                          LanguageMode language_mode,
-                                         int scope_position) {
+                                         bool is_module, int scope_position) {
     uint32_t hash = source->Hash();
     if (shared->HasSourceCode()) {
       // Instead of using the SharedFunctionInfo pointer in the hash
@@ -15783,6 +15791,7 @@ class StringSharedKey : public HashTableKey {
       hash ^= String::cast(script->source())->Hash();
       STATIC_ASSERT(LANGUAGE_END == 3);
       if (is_strict(language_mode)) hash ^= 0x8000;
+      if (is_module) hash ^= 0x4000;

[mike3, 2016/06/19 17:11:41]

Would it be better to preserve the current bits for the hash and instead extend
the size of the hash value to store this bit?

[vogelheim, 2016/06/23 14:53:20]

No, I don't think so. The whole point of a hash is be a small(er) representation
of the inputs, so I don't think extending it to accommodate more inputs would
not make sense.

[mike3, 2016/06/25 19:31:45]

Acknowledged.

       hash += scope_position;
     }
     return hash;
@@ -15790,7 +15799,7 @@ class StringSharedKey : public HashTableKey {
 
   uint32_t Hash() override {
     return StringSharedHashHelper(*source_, *shared_, language_mode_,
-                                  scope_position_);
+                                  is_module_, scope_position_);
   }
 
   uint32_t HashForObject(Object* obj) override {
@@ -15804,18 +15813,21 @@ class StringSharedKey : public HashTableKey {
     int language_unchecked = Smi::cast(other_array->get(2))->value();
     DCHECK(is_valid_language_mode(language_unchecked));
     LanguageMode language_mode = static_cast<LanguageMode>(language_unchecked);
-    int scope_position = Smi::cast(other_array->get(3))->value();
-    return StringSharedHashHelper(source, shared, language_mode,
+    bool is_module =
+        Smi::cast(other_array->get(3))->value() == 1 ? true : false;
+    int scope_position = Smi::cast(other_array->get(4))->value();
+    return StringSharedHashHelper(source, shared, language_mode, is_module,
                                   scope_position);
   }
 
 
   Handle<Object> AsHandle(Isolate* isolate) override {
-    Handle<FixedArray> array = isolate->factory()->NewFixedArray(4);
+    Handle<FixedArray> array = isolate->factory()->NewFixedArray(5);
     array->set(0, *shared_);
     array->set(1, *source_);
     array->set(2, Smi::FromInt(language_mode_));
-    array->set(3, Smi::FromInt(scope_position_));
+    array->set(3, Smi::FromInt(is_module_ ? 1 : 0));

[mike3, 2016/06/19 17:11:41]

I looked for Smi::FromBool, but no dice. This is my best guess for encoding a
boolean value as an SMI--is there some other preferred way to do that?

+    array->set(4, Smi::FromInt(scope_position_));
     return array;
   }
 
@@ -15823,6 +15835,7 @@ class StringSharedKey : public HashTableKey {
   Handle<String> source_;
   Handle<SharedFunctionInfo> shared_;
   LanguageMode language_mode_;
+  bool is_module_;
   int scope_position_;
 };
 
@@ -17045,10 +17058,12 @@ bool StringSet::Has(Handle<String> name) {
 
 Handle<Object> CompilationCacheTable::Lookup(Handle<String> src,
                                              Handle<Context> context,
-                                             LanguageMode language_mode) {
+                                             LanguageMode language_mode,
+                                             bool is_module) {
   Isolate* isolate = GetIsolate();
   Handle<SharedFunctionInfo> shared(context->closure()->shared());
-  StringSharedKey key(src, shared, language_mode, RelocInfo::kNoPosition);
+  StringSharedKey key(src, shared, language_mode, is_module,
+                      RelocInfo::kNoPosition);
   int entry = FindEntry(&key);
   if (entry == kNotFound) return isolate->factory()->undefined_value();
   int index = EntryToIndex(entry);
@@ -17082,10 +17097,10 @@ Handle<Object> CompilationCacheTable::LookupRegExp(Handle<String> src,
   return Handle<Object>(get(EntryToIndex(entry) + 1), isolate);
 }
 
-
 Handle<CompilationCacheTable> CompilationCacheTable::Put(
     Handle<CompilationCacheTable> cache, Handle<String> src,
-    Handle<Context> context, LanguageMode language_mode, Handle<Object> value) {
+    Handle<Context> context, LanguageMode language_mode, bool is_module,
+    Handle<Object> value) {
   Isolate* isolate = cache->GetIsolate();
   Handle<SharedFunctionInfo> shared(context->closure()->shared());
   StringSharedKey key(src, shared, language_mode, RelocInfo::kNoPosition);

[vogelheim, 2016/06/23 14:53:19]

Shouldn't key depend on is_module?

[mike3, 2016/06/25 19:31:45]

Acknowledged.