Get rid of static module allocation, do it in code.

src/runtime.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1340 matching lines @@
     HandleScope scope(isolate);
     Handle<String> name(String::cast(pairs->get(i)));
     Handle<Object> value(pairs->get(i + 1), isolate);
 
     // We have to declare a global const property. To capture we only
     // assign to it when evaluating the assignment for "const x =
     // <expr>" the initial value is the hole.
     bool is_var = value->IsUndefined();
     bool is_const = value->IsTheHole();
     bool is_function = value->IsSharedFunctionInfo();
-    bool is_module = value->IsJSModule();
-    ASSERT(is_var + is_const + is_function + is_module == 1);
+    ASSERT(is_var + is_const + is_function == 1);

[Sven Panne, 2012/11/20 14:39:13]

Cunning check for mutual exclusion... o_O Anyway, I don't think we need this
here, but I leave it up to you.

[rossberg, 2012/11/20 17:23:45]

You think so? There actually was a subtle bug with that while I was working on
some earlier CL, which is why I put it here. ;)

 
     if (is_var || is_const) {
       // Lookup the property in the global object, and don't set the
       // value of the variable if the property is already there.
       // Do the lookup locally only, see ES5 erratum.
       LookupResult lookup(isolate);
       if (FLAG_es52_globals) {
         Object* obj = *global;
         do {
           JSObject::cast(obj)->LocalLookup(*name, &lookup);
@@ skipping 23 matching lines @@
       value = function;
     }
 
     LookupResult lookup(isolate);
     global->LocalLookup(*name, &lookup);
 
     // Compute the property attributes. According to ECMA-262,
     // the property must be non-configurable except in eval.
     int attr = NONE;
     bool is_eval = DeclareGlobalsEvalFlag::decode(flags);
-    if (!is_eval || is_module) {
+    if (!is_eval) {
       attr |= DONT_DELETE;
     }
     bool is_native = DeclareGlobalsNativeFlag::decode(flags);
-    if (is_const || is_module || (is_native && is_function)) {
+    if (is_const || (is_native && is_function)) {
       attr |= READ_ONLY;
     }
 
     LanguageMode language_mode = DeclareGlobalsLanguageMode::decode(flags);
 
-    if (!lookup.IsFound() || is_function || is_module) {
+    if (!lookup.IsFound() || is_function) {
       // If the local property exists, check that we can reconfigure it
       // as required for function declarations.
       if (lookup.IsFound() && lookup.IsDontDelete()) {
         if (lookup.IsReadOnly() || lookup.IsDontEnum() ||
             lookup.IsPropertyCallbacks()) {
-          return ThrowRedeclarationError(
-              isolate, is_function ? "function" : "module", name);
+          return ThrowRedeclarationError(isolate, "function", name);
         }
         // If the existing property is not configurable, keep its attributes.
         attr = lookup.GetAttributes();
       }
       // Define or redefine own property.
       RETURN_IF_EMPTY_HANDLE(isolate,
           JSObject::SetLocalPropertyIgnoreAttributes(
               global, name, value, static_cast<PropertyAttributes>(attr)));
     } else {
       // Do a [[Put]] on the existing (own) property.
@@ skipping 7037 matching lines @@
 
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_IsJSModule) {
   ASSERT(args.length() == 1);
   Object* obj = args[0];
   return isolate->heap()->ToBoolean(obj->IsJSModule());
 }
 
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_PushModuleContext) {
-  NoHandleAllocation ha;
-  ASSERT(args.length() == 1);
-  CONVERT_ARG_HANDLE_CHECKED(JSModule, instance, 0);
+  ASSERT(args.length() == 2);
+  CONVERT_SMI_ARG_CHECKED(index, 0);
 
-  Context* context = Context::cast(instance->context());
+  if (!args[1]->IsScopeInfo()) {
+    // Module already initialized. Find hosting context and retrieve context.
+    Context* host = Context::cast(isolate->context());
+    while (host->IsModuleContext()) host = host->previous();
+    ASSERT(host->IsGlobalContext());
+    Context* context = Context::cast(host->get(index));
+    ASSERT(context->previous() == isolate->context());
+    isolate->set_context(context);
+    return context;
+  }
+
+  CONVERT_ARG_HANDLE_CHECKED(ScopeInfo, scope_info, 1);
+
+  // Allocate module context.
+  HandleScope scope(isolate);
+  Factory* factory = isolate->factory();
+  Handle<Context> context = factory->NewModuleContext(scope_info);
+  Handle<JSModule> module = factory->NewJSModule(context, scope_info);
+  context->set_module(*module);
   Context* previous = isolate->context();
-  ASSERT(context->IsModuleContext());
-  // Initialize the context links.
   context->set_previous(previous);
   context->set_closure(previous->closure());
   context->set_global_object(previous->global_object());
-  isolate->set_context(context);
+  isolate->set_context(*context);
 
-  return context;
+  // Find hosting scope and initialize internal variable holding module there.
+  while (previous->IsModuleContext()) previous = previous->previous();

[Michael Starzinger, 2012/11/20 12:05:05]

Use a Context::global_context() helper here.

[rossberg, 2012/11/20 17:23:45]

Done.

+  ASSERT(previous->IsGlobalContext());
+  previous->set(index, *context);
+
+  return *context;
 }
 
 
+static const PropertyAttributes ro_attr =
+    static_cast<PropertyAttributes>(READ_ONLY | DONT_DELETE | DONT_ENUM);

[Michael Starzinger, 2012/11/20 12:05:05]

It might make sense to move these constants into the PropertyAttributes class
itself, so that others can use them as well.

[Sven Panne, 2012/11/20 14:39:13]

... and it would make even more sense to clean up the confusion between elements
of a set and a set itself, but that is a different story. :-)

[rossberg, 2012/11/20 17:23:45]

Done.

[rossberg, 2012/11/20 17:23:45]

Yeah...

+static const PropertyAttributes rw_attr =
+    static_cast<PropertyAttributes>(DONT_DELETE | DONT_ENUM);
+
+
+RUNTIME_FUNCTION(MaybeObject*, Runtime_DeclareModules) {
+  HandleScope scope(isolate);
+  ASSERT(args.length() == 1);
+  CONVERT_ARG_HANDLE_CHECKED(FixedArray, descriptions, 0);
+  Context* host_context = isolate->context();
+
+  // See FullCodeGen::LinkModules for the format of descriptions.
+
+  for (int i = 0; i < descriptions->length(); ++i) {
+    Handle<FixedArray> description(FixedArray::cast(descriptions->get(i)));
+    int host_index = Smi::cast(description->get(0))->value();
+    Handle<Context> context(Context::cast(host_context->get(host_index)));
+    Handle<JSModule> module(context->module());
+
+    // And populate it.
+    for (int j = 1; j < description->length(); j += 3) {
+      Handle<String> name(String::cast(description->get(j)));
+      VariableMode mode =
+         static_cast<VariableMode>(Smi::cast(description->get(j + 1))->value());
+      int index = Smi::cast(description->get(j + 2))->value();
+      switch (mode) {
+        case VAR:
+        case LET:
+        case CONST:
+        case CONST_HARMONY: {
+          PropertyAttributes attr =
+              IsImmutableVariableMode(mode) ? ro_attr : rw_attr;
+          Handle<AccessorInfo> info =
+              Accessors::MakeModuleExport(name, index, attr);
+          Handle<Object> result = SetAccessor(module, info);
+          ASSERT(!(result.is_null() || result->IsUndefined()));
+          USE(result);
+          break;
+        }
+        case MODULE: {
+          Object* referenced_context = Context::cast(host_context)->get(index);
+          Handle<JSModule> value(Context::cast(referenced_context)->module());
+          JSReceiver::SetProperty(module, name, value, ro_attr, kStrictMode);
+          break;
+        }
+        case INTERNAL:
+        case TEMPORARY:
+        case DYNAMIC:
+        case DYNAMIC_GLOBAL:
+        case DYNAMIC_LOCAL:
+          UNREACHABLE();
+      }
+    }
+
+    USE(JSObject::PreventExtensions(module));

[Michael Starzinger, 2012/11/20 12:05:05]

There should be no need to use the return value of the handlified version.

[rossberg, 2012/11/20 17:23:45]

Done.

+  }
+
+  ASSERT(!isolate->has_pending_exception());
+  return isolate->heap()->undefined_value();
+}
+
+
 RUNTIME_FUNCTION(MaybeObject*, Runtime_DeleteContextSlot) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 2);
 
   CONVERT_ARG_HANDLE_CHECKED(Context, context, 0);
   CONVERT_ARG_HANDLE_CHECKED(String, name, 1);
 
   int index;
   PropertyAttributes attributes;
   ContextLookupFlags flags = FOLLOW_CHAINS;
@@ skipping 4771 matching lines @@
     // Handle last resort GC and make sure to allow future allocations
     // to grow the heap without causing GCs (if possible).
     isolate->counters()->gc_last_resort_from_js()->Increment();
     isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags,
                                        "Runtime::PerformGC");
   }
 }
 
 
 } }  // namespace v8::internal