[module] Track script "module code" status

src/objects.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/objects.h"
 
 #include <cmath>
 #include <iomanip>
 #include <sstream>
 
@@ skipping 15724 matching lines @@
     os << " (" << PrivateSymbolToName() << ")";
   }
   os << ">";
 }
 
 
 // StringSharedKeys are used as keys in the eval cache.
 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;
     if (!other->IsFixedArray()) {
       if (!other->IsNumber()) return false;
       uint32_t other_hash = static_cast<uint32_t>(other->Number());
       return Hash() == other_hash;
     }
     FixedArray* other_array = FixedArray::cast(other);
     SharedFunctionInfo* shared = SharedFunctionInfo::cast(other_array->get(0));
     if (shared != *shared_) return false;
     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);
     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_);
   }
 
   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
       // code computation, we use a combination of the hash of the
       // script source code and the start position of the calling scope.
       // We do this to ensure that the cache entries can survive garbage
       // collection.
       Script* script(Script::cast(shared->script()));
       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;
   }
 
   uint32_t Hash() override {
     return StringSharedHashHelper(*source_, *shared_, language_mode_,
-                                  scope_position_);
+                                  is_module_, scope_position_);
   }
 
   uint32_t HashForObject(Object* obj) override {
     DisallowHeapAllocation no_allocation;
     if (obj->IsNumber()) {
       return static_cast<uint32_t>(obj->Number());
     }
     FixedArray* other_array = FixedArray::cast(obj);
     SharedFunctionInfo* shared = SharedFunctionInfo::cast(other_array->get(0));
     String* source = String::cast(other_array->get(1));
     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;
   }
 
  private:
   Handle<String> source_;
   Handle<SharedFunctionInfo> shared_;
   LanguageMode language_mode_;
+  bool is_module_;
   int scope_position_;
 };
 
 
 namespace {
 
 JSRegExp::Flags RegExpFlagsFromString(Handle<String> flags, bool* success) {
   JSRegExp::Flags value = JSRegExp::kNone;
   int length = flags->length();
   // A longer flags string cannot be valid.
@@ skipping 1202 matching lines @@
   }
   return stringset;
 }
 
 bool StringSet::Has(Handle<String> name) {
   return FindEntry(*name) != kNotFound;
 }
 
 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);
   if (!get(index)->IsFixedArray()) return isolate->factory()->undefined_value();
   return Handle<Object>(get(index + 1), isolate);
 }
 
 
 Handle<Object> CompilationCacheTable::LookupEval(
     Handle<String> src, Handle<SharedFunctionInfo> outer_info,
@@ skipping 13 matching lines @@
 Handle<Object> CompilationCacheTable::LookupRegExp(Handle<String> src,
                                                    JSRegExp::Flags flags) {
   Isolate* isolate = GetIsolate();
   DisallowHeapAllocation no_allocation;
   RegExpKey key(src, flags);
   int entry = FindEntry(&key);
   if (entry == kNotFound) return isolate->factory()->undefined_value();
   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.

   Handle<Object> k = key.AsHandle(isolate);
   cache = EnsureCapacity(cache, 1, &key);
   int entry = cache->FindInsertionEntry(key.Hash());
   cache->set(EntryToIndex(entry), *k);
   cache->set(EntryToIndex(entry) + 1, *value);
   cache->ElementAdded();
   return cache;
 }
 
 
@@ skipping 1756 matching lines @@
   if (cell->value() != *new_value) {
     cell->set_value(*new_value);
     Isolate* isolate = cell->GetIsolate();
     cell->dependent_code()->DeoptimizeDependentCodeGroup(
         isolate, DependentCode::kPropertyCellChangedGroup);
   }
 }
 
 }  // namespace internal
 }  // namespace v8