Fix up ParseProgram and ParseModule to do something sane with module scopes

The FunctionLiteral returned from the parser for modules now has a MODULE_SCOPE,
instead of associating the module scope with a Block inside it. This makes
it easy to get at the ModuleDescriptor from the caller of Parse(), so I've added
a basic test that pokes at the scope and the descriptor. Expect more tests
in this vein.

BUG=v8:1569
LOG=n

Review URL: https://codereview.chromium.org/953983002

Cr-Commit-Position: refs/heads/master@{#26836}

src/parser.cc

@@ -928,6 +928,8 @@ FunctionLiteral* Parser::DoParseProgram(CompilationInfo* info, Scope** scope,
       }
     } else if (info->is_global()) {
       *scope = NewScope(*scope, SCRIPT_SCOPE);
+    } else if (info->is_module()) {
+      *scope = NewScope(*scope, MODULE_SCOPE);
     }
     (*scope)->set_start_position(0);
     // End position will be set by the caller.
@@ -951,10 +953,7 @@ FunctionLiteral* Parser::DoParseProgram(CompilationInfo* info, Scope** scope,
     int beg_pos = scanner()->location().beg_pos;
     if (info->is_module()) {
       DCHECK(allow_harmony_modules());
-      Statement* stmt = ParseModule(&ok);
-      if (ok) {
-        body->Add(stmt, zone());
-      }
+      ParseModule(body, &ok);
     } else {
       ParseStatementList(body, Token::EOS, info->is_eval(), eval_scope, &ok);
     }
@@ -1254,7 +1253,7 @@ Statement* Parser::ParseModuleItem(bool* ok) {
 }
 
 
-Statement* Parser::ParseModule(bool* ok) {
+void* Parser::ParseModule(ZoneList<Statement*>* body, bool* ok) {
   // (Ecma 262 6th Edition, 15.2):
   // Module :
   //    ModuleBody?
@@ -1262,31 +1261,22 @@ Statement* Parser::ParseModule(bool* ok) {
   // ModuleBody :
   //    ModuleItem*
 
-  Block* body = factory()->NewBlock(NULL, 16, false, RelocInfo::kNoPosition);
-  Scope* scope = NewScope(scope_, MODULE_SCOPE);
-  scope->set_start_position(scanner()->location().beg_pos);
-  scope->SetLanguageMode(
-      static_cast<LanguageMode>(scope->language_mode() | STRICT_BIT));
-
-  {
-    BlockState block_state(&scope_, scope);
+  DCHECK(scope_->is_module_scope());
+  scope_->SetLanguageMode(
+      static_cast<LanguageMode>(scope_->language_mode() | STRICT_BIT));
 
   while (peek() != Token::EOS) {
     Statement* stat = ParseModuleItem(CHECK_OK);
     if (stat && !stat->IsEmpty()) {
-        body->AddStatement(stat, zone());
+      body->Add(stat, zone());
     }
   }
-  }
-
-  scope->set_end_position(scanner()->location().end_pos);
-  body->set_scope(scope);
 
   // Check that all exports are bound.
-  ModuleDescriptor* descriptor = scope->module();
+  ModuleDescriptor* descriptor = scope_->module();
   for (ModuleDescriptor::Iterator it = descriptor->iterator(); !it.done();
        it.Advance()) {
-    if (scope->LookupLocal(it.local_name()) == NULL) {
+    if (scope_->LookupLocal(it.local_name()) == NULL) {
       // TODO(adamk): Pass both local_name and export_name once ParserTraits
       // supports multiple arg error messages.
       // Also try to report this at a better location.
@@ -1296,8 +1286,8 @@ Statement* Parser::ParseModule(bool* ok) {
     }
   }
 
-  scope->module()->Freeze();
-  return body;
+  scope_->module()->Freeze();
+  return NULL;
 }
 
 

src/parser.h

@@ -703,7 +703,7 @@ class Parser : public ParserBase<ParserTraits> {
   void* ParseStatementList(ZoneList<Statement*>* body, int end_token,
                            bool is_eval, Scope** ad_hoc_eval_scope, bool* ok);
   Statement* ParseStatementListItem(bool* ok);
-  Statement* ParseModule(bool* ok);
+  void* ParseModule(ZoneList<Statement*>* body, bool* ok);
   Statement* ParseModuleItem(bool* ok);
   Literal* ParseModuleSpecifier(bool* ok);
   Statement* ParseImportDeclaration(bool* ok);

test/cctest/test-parsing.cc

@@ -5072,12 +5072,8 @@ TEST(BasicImportExportParsing) {
                                         128 * 1024);
 
   for (unsigned i = 0; i < arraysize(kSources); ++i) {
-    int kProgramByteSize = i::StrLength(kSources[i]);
-    i::ScopedVector<char> program(kProgramByteSize + 1);
-    i::SNPrintF(program, "%s", kSources[i]);
     i::Handle<i::String> source =
-        factory->NewStringFromUtf8(i::CStrVector(program.start()))
-            .ToHandleChecked();
+        factory->NewStringFromAsciiChecked(kSources[i]);
 
     // Show that parsing as a module works
     {
@@ -5197,12 +5193,8 @@ TEST(ImportExportParsingErrors) {
                                         128 * 1024);
 
   for (unsigned i = 0; i < arraysize(kErrorSources); ++i) {
-    int kProgramByteSize = i::StrLength(kErrorSources[i]);
-    i::ScopedVector<char> program(kProgramByteSize + 1);
-    i::SNPrintF(program, "%s", kErrorSources[i]);
     i::Handle<i::String> source =
-        factory->NewStringFromUtf8(i::CStrVector(program.start()))
-            .ToHandleChecked();
+        factory->NewStringFromAsciiChecked(kErrorSources[i]);
 
     i::Handle<i::Script> script = factory->NewScript(source);
     i::CompilationInfoWithZone info(script);
@@ -5217,6 +5209,42 @@ TEST(ImportExportParsingErrors) {
 }
 
 
+TEST(ModuleParsingInternals) {
+  i::Isolate* isolate = CcTest::i_isolate();
+  i::Factory* factory = isolate->factory();
+  v8::HandleScope handles(CcTest::isolate());
+  v8::Handle<v8::Context> context = v8::Context::New(CcTest::isolate());
+  v8::Context::Scope context_scope(context);
+  isolate->stack_guard()->SetStackLimit(i::GetCurrentStackPosition() -
+                                        128 * 1024);
+
+  static const char kSource[] = "let x = 5; export { x as y };";
+  i::Handle<i::String> source = factory->NewStringFromAsciiChecked(kSource);
+  i::Handle<i::Script> script = factory->NewScript(source);
+  i::CompilationInfoWithZone info(script);
+  i::AstValueFactory avf(info.zone(), isolate->heap()->HashSeed());
+  i::Parser parser(&info, isolate->stack_guard()->real_climit(),
+                   isolate->heap()->HashSeed(), isolate->unicode_cache());
+  parser.set_allow_harmony_modules(true);
+  parser.set_allow_harmony_scoping(true);
+  info.MarkAsModule();
+  CHECK(parser.Parse(&info));
+  i::FunctionLiteral* func = info.function();
+  CHECK_EQ(i::MODULE_SCOPE, func->scope()->scope_type());
+  i::ModuleDescriptor* descriptor = func->scope()->module();
+  CHECK_NOT_NULL(descriptor);
+  const i::AstRawString* name_x = avf.GetOneByteString("x");
+  const i::AstRawString* name_y = avf.GetOneByteString("y");
+  int num_exports = 0;
+  for (auto it = descriptor->iterator(); !it.done(); it.Advance()) {
+    ++num_exports;
+    CHECK(*name_x == *it.local_name());
+    CHECK(*name_y == *it.export_name());
+  }
+  CHECK_EQ(1, num_exports);
+}
+
+
 TEST(DuplicateProtoError) {
   const char* context_data[][2] = {
     {"({", "});"},