Fully deserialize the scope chain after parsing, not before

src/parsing/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/parsing/parser.h"
 
 #include <memory>
 
 #include "src/api.h"
 #include "src/ast/ast-expression-rewriter.h"
@@ skipping 585 matching lines @@
   }
   if (info->ast_value_factory() == NULL) {
     // info takes ownership of AstValueFactory.
     info->set_ast_value_factory(new AstValueFactory(zone(), info->hash_seed()));
     info->set_ast_value_factory_owned();
     ast_value_factory_ = info->ast_value_factory();
     ast_node_factory_.set_ast_value_factory(ast_value_factory_);
   }
 }
 
-void Parser::DeserializeScopeChain(
-    ParseInfo* info, Handle<Context> context,
-    Scope::DeserializationMode deserialization_mode) {
+void Parser::DeserializeScopeChain(ParseInfo* info,
+                                   MaybeHandle<Context> maybe_context) {
   DCHECK(ThreadId::Current().Equals(info->isolate()->thread_id()));
   // TODO(wingo): Add an outer SCRIPT_SCOPE corresponding to the native
   // context, which will have the "this" binding for script scopes.
   DeclarationScope* script_scope = NewScriptScope();
   info->set_script_scope(script_scope);
   Scope* scope = script_scope;
-  if (!context.is_null() && !context->IsNativeContext()) {
-    scope = Scope::DeserializeScopeChain(info->isolate(), zone(), *context,
-                                         script_scope, ast_value_factory(),
-                                         deserialization_mode);
+  Handle<Context> context;
+  if (maybe_context.ToHandle(&context) && !context->IsNativeContext()) {
+    scope = Scope::DeserializeScopeChain(
+        info->isolate(), zone(), *context, script_scope, ast_value_factory(),
+        Scope::DeserializationMode::kScopesOnly);
     DCHECK(!info->is_module() || scope->is_module_scope());
-    if (info->context().is_null()) {
-      DCHECK(deserialization_mode ==
-             Scope::DeserializationMode::kDeserializeOffHeap);
-    } else {
-      // The Scope is backed up by ScopeInfo (which is in the V8 heap); this
-      // means the Parser cannot operate independent of the V8 heap. Tell the
-      // string table to internalize strings and values right after they're
-      // created. This kind of parsing can only be done in the main thread.
-      DCHECK(parsing_on_main_thread_);
-      ast_value_factory()->Internalize(info->isolate());
-    }
   }
   original_scope_ = scope;
 }
 
 FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
   // TODO(bmeurer): We temporarily need to pass allow_nesting = true here,
   // see comment for HistogramTimerScope class.
 
   // It's OK to use the Isolate & counters here, since this function is only
   // called in the main thread.
@@ skipping 12 matching lines @@
 
   // Initialize parser state.
   CompleteParserRecorder recorder;
 
   if (produce_cached_parse_data()) {
     log_ = &recorder;
   } else if (consume_cached_parse_data()) {
     cached_parse_data_->Initialize();
   }
 
-  DeserializeScopeChain(info, info->context(),
-                        Scope::DeserializationMode::kKeepScopeInfo);
+  DeserializeScopeChain(info, info->context());
 
   source = String::Flatten(source);
   FunctionLiteral* result;
 
   {
     std::unique_ptr<Utf16CharacterStream> stream;
     if (source->IsExternalTwoByteString()) {
       stream.reset(new ExternalTwoByteStringUtf16CharacterStream(
           Handle<ExternalTwoByteString>::cast(source), 0, source->length()));
     } else if (source->IsExternalOneByteString()) {
@@ skipping 98 matching lines @@
     if (ok && is_strict(language_mode())) {
       CheckStrictOctalLiteral(beg_pos, scanner()->location().end_pos, &ok);
       CheckDecimalLiteralWithLeadingZero(beg_pos,
                                          scanner()->location().end_pos);
     }
     if (ok && is_sloppy(language_mode())) {
       // TODO(littledan): Function bindings on the global object that modify
       // pre-existing bindings should be made writable, enumerable and
       // nonconfigurable if possible, whereas this code will leave attributes
       // unchanged if the property already exists.
-      InsertSloppyBlockFunctionVarBindings(scope, &ok);
+      InsertSloppyBlockFunctionVarBindings(scope);
     }
     if (ok) {
       CheckConflictingVarDeclarations(scope, &ok);
     }
 
     if (ok && info->parse_restriction() == ONLY_SINGLE_FUNCTION_LITERAL) {
       if (body->length() != 1 ||
           !body->at(0)->IsExpressionStatement() ||
           !body->at(0)->AsExpressionStatement()->
               expression()->IsFunctionLiteral()) {
@@ skipping 25 matching lines @@
   RuntimeCallTimerScope runtime_timer(isolate, &RuntimeCallStats::ParseLazy);
   HistogramTimerScope timer_scope(isolate->counters()->parse_lazy());
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"), "V8.ParseLazy");
   Handle<String> source(String::cast(info->script()->source()));
   isolate->counters()->total_parse_size()->Increment(source->length());
   base::ElapsedTimer timer;
   if (FLAG_trace_parse) {
     timer.Start();
   }
   Handle<SharedFunctionInfo> shared_info = info->shared_info();
-  DeserializeScopeChain(info, info->context(),
-                        Scope::DeserializationMode::kKeepScopeInfo);
+  DeserializeScopeChain(info, info->context());
 
   // Initialize parser state.
   source = String::Flatten(source);
   FunctionLiteral* result;
   {
     std::unique_ptr<Utf16CharacterStream> stream;
     if (source->IsExternalTwoByteString()) {
       stream.reset(new ExternalTwoByteStringUtf16CharacterStream(
           Handle<ExternalTwoByteString>::cast(source),
           shared_info->start_position(), shared_info->end_position()));
@@ skipping 2818 matching lines @@
 
   if (!parameters.is_simple) {
     DCHECK_NOT_NULL(inner_scope);
     DCHECK_EQ(function_scope, scope());
     DCHECK_EQ(function_scope, inner_scope->outer_scope());
     DCHECK_EQ(body, inner_block->statements());
     SetLanguageMode(function_scope, inner_scope->language_mode());
     Block* init_block = BuildParameterInitializationBlock(parameters, CHECK_OK);
 
     if (is_sloppy(inner_scope->language_mode())) {
-      InsertSloppyBlockFunctionVarBindings(inner_scope, CHECK_OK);
+      InsertSloppyBlockFunctionVarBindings(inner_scope);
     }
 
     // TODO(littledan): Merge the two rejection blocks into one
     if (IsAsyncFunction(kind)) {
       init_block = BuildRejectPromiseOnException(init_block, CHECK_OK);
     }
 
     DCHECK_NOT_NULL(init_block);
 
     inner_scope->set_end_position(scanner()->location().end_pos);
     if (inner_scope->FinalizeBlockScope() != nullptr) {
       CheckConflictingVarDeclarations(inner_scope, CHECK_OK);
       InsertShadowingVarBindingInitializers(inner_block);
     }
     inner_scope = nullptr;
 
     result->Add(init_block, zone());
     result->Add(inner_block, zone());
   } else {
     DCHECK_EQ(inner_scope, function_scope);
     if (is_sloppy(function_scope->language_mode())) {
-      InsertSloppyBlockFunctionVarBindings(function_scope, CHECK_OK);
+      InsertSloppyBlockFunctionVarBindings(function_scope);
     }
   }
 
   if (function_type == FunctionLiteral::kNamedExpression) {
     // Now that we know the language mode, we can create the const assignment
     // in the previously reserved spot.
     DCHECK_EQ(function_scope, scope());
     Variable* fvar = function_scope->DeclareFunctionVar(function_name);
     VariableProxy* fproxy = factory()->NewVariableProxy(fvar);
     result->Set(kFunctionNameAssignmentIndex,
@@ skipping 197 matching lines @@
     VariableProxy* to = NewUnresolved(name);
     VariableProxy* from = factory()->NewVariableProxy(parameter);
     Expression* assignment =
         factory()->NewAssignment(Token::ASSIGN, to, from, kNoSourcePosition);
     Statement* statement =
         factory()->NewExpressionStatement(assignment, kNoSourcePosition);
     inner_block->statements()->InsertAt(0, statement, zone());
   }
 }
 
-void Parser::InsertSloppyBlockFunctionVarBindings(DeclarationScope* scope,
-                                                  bool* ok) {
-  scope->HoistSloppyBlockFunctions(factory(), CHECK_OK_VOID);
-
-  SloppyBlockFunctionMap* map = scope->sloppy_block_function_map();
-  for (ZoneHashMap::Entry* p = map->Start(); p != nullptr; p = map->Next(p)) {
-    // Write in assignments to var for each block-scoped function declaration
-    auto delegates = static_cast<SloppyBlockFunctionStatement*>(p->value);
-    for (SloppyBlockFunctionStatement* delegate = delegates;
-         delegate != nullptr; delegate = delegate->next()) {
-      if (delegate->to() == nullptr) {
-        continue;
-      }
-      Expression* assignment = factory()->NewAssignment(
-          Token::ASSIGN, delegate->to(), delegate->from(), kNoSourcePosition);
-      Statement* statement =
-          factory()->NewExpressionStatement(assignment, kNoSourcePosition);
-      delegate->set_statement(statement);
-    }
+void Parser::InsertSloppyBlockFunctionVarBindings(DeclarationScope* scope) {
+  // For the outer most eval scope (which is inside a script scope), don't
+  // hoist during parsing, but later during DeclarationScope::Analyze when the
+  // actual scope chain is available.

[marja, 2016/09/14 09:45:21]

Nit: this comment still doesn't explain why. Also, is the outermost eval scope
necessarily inside a script scope? What if we're just parsing an eval which is
inside a function scope? That's confusing.

Maybe something like this:

For the outermost eval scope, we cannot hoist during parsing: let declarations
in the surrounding scope may prevent hoisting, but the information is
unaccessible during parsing. In this case, we hoist later in
DeclarationScope::Analyze.

[jochen (gone - plz use gerrit), 2016/09/14 11:27:30]

done

+  if (scope->is_eval_scope() && scope->outer_scope() == original_scope_) {
+    return;
   }
+  scope->HoistSloppyBlockFunctions(factory());
 }
 
-
 // ----------------------------------------------------------------------------
 // Parser support
 
 bool Parser::TargetStackContainsLabel(const AstRawString* label) {
   for (ParserTarget* t = target_stack_; t != NULL; t = t->previous()) {
     if (ContainsLabel(t->statement()->labels(), label)) return true;
   }
   return false;
 }
 
@@ skipping 1678 matching lines @@
   node->Print(Isolate::Current());
 }
 #endif  // DEBUG
 
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8