[modules] Fix lazy parsing of functions in modules.

The information that such functions must be parsed in module mode
didn't get properly propagated.

Also refactor some related code to make it more robust. In particular,
set parsing_module_ at parser construction time only.

Bug: v8:1569, v8:6919
Change-Id: Id136fb15c240373cad07c82025b778d0c0c43148
Reviewed-on: https://chromium-review.googlesource.com/716478
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48811}

src/ast/scopes.cc

@@ -1444,16 +1444,6 @@ bool Scope::NeedsScopeInfo() const {
   return NeedsContext();
 }
 
-ModuleScope* Scope::GetModuleScope() {
-  Scope* scope = this;
-  DCHECK(!scope->is_script_scope());
-  while (!scope->is_module_scope()) {
-    scope = scope->outer_scope();
-    DCHECK_NOT_NULL(scope);
-  }
-  return scope->AsModuleScope();
-}
-
 DeclarationScope* Scope::GetReceiverScope() {
   Scope* scope = this;
   while (!scope->is_script_scope() &&

src/ast/scopes.h

@@ -442,9 +442,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // 'this' is bound, and what determines the function kind.
   DeclarationScope* GetReceiverScope();
 
-  // Find the module scope, assuming there is one.
-  ModuleScope* GetModuleScope();
-
   // Find the innermost outer scope that needs a context.
   Scope* GetOuterScopeWithContext();
 

src/compiler.cc

@@ -1102,6 +1102,7 @@ MaybeHandle<JSFunction> Compiler::GetFunctionFromEval(
     if (!context->IsNativeContext()) {
       parse_info.set_outer_scope_info(handle(context->scope_info()));
     }
+    DCHECK(!parse_info.is_module());
 
     if (!CompileToplevel(&parse_info, isolate).ToHandle(&shared_info)) {
       return MaybeHandle<JSFunction>();

src/debug/debug-scopes.cc

@@ -105,7 +105,7 @@ void ScopeIterator::TryParseAndRetrieveScopes(ScopeIterator::Option option) {
       // Retrieve it from shared function info.
       info->set_language_mode(shared_info->language_mode());
     } else if (scope_info->scope_type() == MODULE_SCOPE) {
-      info->set_module();
+      DCHECK(info->is_module());
     } else {
       DCHECK(scope_info->scope_type() == SCRIPT_SCOPE);
     }

src/parsing/parse-info.cc

@@ -53,13 +53,14 @@ ParseInfo::ParseInfo(Handle<SharedFunctionInfo> shared)
   set_end_position(shared->end_position());
   function_literal_id_ = shared->function_literal_id();
   set_language_mode(shared->language_mode());
-  set_module(shared->kind() == FunctionKind::kModule);
   set_asm_wasm_broken(shared->is_asm_wasm_broken());
 
   Handle<Script> script(Script::cast(shared->script()));
   set_script(script);
   set_native(script->type() == Script::TYPE_NATIVE);
   set_eval(script->compilation_type() == Script::COMPILATION_TYPE_EVAL);
+  set_module(script->origin_options().IsModule());
+  DCHECK(!(is_eval() && is_module()));
 
   Handle<HeapObject> scope_info(shared->outer_scope_info());
   if (!scope_info->IsTheHole(isolate) &&
@@ -90,6 +91,8 @@ ParseInfo::ParseInfo(Handle<Script> script)
 
   set_native(script->type() == Script::TYPE_NATIVE);
   set_eval(script->compilation_type() == Script::COMPILATION_TYPE_EVAL);
+  set_module(script->origin_options().IsModule());
+  DCHECK(!(is_eval() && is_module()));
 
   set_collect_type_profile(script->GetIsolate()->is_collecting_type_profile() &&
                            script->IsUserJavaScript());
@@ -114,7 +117,6 @@ ParseInfo* ParseInfo::AllocateWithoutScript(Handle<SharedFunctionInfo> shared) {
   p->set_end_position(shared->end_position());
   p->function_literal_id_ = shared->function_literal_id();
   p->set_language_mode(shared->language_mode());
-  p->set_module(shared->kind() == FunctionKind::kModule);
 
   // BUG(5946): This function exists as a workaround until we can
   // get rid of %SetCode in our native functions. The ParseInfo
@@ -126,6 +128,8 @@ ParseInfo* ParseInfo::AllocateWithoutScript(Handle<SharedFunctionInfo> shared) {
   // We tolerate a ParseInfo without a Script in this case.
   p->set_native(true);
   p->set_eval(false);
+  p->set_module(false);
+  DCHECK_NE(shared->kind(), FunctionKind::kModule);
 
   Handle<HeapObject> scope_info(shared->outer_scope_info());
   if (!scope_info->IsTheHole(isolate) &&

src/parsing/parser-base.h

@@ -251,7 +251,7 @@ class ParserBase {
 
   ParserBase(Zone* zone, Scanner* scanner, uintptr_t stack_limit,
              v8::Extension* extension, AstValueFactory* ast_value_factory,
-             RuntimeCallStats* runtime_call_stats,
+             RuntimeCallStats* runtime_call_stats, bool parsing_module,
              bool parsing_on_main_thread = true)
       : scope_(nullptr),
         original_scope_(nullptr),
@@ -262,7 +262,7 @@ class ParserBase {
         ast_node_factory_(ast_value_factory, zone),
         runtime_call_stats_(runtime_call_stats),
         parsing_on_main_thread_(parsing_on_main_thread),
-        parsing_module_(false),
+        parsing_module_(parsing_module),
         stack_limit_(stack_limit),
         zone_(zone),
         classifier_(nullptr),
@@ -1486,7 +1486,7 @@ class ParserBase {
   typename Types::Factory ast_node_factory_;
   RuntimeCallStats* runtime_call_stats_;
   bool parsing_on_main_thread_;
-  bool parsing_module_;
+  const bool parsing_module_;
   uintptr_t stack_limit_;
 
   // Parser base's private field members.

src/parsing/parser.cc

@@ -472,7 +472,7 @@ Expression* Parser::NewV8Intrinsic(const AstRawString* name,
 Parser::Parser(ParseInfo* info)
     : ParserBase<Parser>(info->zone(), &scanner_, info->stack_limit(),
                          info->extension(), info->GetOrCreateAstValueFactory(),
-                         info->runtime_call_stats(), true),
+                         info->runtime_call_stats(), info->is_module(), true),
       scanner_(info->unicode_cache(), use_counts_),
       reusable_preparser_(nullptr),
       mode_(PARSE_EAGERLY),  // Lazy mode must be set explicitly.
@@ -537,7 +537,6 @@ void Parser::DeserializeScopeChain(
     scope = Scope::DeserializeScopeChain(
         zone(), *outer_scope_info, script_scope, ast_value_factory(),
         Scope::DeserializationMode::kScopesOnly);
-    DCHECK(!info->is_module() || scope->is_module_scope());
   }
   original_scope_ = scope;
 }
@@ -630,7 +629,6 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
   {
     Scope* outer = original_scope_;
     DCHECK_NOT_NULL(outer);
-    parsing_module_ = info->is_module();
     if (info->is_eval()) {
       outer = NewEvalScope(outer);
     } else if (parsing_module_) {
@@ -648,6 +646,7 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
     bool ok = true;
     int beg_pos = scanner()->location().beg_pos;
     if (parsing_module_) {
+      DCHECK(info->is_module());
       // Declare the special module parameter.
       auto name = ast_value_factory()->empty_string();
       bool is_duplicate = false;
@@ -2775,8 +2774,8 @@ Parser::LazyParsingResult Parser::SkipFunction(
   DCHECK(!is_inner_function || !may_abort);
 
   PreParser::PreParseResult result = reusable_preparser()->PreParseFunction(
-      function_name, kind, function_type, function_scope, parsing_module_,
-      is_inner_function, may_abort, use_counts_, produced_preparsed_scope_data);
+      function_name, kind, function_type, function_scope, is_inner_function,
+      may_abort, use_counts_, produced_preparsed_scope_data);
 
   // Return immediately if pre-parser decided to abort parsing.
   if (result == PreParser::kPreParseAbort) return kLazyParsingAborted;

src/parsing/parser.h

@@ -287,7 +287,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
       reusable_preparser_ =
           new PreParser(zone(), &scanner_, stack_limit_, ast_value_factory(),
                         &pending_error_handler_, runtime_call_stats_,
-                        parsing_on_main_thread_);
+                        parsing_module_, parsing_on_main_thread_);
 #define SET_ALLOW(name) reusable_preparser_->set_allow_##name(allow_##name());
       SET_ALLOW(natives);
       SET_ALLOW(harmony_do_expressions);

src/parsing/preparser.cc

@@ -87,7 +87,7 @@ PreParserIdentifier PreParser::GetSymbol() const {
   return symbol;
 }
 
-PreParser::PreParseResult PreParser::PreParseProgram(bool is_module) {
+PreParser::PreParseResult PreParser::PreParseProgram() {
   DCHECK_NULL(scope_);
   DeclarationScope* scope = NewScriptScope();
 #ifdef DEBUG
@@ -97,13 +97,12 @@ PreParser::PreParseResult PreParser::PreParseProgram(bool is_module) {
   // ModuleDeclarationInstantiation for Source Text Module Records creates a
   // new Module Environment Record whose outer lexical environment record is
   // the global scope.
-  if (is_module) scope = NewModuleScope(scope);
+  if (parsing_module_) scope = NewModuleScope(scope);
 
   FunctionState top_scope(&function_state_, &scope_, scope);
   original_scope_ = scope_;
   bool ok = true;
   int start_position = scanner()->peek_location().beg_pos;
-  parsing_module_ = is_module;
   PreParserStatementList body;
   ParseStatementList(body, Token::EOS, &ok);
   original_scope_ = nullptr;
@@ -119,11 +118,10 @@ PreParser::PreParseResult PreParser::PreParseProgram(bool is_module) {
 PreParser::PreParseResult PreParser::PreParseFunction(
     const AstRawString* function_name, FunctionKind kind,
     FunctionLiteral::FunctionType function_type,
-    DeclarationScope* function_scope, bool parsing_module,
-    bool is_inner_function, bool may_abort, int* use_counts,
+    DeclarationScope* function_scope, bool is_inner_function, bool may_abort,
+    int* use_counts,
     ProducedPreParsedScopeData** produced_preparsed_scope_data) {
   DCHECK_EQ(FUNCTION_SCOPE, function_scope->scope_type());
-  parsing_module_ = parsing_module;
   use_counts_ = use_counts;
   DCHECK(!track_unresolved_variables_);
   track_unresolved_variables_ = is_inner_function;

src/parsing/preparser.h

@@ -880,11 +880,11 @@ class PreParser : public ParserBase<PreParser> {
   PreParser(Zone* zone, Scanner* scanner, uintptr_t stack_limit,
             AstValueFactory* ast_value_factory,
             PendingCompilationErrorHandler* pending_error_handler,
-            RuntimeCallStats* runtime_call_stats,
+            RuntimeCallStats* runtime_call_stats, bool parsing_module = false,
             bool parsing_on_main_thread = true)
       : ParserBase<PreParser>(zone, scanner, stack_limit, nullptr,
                               ast_value_factory, runtime_call_stats,
-                              parsing_on_main_thread),
+                              parsing_module, parsing_on_main_thread),
         use_counts_(nullptr),
         track_unresolved_variables_(false),
         pending_error_handler_(pending_error_handler),
@@ -898,7 +898,7 @@ class PreParser : public ParserBase<PreParser> {
   // success (even if parsing failed, the pre-parse data successfully
   // captured the syntax error), and false if a stack-overflow happened
   // during parsing.
-  PreParseResult PreParseProgram(bool is_module = false);
+  PreParseResult PreParseProgram();
 
   // Parses a single function literal, from the opening parentheses before
   // parameters to the closing brace after the body.
@@ -911,8 +911,8 @@ class PreParser : public ParserBase<PreParser> {
   PreParseResult PreParseFunction(
       const AstRawString* function_name, FunctionKind kind,
       FunctionLiteral::FunctionType function_type,
-      DeclarationScope* function_scope, bool parsing_module,
-      bool track_unresolved_variables, bool may_abort, int* use_counts,
+      DeclarationScope* function_scope, bool track_unresolved_variables,
+      bool may_abort, int* use_counts,
       ProducedPreParsedScopeData** produced_preparser_scope_data);
 
   ProducedPreParsedScopeData* produced_preparsed_scope_data() const {

test/cctest/test-parsing.cc

@@ -1383,10 +1383,11 @@ void TestParserSyncWithFlags(i::Handle<i::String> source,
         CcTest::i_isolate()->heap()->HashSeed());
     i::PreParser preparser(&zone, &scanner, stack_limit, &ast_value_factory,
                            &pending_error_handler,
-                           isolate->counters()->runtime_call_stats());
+                           isolate->counters()->runtime_call_stats(),
+                           is_module);
     SetParserFlags(&preparser, flags);
     scanner.Initialize(stream.get(), is_module);
-    i::PreParser::PreParseResult result = preparser.PreParseProgram(is_module);
+    i::PreParser::PreParseResult result = preparser.PreParseProgram();
     CHECK_EQ(i::PreParser::kPreParseSuccess, result);
   }
 

test/mjsunit/harmony/modules-import-meta.js

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 // MODULE
-// Flags: --harmony-import-meta --no-lazy
+// Flags: --harmony-import-meta
 
 import foreign, { url as otherUrl } from './modules-skip-export-import-meta.js';