[parser] Remove hoist_scope from DeclarationDescriptor

The hoist_scope member of DeclarationDescriptor was only used to pass the function
scope for declaration of parameters containing sloppy evals, for example:

  function f(x = eval("var y")) { }

In cases like this, "x" is declared in the function scope but "y" is declared in an inner scope.
Rather than passing the function scope as "hoist_scope", we simply ask for the outer_scope()
of the inner scope as needed in PatternRewriter.

This reduces the cognitive overhead of understanding what a DeclarationDescriptor has; for
example, it removes some dead code from the PreParser which never has to deal
with a situation like the example above.

Review-Url: https://codereview.chromium.org/2662183002
Cr-Commit-Position: refs/heads/master@{#42861}

src/parsing/parser-base.h

@@ -611,7 +611,6 @@ class ParserBase {
   struct DeclarationDescriptor {
     enum Kind { NORMAL, PARAMETER };
     Scope* scope;
-    Scope* hoist_scope;
     VariableMode mode;
     int declaration_pos;
     int initialization_pos;
@@ -3694,7 +3693,6 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseVariableDeclarations(
   }
 
   parsing_result->descriptor.scope = scope();
-  parsing_result->descriptor.hoist_scope = nullptr;
 
   // The scope of a var/const declared variable anywhere inside a function
   // is the entire function (ECMA-262, 3rd, 10.1.3, and 12.2). The scope

src/parsing/parser.cc

@@ -1684,7 +1684,6 @@ void Parser::RewriteCatchPattern(CatchInfo* catch_info, bool* ok) {
     DeclarationDescriptor descriptor;
     descriptor.declaration_kind = DeclarationDescriptor::NORMAL;
     descriptor.scope = scope();
-    descriptor.hoist_scope = nullptr;
     descriptor.mode = LET;
     descriptor.declaration_pos = catch_info->pattern->position();
     descriptor.initialization_pos = catch_info->pattern->position();
@@ -2921,7 +2920,6 @@ Block* Parser::BuildParameterInitializationBlock(
     DeclarationDescriptor descriptor;
     descriptor.declaration_kind = DeclarationDescriptor::PARAMETER;
     descriptor.scope = scope();
-    descriptor.hoist_scope = nullptr;
     descriptor.mode = LET;
     descriptor.declaration_pos = parameter->pattern->position();
     // The position that will be used by the AssignmentExpression
@@ -2956,7 +2954,6 @@ Block* Parser::BuildParameterInitializationBlock(
       param_scope->RecordEvalCall();
       param_block = factory()->NewBlock(NULL, 8, true, kNoSourcePosition);
       param_block->set_scope(param_scope);
-      descriptor.hoist_scope = scope();
       // Pass the appropriate scope in so that PatternRewriter can appropriately
       // rewrite inner initializers of the pattern to param_scope
       descriptor.scope = param_scope;

src/parsing/parser.h

@@ -429,6 +429,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
     PatternContext SetAssignmentContextIfNeeded(Expression* node);
     PatternContext SetInitializerContextIfNeeded(Expression* node);
 
+    bool DeclaresParameterContainingSloppyEval() const;
     void RewriteParameterScopes(Expression* expr);
 
     Variable* CreateTempVar(Expression* value = nullptr);

src/parsing/pattern-rewriter.cc

@@ -137,22 +137,31 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
       factory()->NewVariableProxy(name, NORMAL_VARIABLE, pattern->position());
   Declaration* declaration = factory()->NewVariableDeclaration(
       proxy, descriptor_->scope, descriptor_->declaration_pos);
+
+  // When an extra declaration scope needs to be inserted to account for
+  // a sloppy eval in a default parameter or function body, the parameter
+  // needs to be declared in the function's scope, not in the varblock
+  // scope which will be used for the initializer expression.
+  Scope* outer_function_scope = nullptr;
+  if (DeclaresParameterContainingSloppyEval()) {
+    outer_function_scope = descriptor_->scope->outer_scope();
+  }
   Variable* var = parser_->Declare(
       declaration, descriptor_->declaration_kind, descriptor_->mode,
       Variable::DefaultInitializationFlag(descriptor_->mode), ok_,
-      descriptor_->hoist_scope);
+      outer_function_scope);
   if (!*ok_) return;
   DCHECK_NOT_NULL(var);
   DCHECK(proxy->is_resolved());
   DCHECK(initializer_position_ != kNoSourcePosition);
   var->set_initializer_position(initializer_position_);
 
-  // TODO(adamk): This should probably be checking hoist_scope.
+  Scope* declaration_scope =
-  // Move it to Parser::Declare() to make it easier to test
+      outer_function_scope != nullptr
-  // the right scope.
+          ? outer_function_scope
-  Scope* declaration_scope = IsLexicalVariableMode(descriptor_->mode)
+          : (IsLexicalVariableMode(descriptor_->mode)
-                                 ? descriptor_->scope
+                 ? descriptor_->scope
-                                 : descriptor_->scope->GetDeclarationScope();
+                 : descriptor_->scope->GetDeclarationScope());
   if (declaration_scope->num_var() > kMaxNumFunctionLocals) {
     parser_->ReportMessage(MessageTemplate::kTooManyVariables);
     *ok_ = false;
@@ -313,20 +322,31 @@ void Parser::PatternRewriter::VisitRewritableExpression(
   set_context(old_context);
 }
 
+bool Parser::PatternRewriter::DeclaresParameterContainingSloppyEval() const {
+  // Need to check for a binding context to make sure we have a descriptor.
+  if (IsBindingContext() &&
+      // Only relevant for parameters.
+      descriptor_->declaration_kind == DeclarationDescriptor::PARAMETER &&
+      // And only when scope is a block scope;
+      // without eval, it is a function scope.
+      scope()->is_block_scope()) {
+    DCHECK(scope()->calls_sloppy_eval());
+    DCHECK(scope()->is_declaration_scope());
+    DCHECK(scope()->outer_scope()->is_function_scope());
+    return true;
+  }
+
+  return false;
+}
+
 // When an extra declaration scope needs to be inserted to account for
 // a sloppy eval in a default parameter or function body, the expressions
 // needs to be in that new inner scope which was added after initial
 // parsing.
 void Parser::PatternRewriter::RewriteParameterScopes(Expression* expr) {
-  if (!IsBindingContext()) return;
+  if (DeclaresParameterContainingSloppyEval()) {
-  if (descriptor_->declaration_kind != DeclarationDescriptor::PARAMETER) return;
+    ReparentParameterExpressionScope(parser_->stack_limit(), expr, scope());
-  if (!scope()->is_block_scope()) return;
+  }
-
-  DCHECK(scope()->is_declaration_scope());
-  DCHECK(scope()->outer_scope()->is_function_scope());
-  DCHECK(scope()->calls_sloppy_eval());
-
-  ReparentParameterExpressionScope(parser_->stack_limit(), expr, scope());
 }
 
 void Parser::PatternRewriter::VisitObjectLiteral(ObjectLiteral* pattern,

src/parsing/preparser.cc

@@ -307,14 +307,10 @@ void PreParser::DeclareAndInitializeVariables(
     ZoneList<const AstRawString*>* names, bool* ok) {
   if (declaration->pattern.variables_ != nullptr) {
     DCHECK(FLAG_lazy_inner_functions);
-    Scope* scope = declaration_descriptor->hoist_scope;
-    if (scope == nullptr) {
-      scope = this->scope();
-    }
     for (auto variable : *(declaration->pattern.variables_)) {
       declaration_descriptor->scope->RemoveUnresolved(variable);
-      scope->DeclareVariableName(variable->raw_name(),
+      scope()->DeclareVariableName(variable->raw_name(),
-                                 declaration_descriptor->mode);
+                                   declaration_descriptor->mode);
     }
   }
 }