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

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index 735c77a2eca0b8e6ec227a5f488b630ce3a563ff..1df500cd29d4ce90a6df1354e19434a8208e57e2 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -1358,8 +1358,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DeclareGlobals) {
     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
@@ -1403,24 +1402,23 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DeclareGlobals) {
     // 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();
@@ -8478,20 +8476,103 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_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);
+
+  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);
 
-  Context* context = Context::cast(instance->context());
+  // 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();
 }