[parser] Disambiguate variables through expression-scope

Previously we'd always push variable proxies into the unresolved list of the
current scope, and possibly delete them from the list later in case they end up
being declarations. If variables become assigned, there were two ways to mark
them as such: The preparser would marked the variables tracked on the
PreParserExpression, and the parser would traverse the LHS AST to find and mark
all variables.

After this CL, if the scope already knows it's tracking declarations, the
variables are never added to the unresolved list in the first place. If the
scope is ambigous, it tracks the variable proxies on the side and only adds
them to the unresolved list if they end up being references rather than
declarations. The same list is now used to bulk mark all LHS variables as
assigned; uniformely for both the parser and the preparser.

In a next step we'll also use the scope to create declarations. That way we can
stop tracking variables_ on PreParserExpression altogether.

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

src/ast/ast-traversal-visitor.h

@@ -577,13 +577,6 @@ void AstTraversalVisitor<Subclass>::VisitSuperCallReference(
   RECURSE_EXPRESSION(VisitVariableProxy(expr->this_function_var()));
 }
 
-template <class Subclass>
-void AstTraversalVisitor<Subclass>::VisitRewritableExpression(
-    RewritableExpression* expr) {
-  PROCESS_EXPRESSION(expr);
-  RECURSE(Visit(expr->expression()));
-}
-
 #undef PROCESS_NODE
 #undef PROCESS_EXPRESSION
 #undef RECURSE_EXPRESSION

src/ast/ast.h

@@ -96,7 +96,6 @@ namespace internal {
   V(NativeFunctionLiteral)      \
   V(Property)                   \
   V(ResolvedProperty)           \
-  V(RewritableExpression)       \
   V(Spread)                     \
   V(StoreInArrayLiteral)        \
   V(SuperCallReference)         \
@@ -2197,58 +2196,6 @@ class CompoundAssignment final : public Assignment {
   BinaryOperation* binary_operation_;
 };
 
-// The RewritableExpression class is a wrapper for AST nodes that wait
-// for some potential rewriting.  However, even if such nodes are indeed
-// rewritten, the RewritableExpression wrapper nodes will survive in the
-// final AST and should be just ignored, i.e., they should be treated as
-// equivalent to the wrapped nodes.  For this reason and to simplify later
-// phases, RewritableExpressions are considered as exceptions of AST nodes
-// in the following sense:
-//
-// 1. IsRewritableExpression and AsRewritableExpression behave as usual.
-// 2. All other Is* and As* methods are practically delegated to the
-//    wrapped node, i.e. IsArrayLiteral() will return true iff the
-//    wrapped node is an array literal.
-//
-// Furthermore, an invariant that should be respected is that the wrapped
-// node is not a RewritableExpression.
-class RewritableExpression final : public Expression {
- public:
-  Expression* expression() const { return expr_; }
-  bool is_rewritten() const { return IsRewrittenField::decode(bit_field_); }
-  void set_rewritten() {
-    bit_field_ = IsRewrittenField::update(bit_field_, true);
-  }
-
-  void Rewrite(Expression* new_expression) {
-    DCHECK(!is_rewritten());
-    DCHECK_NOT_NULL(new_expression);
-    DCHECK(!new_expression->IsRewritableExpression());
-    expr_ = new_expression;
-    set_rewritten();
-  }
-
-  Scope* scope() const { return scope_; }
-  void set_scope(Scope* scope) { scope_ = scope; }
-
- private:
-  friend class AstNodeFactory;
-
-  RewritableExpression(Expression* expression, Scope* scope)
-      : Expression(expression->position(), kRewritableExpression),
-        expr_(expression),
-        scope_(scope) {
-    bit_field_ |= IsRewrittenField::encode(false);
-    DCHECK(!expression->IsRewritableExpression());
-  }
-
-  Expression* expr_;
-  Scope* scope_;
-
-  class IsRewrittenField
-      : public BitField<bool, Expression::kNextBitFieldIndex, 1> {};
-};
-
 // There are several types of Suspend node:
 //
 // Yield
@@ -2969,6 +2916,7 @@ class AstNodeFactory final {
         empty_statement_(new (zone) class EmptyStatement()),
         failure_expression_(new (zone) class FailureExpression()) {}
 
+  AstNodeFactory* ast_node_factory() { return this; }
   AstValueFactory* ast_value_factory() const { return ast_value_factory_; }
 
   VariableDeclaration* NewVariableDeclaration(VariableProxy* proxy, int pos) {
@@ -3326,12 +3274,6 @@ class AstNodeFactory final {
         Conditional(condition, then_expression, else_expression, position);
   }
 
-  RewritableExpression* NewRewritableExpression(Expression* expression,
-                                                Scope* scope) {
-    DCHECK_NOT_NULL(expression);
-    return new (zone_) RewritableExpression(expression, scope);
-  }
-
   Assignment* NewAssignment(Token::Value op,
                             Expression* target,
                             Expression* value,
@@ -3508,38 +3450,15 @@ class AstNodeFactory final {
 // Inline functions for AstNode.
 
 #define DECLARE_NODE_FUNCTIONS(type)                                         \
-  bool AstNode::Is##type() const {                                           \
-    NodeType mine = node_type();                                             \
-    if (mine == AstNode::kRewritableExpression &&                            \
-        AstNode::k##type == AstNode::kAssignment)                            \
-      mine = reinterpret_cast<const RewritableExpression*>(this)             \
-                 ->expression()                                              \
-                 ->node_type();                                              \
-    return mine == AstNode::k##type;                                         \
-  }                                                                          \
+  bool AstNode::Is##type() const { return node_type() == AstNode::k##type; } \
   type* AstNode::As##type() {                                                \
-    NodeType mine = node_type();                                             \
-    AstNode* result = this;                                                  \
-    if (mine == AstNode::kRewritableExpression &&                            \
-        AstNode::k##type == AstNode::kAssignment) {                          \
-      result =                                                               \
-          reinterpret_cast<const RewritableExpression*>(this)->expression(); \
-      mine = result->node_type();                                            \
-    }                                                                        \
-    return mine == AstNode::k##type ? reinterpret_cast<type*>(result)        \
-                                    : nullptr;                               \
+    return node_type() == AstNode::k##type ? reinterpret_cast<type*>(this)   \
+                                           : nullptr;                        \
   }                                                                          \
   const type* AstNode::As##type() const {                                    \
-    NodeType mine = node_type();                                             \
-    const AstNode* result = this;                                            \
-    if (mine == AstNode::kRewritableExpression &&                            \
-        AstNode::k##type != AstNode::kRewritableExpression) {                \
-      result =                                                               \
-          reinterpret_cast<const RewritableExpression*>(this)->expression(); \
-      mine = result->node_type();                                            \
-    }                                                                        \
-    return mine == AstNode::k##type ? reinterpret_cast<const type*>(result)  \
-                                    : nullptr;                               \
+    return node_type() == AstNode::k##type                                   \
+               ? reinterpret_cast<const type*>(this)                         \
+               : nullptr;                                                    \
   }
 AST_NODE_LIST(DECLARE_NODE_FUNCTIONS)
 FAILURE_NODE_LIST(DECLARE_NODE_FUNCTIONS)

src/ast/prettyprinter.cc

@@ -497,10 +497,6 @@ void CallPrinter::VisitSuperCallReference(SuperCallReference* node) {
 }
 
 
-void CallPrinter::VisitRewritableExpression(RewritableExpression* node) {
-  Find(node->expression());
-}
-
 void CallPrinter::FindStatements(const ZonePtrList<Statement>* statements) {
   if (statements == nullptr) return;
   for (int i = 0; i < statements->length(); i++) {
@@ -742,8 +738,9 @@ void AstPrinter::PrintLiteralWithModeIndented(const char* info, Variable* var,
   } else {
     EmbeddedVector<char, 256> buf;
     int pos =
-        SNPrintF(buf, "%s (%p) (mode = %s", info, reinterpret_cast<void*>(var),
-                 VariableMode2String(var->mode()));
+        SNPrintF(buf, "%s (%p) (mode = %s, assigned = %s", info,
+                 reinterpret_cast<void*>(var), VariableMode2String(var->mode()),
+                 var->maybe_assigned() == kMaybeAssigned ? "true" : "false");
     SNPrintF(buf + pos, ")");
     PrintLiteralIndented(buf.start(), value, true);
   }
@@ -1389,11 +1386,6 @@ void AstPrinter::VisitSuperCallReference(SuperCallReference* node) {
 }
 
 
-void AstPrinter::VisitRewritableExpression(RewritableExpression* node) {
-  Visit(node->expression());
-}
-
-
 #endif  // DEBUG
 
 }  // namespace internal

src/ast/scopes.cc

@@ -770,27 +770,11 @@ Variable* DeclarationScope::DeclareGeneratorObjectVar(
   return result;
 }
 
-bool Scope::HasBeenRemoved() const {
-  if (sibling() == this) {
-    DCHECK_NULL(inner_scope_);
-    DCHECK(is_block_scope());
-    return true;
-  }
-  return false;
-}
-
-Scope* Scope::GetUnremovedScope() {
-  Scope* scope = this;
-  while (scope != nullptr && scope->HasBeenRemoved()) {
-    scope = scope->outer_scope();
-  }
-  DCHECK_NOT_NULL(scope);
-  return scope;
-}
-
 Scope* Scope::FinalizeBlockScope() {
   DCHECK(is_block_scope());
-  DCHECK(!HasBeenRemoved());
+#ifdef DEBUG
+  DCHECK_NE(sibling_, this);
+#endif
 
   if (variables_.occupancy() > 0 ||
       (is_declaration_scope() && AsDeclarationScope()->calls_sloppy_eval())) {
@@ -830,8 +814,9 @@ Scope* Scope::FinalizeBlockScope() {
   num_heap_slots_ = 0;
 
   // Mark scope as removed by making it its own sibling.
+#ifdef DEBUG
   sibling_ = this;
-  DCHECK(HasBeenRemoved());
+#endif
 
   return nullptr;
 }
@@ -986,6 +971,11 @@ Variable* DeclarationScope::DeclareParameter(const AstRawString* name,
   if (name == ast_value_factory->arguments_string()) {
     has_arguments_parameter_ = true;
   }
+  // Params are automatically marked as used to make sure that the debugger and
+  // function.arguments sees them.
+  // TODO(verwaest): Reevaluate whether we always need to do this, since
+  // strict-mode function.arguments does not make the arguments available.
+  var->set_is_used();
   return var;
 }
 
@@ -1010,6 +1000,11 @@ Variable* DeclarationScope::DeclareParameterName(
   if (add_parameter) {
     params_.Add(var, zone());
   }
+  // Params are automatically marked as used to make sure that the debugger and
+  // function.arguments sees them.
+  // TODO(verwaest): Reevaluate whether we always need to do this, since
+  // strict-mode function.arguments does not make the arguments available.
+  var->set_is_used();
   return var;
 }
 

src/ast/scopes.h

@@ -190,11 +190,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // tree and its children are reparented.
   Scope* FinalizeBlockScope();
 
-  bool HasBeenRemoved() const;
-
-  // Find the first scope that hasn't been removed.
-  Scope* GetUnremovedScope();
-
   // Inserts outer_scope into this scope's scope chain (and removes this
   // from the current outer_scope_'s inner scope list).
   // Assumes outer_scope_ is non-null.

src/interpreter/bytecode-generator.cc

@@ -5328,10 +5328,6 @@ void BytecodeGenerator::VisitNaryLogicalAndExpression(NaryOperation* expr) {
   }
 }
 
-void BytecodeGenerator::VisitRewritableExpression(RewritableExpression* expr) {
-  Visit(expr->expression());
-}
-
 void BytecodeGenerator::BuildNewLocalActivationContext() {
   ValueResultScope value_execution_result(this);
   Scope* scope = closure_scope();

src/parsing/expression-scope-reparenter.cc

@@ -27,7 +27,6 @@ class Reparenter final : public AstTraversalVisitor<Reparenter> {
   void VisitFunctionLiteral(FunctionLiteral* expr);
   void VisitClassLiteral(ClassLiteral* expr);
   void VisitVariableProxy(VariableProxy* expr);
-  void VisitRewritableExpression(RewritableExpression* expr);
 
   void VisitBlock(Block* stmt);
   void VisitTryCatchStatement(TryCatchStatement* stmt);
@@ -79,11 +78,6 @@ void Reparenter::VisitVariableProxy(VariableProxy* proxy) {
   }
 }
 
-void Reparenter::VisitRewritableExpression(RewritableExpression* expr) {
-  Visit(expr->expression());
-  expr->set_scope(scope_);
-}
-
 void Reparenter::VisitBlock(Block* stmt) {
   if (stmt->scope())
     stmt->scope()->ReplaceOuterScope(scope_);

src/parsing/parser.h

@@ -151,7 +151,6 @@ struct ParserTypes<Parser> {
   typedef v8::internal::IterationStatement* IterationStatement;
   typedef ObjectLiteral::Property* ObjectLiteralProperty;
   typedef ScopedPtrList<v8::internal::ObjectLiteralProperty> ObjectPropertyList;
-  typedef v8::internal::RewritableExpression* RewritableExpression;
   typedef v8::internal::Statement* Statement;
   typedef ScopedPtrList<v8::internal::Statement> StatementList;
   typedef v8::internal::Suspend* Suspend;
@@ -370,12 +369,10 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   // PatternRewriter and associated methods defined in pattern-rewriter.cc.
   friend class PatternRewriter;
-  void DeclareAndInitializeVariables(
+  void InitializeVariables(
       Block* block, const DeclarationDescriptor* declaration_descriptor,
       const DeclarationParsingResult::Declaration* declaration,
       ZonePtrList<const AstRawString>* names);
-  void RewriteDestructuringAssignment(RewritableExpression* expr);
-  Expression* RewriteDestructuringAssignment(Assignment* assignment);
 
   // [if (IteratorType == kAsync)]
   //     !%_IsJSReceiver(result = Await(next.[[Call]](iterator, « »)) &&
@@ -548,14 +545,8 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   void SetLanguageMode(Scope* scope, LanguageMode mode);
   void SetAsmModule();
 
-  // Rewrite all DestructuringAssignments in the current FunctionState.
-  void RewriteDestructuringAssignments();
-
   Expression* RewriteSpreads(ArrayLiteral* lit);
 
-  void QueueDestructuringAssignmentForRewriting(
-      RewritableExpression* assignment);
-
   friend class InitializerRewriter;
   void RewriteParameterInitializer(Expression* expr);
 
@@ -717,17 +708,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
     }
   }
 
-  V8_INLINE static void MarkPatternAsAssigned(Expression* expression) {}
-
-  // Determine if the expression is a variable proxy and mark it as being used
-  // in an assignment or with a increment/decrement operator.
-  V8_INLINE static void MarkExpressionAsAssigned(Expression* expression) {
-    DCHECK_NOT_NULL(expression);
-    if (expression->IsVariableProxy()) {
-      expression->AsVariableProxy()->set_is_assigned();
-    }
-  }
-
   // A shortcut for performing a ToString operation
   V8_INLINE Expression* ToString(Expression* expr) {
     if (expr->IsStringLiteral()) return expr;
@@ -877,7 +857,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
     if (infer == InferName::kYes) {
       fni_.PushVariableName(name);
     }
-    return NewUnresolved(name, start_position);
+    return expression_scope()->NewVariable(name, start_position);
   }
 
   V8_INLINE Variable* DeclareCatchVariableName(Scope* scope,

src/parsing/pattern-rewriter.cc

@@ -25,15 +25,12 @@ class PatternRewriter final : public AstVisitor<PatternRewriter> {
 
   typedef Parser::DeclarationDescriptor DeclarationDescriptor;
 
-  static void DeclareAndInitializeVariables(
+  static void InitializeVariables(
       Parser* parser, Block* block,
       const DeclarationDescriptor* declaration_descriptor,
       const Parser::DeclarationParsingResult::Declaration* declaration,
       ZonePtrList<const AstRawString>* names);
 
-  static Expression* RewriteDestructuringAssignment(Parser* parser,
-                                                    Assignment* to_rewrite);
-
  private:
   enum PatternContext : uint8_t { BINDING, ASSIGNMENT };
 
@@ -128,51 +125,16 @@ class PatternRewriter final : public AstVisitor<PatternRewriter> {
   DEFINE_AST_VISITOR_MEMBERS_WITHOUT_STACKOVERFLOW()
 };
 
-void Parser::DeclareAndInitializeVariables(
+void Parser::InitializeVariables(
     Block* block, const DeclarationDescriptor* declaration_descriptor,
     const DeclarationParsingResult::Declaration* declaration,
     ZonePtrList<const AstRawString>* names) {
   if (has_error()) return;
-  PatternRewriter::DeclareAndInitializeVariables(
-      this, block, declaration_descriptor, declaration, names);
-}
-
-namespace {
-// Lightweight visitor for the case where bytecode does the desugaring.
-void MarkVariablesWritten(Expression* expr) {
-  if (expr->IsVariableProxy()) {
-    expr->AsVariableProxy()->set_is_assigned();
-  } else if (expr->IsObjectLiteral()) {
-    for (ObjectLiteralProperty* prop : *expr->AsObjectLiteral()->properties()) {
-      MarkVariablesWritten(prop->value());
-    }
-  } else if (expr->IsArrayLiteral()) {
-    for (Expression* value : *expr->AsArrayLiteral()->values()) {
-      MarkVariablesWritten(value);
-    }
-  } else if (expr->IsSpread()) {
-    MarkVariablesWritten(expr->AsSpread()->expression());
-  } else if (expr->IsAssignment()) {
-    MarkVariablesWritten(expr->AsAssignment()->target());
-  }
-}
-}  // namespace
-
-void Parser::RewriteDestructuringAssignment(RewritableExpression* to_rewrite) {
-  DCHECK(!to_rewrite->is_rewritten());
-  Assignment* assignment = to_rewrite->expression()->AsAssignment();
-  DCHECK_NOT_NULL(assignment);
-  MarkVariablesWritten(assignment->target());
+  PatternRewriter::InitializeVariables(this, block, declaration_descriptor,
+                                       declaration, names);
 }
 
-Expression* Parser::RewriteDestructuringAssignment(Assignment* assignment) {
-  DCHECK_NOT_NULL(assignment);
-  DCHECK_EQ(Token::ASSIGN, assignment->op());
-  MarkVariablesWritten(assignment->target());
-  return assignment;
-}
-
-void PatternRewriter::DeclareAndInitializeVariables(
+void PatternRewriter::InitializeVariables(
     Parser* parser, Block* block,
     const DeclarationDescriptor* declaration_descriptor,
     const Parser::DeclarationParsingResult::Declaration* declaration,
@@ -191,14 +153,6 @@ void PatternRewriter::DeclareAndInitializeVariables(
                                  declaration->initializer);
 }
 
-Expression* PatternRewriter::RewriteDestructuringAssignment(
-    Parser* parser, Assignment* to_rewrite) {
-  DCHECK(!parser->scope()->HasBeenRemoved());
-
-  PatternRewriter rewriter(parser, ASSIGNMENT);
-  return rewriter.Rewrite(to_rewrite);
-}
-
 void PatternRewriter::VisitVariableProxy(VariableProxy* proxy) {
   Expression* value = current_value_;
 
@@ -219,7 +173,6 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* proxy) {
   if (declares_parameter_containing_sloppy_eval_) {
     target_scope = target_scope->outer_scope();
   }
-  target_scope->DeleteUnresolved(proxy);
 
   // Declare variable.
   // Note that we *always* must treat the initial value via a separate init
@@ -312,16 +265,6 @@ Variable* PatternRewriter::CreateTempVar(Expression* value) {
   return temp;
 }
 
-void PatternRewriter::VisitRewritableExpression(RewritableExpression* node) {
-  DCHECK(node->expression()->IsAssignment());
-  // This is not a top-level destructuring assignment. Mark the node as
-  // rewritten to prevent redundant rewriting and visit the underlying
-  // expression.
-  DCHECK(!node->is_rewritten());
-  node->set_rewritten();
-  return Visit(node->expression());
-}
-
 // 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

src/parsing/preparser.cc

@@ -142,8 +142,7 @@ PreParser::PreParseResult PreParser::PreParseFunction(
 
     // Parse non-arrow function parameters. For arrow functions, the parameters
     // have already been parsed.
-    DeclarationParsingScope formals_scope(
-        this, ExpressionScope::kParameterDeclaration);
+    ParameterDeclarationParsingScope formals_scope(this);
     // We return kPreParseSuccess in failure cases too - errors are retrieved
     // separately by Parser::SkipLazyFunctionBody.
     ParseFormalParameterList(&formals);
@@ -302,8 +301,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
     function_scope->set_start_position(start_position);
     PreParserFormalParameters formals(function_scope);
     {
-      DeclarationParsingScope formals_scope(
-          this, ExpressionScope::kParameterDeclaration);
+      ParameterDeclarationParsingScope formals_scope(this);
       ParseFormalParameterList(&formals);
     }
     Expect(Token::RPAREN);
@@ -410,22 +408,20 @@ bool PreParser::IdentifierEquals(const PreParserIdentifier& identifier,
 
 PreParserExpression PreParser::ExpressionFromIdentifier(
     const PreParserIdentifier& name, int start_position, InferName infer) {
-  VariableProxy* proxy = nullptr;
   DCHECK_IMPLIES(name.string_ == nullptr, has_error());
   if (name.string_ == nullptr) return PreParserExpression::Default();
-  proxy = scope()->NewUnresolved(factory()->ast_node_factory(), name.string_,
-                                 start_position, NORMAL_VARIABLE);
+  VariableProxy* proxy =
+      expression_scope()->NewVariable(name.string_, start_position);
   return PreParserExpression::FromIdentifier(name, proxy, zone());
 }
 
-void PreParser::DeclareAndInitializeVariables(
+void PreParser::InitializeVariables(
     PreParserStatement block,
     const DeclarationDescriptor* declaration_descriptor,
     const DeclarationParsingResult::Declaration* declaration,
     ZonePtrList<const AstRawString>* names) {
   if (declaration->pattern.variables_ != nullptr) {
     for (auto variable : *(declaration->pattern.variables_)) {
-      scope()->DeleteUnresolved(variable);
       Variable* var = scope()->DeclareVariableName(
           variable->raw_name(), declaration_descriptor->mode);
       if (var == nullptr) {

src/parsing/preparser.h

@@ -571,15 +571,7 @@ class PreParserFactory {
 
   PreParserExpression NewStringLiteral(const PreParserIdentifier& identifier,
                                        int pos) {
-    // This is needed for object literal property names. Property names are
-    // normalized to string literals during object literal parsing.
-    PreParserExpression expression = PreParserExpression::Default();
-    if (identifier.string_ != nullptr) {
-      VariableProxy* variable = ast_node_factory_.NewVariableProxy(
-          identifier.string_, NORMAL_VARIABLE);
-      expression.AddVariable(variable, zone_);
-    }
-    return expression;
+    return PreParserExpression::Default();
   }
   PreParserExpression NewNumberLiteral(double number,
                                        int pos) {
@@ -658,10 +650,6 @@ class PreParserFactory {
                                           int pos) {
     return PreParserExpression::Default();
   }
-  PreParserExpression NewRewritableExpression(
-      const PreParserExpression& expression, Scope* scope) {
-    return expression;
-  }
   PreParserExpression NewAssignment(Token::Value op,
                                     const PreParserExpression& left,
                                     const PreParserExpression& right, int pos) {
@@ -943,7 +931,6 @@ struct ParserTypes<PreParser> {
   typedef PreParserExpression Expression;
   typedef PreParserExpression FunctionLiteral;
   typedef PreParserExpression ObjectLiteralProperty;
-  typedef PreParserExpression RewritableExpression;
   typedef PreParserExpression Suspend;
   typedef PreParserExpressionList ExpressionList;
   typedef PreParserExpressionList ObjectPropertyList;
@@ -1116,14 +1103,12 @@ class PreParser : public ParserBase<PreParser> {
   SpreadCallNew(const PreParserExpression& function,
                 const PreParserExpressionList& args, int pos);
 
-  V8_INLINE void RewriteDestructuringAssignments() {}
-
   V8_INLINE void PrepareGeneratorVariables() {}
   V8_INLINE void RewriteAsyncFunctionBody(
       const PreParserScopedStatementList* body, PreParserStatement block,
       const PreParserExpression& return_value) {}
 
-  void DeclareAndInitializeVariables(
+  void InitializeVariables(
       PreParserStatement block,
       const DeclarationDescriptor* declaration_descriptor,
       const DeclarationParsingResult::Declaration* declaration,
@@ -1401,25 +1386,6 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE static void CheckAssigningFunctionLiteralToProperty(
       const PreParserExpression& left, const PreParserExpression& right) {}
 
-  V8_INLINE void MarkPatternAsAssigned(const PreParserExpression& expression) {
-    // TODO(marja): To be able to produce the same errors, the preparser needs
-    // to start tracking which expressions are variables and which are assigned.
-    if (expression.variables_ != nullptr) {
-      for (auto variable : *expression.variables_) {
-        variable->set_is_assigned();
-      }
-    }
-  }
-
-  V8_INLINE void MarkExpressionAsAssigned(
-      const PreParserExpression& expression) {
-    if (IsIdentifier(expression)) {
-      DCHECK_NOT_NULL(expression.variables_);
-      DCHECK_EQ(1, expression.variables_->LengthForTest());
-      expression.variables_->first()->set_is_assigned();
-    }
-  }
-
   V8_INLINE bool ShortcutNumericLiteralBinaryExpression(
       PreParserExpression* x, const PreParserExpression& y, Token::Value op,
       int pos) {
@@ -1442,9 +1408,8 @@ class PreParser : public ParserBase<PreParser> {
   BuildInitializationBlock(DeclarationParsingResult* parsing_result,
                            ZonePtrList<const AstRawString>* names) {
     for (auto declaration : parsing_result->declarations) {
-      DeclareAndInitializeVariables(PreParserStatement::Default(),
-                                    &(parsing_result->descriptor), &declaration,
-                                    names);
+      InitializeVariables(PreParserStatement::Default(),
+                          &(parsing_result->descriptor), &declaration, names);
     }
     return PreParserStatement::Default();
   }
@@ -1452,7 +1417,6 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE PreParserStatement InitializeForEachStatement(
       PreParserStatement stmt, const PreParserExpression& each,
       const PreParserExpression& subject, PreParserStatement body) {
-    MarkPatternAsAssigned(each);
     return stmt;
   }
 
@@ -1461,7 +1425,6 @@ class PreParser : public ParserBase<PreParser> {
       const PreParserExpression& iterable, PreParserStatement body,
       bool finalize, IteratorType type,
       int next_result_pos = kNoSourcePosition) {
-    MarkPatternAsAssigned(each);
     return stmt;
   }
 
@@ -1480,10 +1443,10 @@ class PreParser : public ParserBase<PreParser> {
         IsLexicalVariableMode(for_info->parsing_result.descriptor.mode) ||
         is_for_var_of;
 
-    DeclareAndInitializeVariables(
-        PreParserStatement::Default(), &for_info->parsing_result.descriptor,
-        &for_info->parsing_result.declarations[0],
-        collect_names ? &for_info->bound_names : nullptr);
+    InitializeVariables(PreParserStatement::Default(),
+                        &for_info->parsing_result.descriptor,
+                        &for_info->parsing_result.declarations[0],
+                        collect_names ? &for_info->bound_names : nullptr);
   }
 
   V8_INLINE PreParserBlock CreateForEachStatementTDZ(PreParserBlock init_block,

src/zone/zone.h

@@ -345,6 +345,7 @@ class ScopedPtrList final {
   int length() const { return static_cast<int>(end_ - start_); }
   T* at(int i) const {
     size_t index = start_ + i;
+    DCHECK_LE(start_, index);
     DCHECK_LT(index, buffer_.size());
     return reinterpret_cast<T*>(buffer_[index]);
   }