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

src/parser.cc

 // Copyright 2012 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/v8.h"
 
 #include "src/api.h"
 #include "src/ast.h"
 #include "src/bailout-reason.h"
 #include "src/base/platform/platform.h"
@@ skipping 910 matching lines @@
       DCHECK(parsing_on_main_thread_);
       ast_value_factory()->Internalize(info->isolate());
     }
     original_scope_ = *scope;
     if (info->is_eval()) {
       if (!(*scope)->is_script_scope() || is_strict(info->language_mode())) {
         *scope = NewScope(*scope, EVAL_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.
 
     // Compute the parsing mode.
     Mode mode = (FLAG_lazy && allow_lazy()) ? PARSE_LAZILY : PARSE_EAGERLY;
     if (allow_natives() || extension_ != NULL ||
         (*scope)->is_eval_scope()) {
       mode = PARSE_EAGERLY;
     }
     ParsingModeScope parsing_mode(this, mode);
 
     // Enters 'scope'.
     AstNodeFactory function_factory(ast_value_factory());
     FunctionState function_state(&function_state_, &scope_, *scope,
                                  kNormalFunction, &function_factory);
 
     scope_->SetLanguageMode(info->language_mode());
     ZoneList<Statement*>* body = new(zone()) ZoneList<Statement*>(16, zone());
     bool ok = true;
     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);
     }
 
     if (ok && is_strict(language_mode())) {
       CheckStrictOctalLiteral(beg_pos, scanner()->location().end_pos, &ok);
     }
 
     if (ok && allow_harmony_scoping() && is_strict(language_mode())) {
       CheckConflictingVarDeclarations(scope_, &ok);
@@ skipping 279 matching lines @@
     case Token::IMPORT:
       return ParseImportDeclaration(ok);
     case Token::EXPORT:
       return ParseExportDeclaration(ok);
     default:
       return ParseStatementListItem(ok);
   }
 }
 
 
-Statement* Parser::ParseModule(bool* ok) {
+void* Parser::ParseModule(ZoneList<Statement*>* body, bool* ok) {

[arv (Not doing code reviews), 2015/02/24 22:13:34]

Why return void* and not void?

[adamk, 2015/02/24 22:17:54]

For CHECK_OK.

We could do something else, like have two different sections of parse methods
that use CHECK_OK, not use CHECK_OK in void methods, or add CHECK_OK_VOID.

   // (Ecma 262 6th Edition, 15.2):
   // Module :
   //    ModuleBody?
   //
   // 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));
+  DCHECK(scope_->is_module_scope());
+  scope_->SetLanguageMode(
+      static_cast<LanguageMode>(scope_->language_mode() | STRICT_BIT));
 
-  {
-    BlockState block_state(&scope_, scope);
-
-    while (peek() != Token::EOS) {
-      Statement* stat = ParseModuleItem(CHECK_OK);
-      if (stat && !stat->IsEmpty()) {
-        body->AddStatement(stat, zone());
-      }
+  while (peek() != Token::EOS) {
+    Statement* stat = ParseModuleItem(CHECK_OK);
+    if (stat && !stat->IsEmpty()) {
+      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.
       ParserTraits::ReportMessage("module_export_undefined", it.local_name());
       *ok = false;
       return NULL;
     }
   }
 
-  scope->module()->Freeze();
-  return body;
+  scope_->module()->Freeze();
+  return NULL;
 }
 
 
 Literal* Parser::ParseModuleSpecifier(bool* ok) {
   // ModuleSpecifier :
   //    StringLiteral
 
   int pos = peek_position();
   Expect(Token::STRING, CHECK_OK);
   return factory()->NewStringLiteral(GetSymbol(scanner()), pos);
@@ skipping 4174 matching lines @@
     } else {
       const uc16* data = reinterpret_cast<const uc16*>(raw_string->raw_data());
       running_hash = StringHasher::ComputeRunningHash(running_hash, data,
                                                       raw_string->length());
     }
   }
 
   return running_hash;
 }
 } }  // namespace v8::internal