Make non-Module generators only context allocate parameters.

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 <algorithm>
 #include <memory>
 
 #include "src/api.h"
@@ skipping 688 matching lines @@
       bool is_duplicate;
       bool is_rest = false;
       bool is_optional = false;
       auto var =
           scope->DeclareParameter(name, VAR, is_optional, is_rest,
                                   &is_duplicate, ast_value_factory(), beg_pos);
       DCHECK(!is_duplicate);
       var->AllocateTo(VariableLocation::PARAMETER, 0);
 
       PrepareGeneratorVariables();
+      scope->ForceContextAllocation();

[neis, 2017/05/24 10:39:37]

Do we still need this? If not, do we still want it or should we remove it in a
followup CL? Offhand I don't see a reason for keeping it.

[adamk, 2017/05/24 13:04:00]

Do you mean "is this call now redundant?" or "do we want to make module scopes
not all context-allocated?" The former is not the case: this call is necessary
to get module scopes to be context-allocated (a fact which we depend on to allow
preparsing of functions in modules). For the latter, it's not clear to me if the
tradeoffs are different than the ones in generators, so I mostly wanted to avoid
making that decision for modules in this patch.

[neis, 2017/05/24 13:59:46]

I meant do we still need/want to force context allocation for modules. I'll
check if there's still a technical reason to do so.

[adamk, 2017/05/24 14:31:22]

This would depend on how advanced the preparsing-of-inner-functions work is, so
checking with Marja seems like the right next step.

       Expression* initial_yield =
           BuildInitialYield(kNoSourcePosition, kGeneratorFunction);
       body->Add(
           factory()->NewExpressionStatement(initial_yield, kNoSourcePosition),
           zone());
 
       ParseModuleItemList(body, &ok);
       ok = ok &&
            module()->Validate(this->scope()->AsModuleScope(),
                               &pending_error_handler_, zone());
@@ skipping 1806 matching lines @@
   DeclareFormalParameters(parameters->scope, parameters->params);
   if (!this->classifier()
            ->is_valid_formal_parameter_list_without_duplicates()) {
     *duplicate_loc =
         this->classifier()->duplicate_formal_parameter_error().location;
   }
   DCHECK_EQ(parameters->is_simple, parameters->scope->has_simple_parameters());
 }
 
 void Parser::PrepareGeneratorVariables() {
-  // For generators, allocating variables in contexts is currently a win because
-  // it minimizes the work needed to suspend and resume an activation.  The
-  // code produced for generators relies on this forced context allocation (it
-  // does not restore the frame's parameters upon resume).
-  function_state_->scope()->ForceContextAllocation();
+  // The code produced for generators relies on forced context allocation of
+  // parameters (it does not restore the frame's parameters upon resume).
+  function_state_->scope()->ForceContextAllocationForParameters();
 
   // Calling a generator returns a generator object.  That object is stored
   // in a temporary variable, a definition that is used by "yield"
   // expressions.
   function_state_->scope()->DeclareGeneratorObjectVar(
       ast_value_factory()->dot_generator_object_string());
 }
 
 FunctionLiteral* Parser::ParseFunctionLiteral(
     const AstRawString* function_name, Scanner::Location function_name_location,
@@ skipping 2594 matching lines @@
     literal->SetShouldEagerCompile();
   }
 }
 
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8