[parser] Move MakeNonSimpleParameter to DeclarationScope

Walk the VariableMap instead of the ast.

Change-Id: I03ee9145230bcbfe04c5e31dc8d8b3a98a00a4be
Reviewed-on: https://chromium-review.googlesource.com/c/1424865
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58968}

src/ast/scopes.h

@@ -889,6 +889,15 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
     has_simple_parameters_ = false;
   }
 
+  void MakeParametersNonSimple() {
+    SetHasNonSimpleParameters();
+    for (ZoneHashMap::Entry* p = variables_.Start(); p != nullptr;
+         p = variables_.Next(p)) {
+      Variable* var = reinterpret_cast<Variable*>(p->value);
+      if (var->is_parameter()) var->MakeParameterNonSimple();
+    }
+  }
+
   // Returns whether the arguments object aliases formal parameters.
   CreateArgumentsType GetArgumentsType() const {
     DCHECK(is_function_scope());

src/ast/variables.h

@@ -176,7 +176,7 @@ class Variable final : public ZoneObject {
     index_ = index;
   }
 
-  void MakeNonSimpleParameter() {
+  void MakeParameterNonSimple() {
     DCHECK(is_parameter());
     bit_field_ = VariableModeField::update(bit_field_, VariableMode::kLet);
     bit_field_ =

src/parsing/parser.h

@@ -867,7 +867,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   V8_INLINE void DeclareFormalParameters(ParserFormalParameters* parameters) {
     bool is_simple = parameters->is_simple;
     DeclarationScope* scope = parameters->scope;
-    if (!is_simple) scope->SetHasNonSimpleParameters();
+    if (!is_simple) scope->MakeParametersNonSimple();
     for (auto parameter : parameters->params) {
       bool is_optional = parameter->initializer() != nullptr;
       // If the parameter list is simple, declare the parameters normally with

src/parsing/pattern-rewriter.cc

@@ -34,11 +34,9 @@ class PatternRewriter final : public AstVisitor<PatternRewriter> {
       const Parser::DeclarationParsingResult::Declaration* declaration);
 
  private:
-  PatternRewriter(Parser* parser, VariableKind kind,
-                  int initializer_position = kNoSourcePosition,
-                  bool declares_parameter_containing_sloppy_eval = false)
+  PatternRewriter(Parser* parser, VariableKind kind, int initializer_position,
+                  bool declares_parameter_containing_sloppy_eval)
       : parser_(parser),
-        kind_(kind),
         initializer_position_(initializer_position),
         declares_parameter_containing_sloppy_eval_(
             declares_parameter_containing_sloppy_eval) {}
@@ -48,8 +46,6 @@ class PatternRewriter final : public AstVisitor<PatternRewriter> {
   AST_NODE_LIST(DECLARE_VISIT)
 #undef DECLARE_VISIT
 
-  void RecurseIntoSubpattern(AstNode* pattern) { Visit(pattern); }
-
   Expression* Visit(Assignment* assign) {
     if (parser_->has_error()) return parser_->FailureExpression();
     DCHECK_EQ(Token::ASSIGN, assign->op());
@@ -69,7 +65,6 @@ class PatternRewriter final : public AstVisitor<PatternRewriter> {
   Scope* scope() const { return parser_->scope(); }
 
   Parser* const parser_;
-  VariableKind kind_;
   const int initializer_position_;
   const bool declares_parameter_containing_sloppy_eval_;
 
@@ -80,17 +75,22 @@ void Parser::InitializeVariables(
     ScopedPtrList<Statement>* statements, VariableKind kind,
     const DeclarationParsingResult::Declaration* declaration) {
   if (has_error()) return;
-  PatternRewriter::InitializeVariables(this, kind, declaration);
 
-  if (declaration->initializer) {
-    int pos = declaration->value_beg_position;
-    if (pos == kNoSourcePosition) {
-      pos = declaration->initializer_position;
-    }
-    Assignment* assignment = factory()->NewAssignment(
-        Token::INIT, declaration->pattern, declaration->initializer, pos);
-    statements->Add(factory()->NewExpressionStatement(assignment, pos));
+  if (!declaration->initializer) {
+    // The parameter scope is only a block scope if the initializer calls sloppy
+    // eval. Since there is no initializer, we can't be calling sloppy eval.
+    DCHECK_IMPLIES(kind == PARAMETER_VARIABLE, scope()->is_function_scope());
+    return;
   }
+
+  PatternRewriter::InitializeVariables(this, kind, declaration);
+  int pos = declaration->value_beg_position;
+  if (pos == kNoSourcePosition) {
+    pos = declaration->initializer_position;
+  }
+  Assignment* assignment = factory()->NewAssignment(
+      Token::INIT, declaration->pattern, declaration->initializer, pos);
+  statements->Add(factory()->NewExpressionStatement(assignment, pos));
 }
 
 void PatternRewriter::InitializeVariables(
@@ -100,7 +100,7 @@ void PatternRewriter::InitializeVariables(
       parser, kind, declaration->initializer_position,
       kind == PARAMETER_VARIABLE && parser->scope()->is_block_scope());
 
-  rewriter.RecurseIntoSubpattern(declaration->pattern);
+  rewriter.Visit(declaration->pattern);
 }
 
 void PatternRewriter::VisitVariableProxy(VariableProxy* proxy) {
@@ -110,11 +110,8 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* proxy) {
           ? proxy->var()
           : scope()->GetDeclarationScope()->LookupLocal(proxy->raw_name());
 
-  // TODO(verwaest): Use ScopedPtrList of Variable(Proxy?) in the
-  // ExpressionScope instead.
-  if (kind_ == PARAMETER_VARIABLE) var->MakeNonSimpleParameter();
-
   DCHECK_NOT_NULL(var);
+
   DCHECK_NE(initializer_position_, kNoSourcePosition);
   var->set_initializer_position(initializer_position_);
 }
@@ -137,14 +134,14 @@ void PatternRewriter::VisitObjectLiteral(ObjectLiteral* pattern) {
       // scope rewriting.
       RewriteParameterScopes(key);
     }
-    RecurseIntoSubpattern(property->value());
+    Visit(property->value());
   }
 }
 
 void PatternRewriter::VisitArrayLiteral(ArrayLiteral* node) {
   for (Expression* value : *node->values()) {
     if (value->IsTheHoleLiteral()) continue;
-    RecurseIntoSubpattern(value);
+    Visit(value);
   }
 }
 
@@ -154,12 +151,10 @@ void PatternRewriter::VisitAssignment(Assignment* node) {
   // Initializer may have been parsed in the wrong scope.
   RewriteParameterScopes(node->value());
 
-  RecurseIntoSubpattern(node->target());
+  Visit(node->target());
 }
 
-void PatternRewriter::VisitSpread(Spread* node) {
-  RecurseIntoSubpattern(node->expression());
-}
+void PatternRewriter::VisitSpread(Spread* node) { Visit(node->expression()); }
 
 // =============== UNREACHABLE =============================