[parser] Declare variables through ExpressionScope

Use variable tracking from ExpressionScopes rather than the PatternRewriter and
PreParserExpression::variables_ to declare variables.

We only figure out that variables are non-simple parameters once we see the
first non-simple parameter. This still uses the pattern rewriter to make
variables non-simple (kLet instead of kVar).

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

src/ast/ast.h

@@ -1533,6 +1533,10 @@ class VariableProxy final : public Expression {
     var_ = v;
   }
 
+  Scanner::Location location() {
+    return Scanner::Location(position(), position() + raw_name()->length());
+  }
+
   bool is_this() const { return IsThisField::decode(bit_field_); }
 
   bool is_assigned() const { return IsAssignedField::decode(bit_field_); }

src/ast/scopes.cc

@@ -40,19 +40,19 @@ Variable* VariableMap::Declare(Zone* zone, Scope* scope,
                                VariableKind kind,
                                InitializationFlag initialization_flag,
                                MaybeAssignedFlag maybe_assigned_flag,
-                               base::ThreadedList<Variable>* variable_list) {
+                               bool* was_added) {
   // AstRawStrings are unambiguous, i.e., the same string is always represented
   // by the same AstRawString*.
   // FIXME(marja): fix the type of Lookup.
   Entry* p =
       ZoneHashMap::LookupOrInsert(const_cast<AstRawString*>(name), name->Hash(),
                                   ZoneAllocationPolicy(zone));
-  if (p->value == nullptr) {
+  *was_added = p->value == nullptr;
+  if (*was_added) {
     // The variable has not been declared yet -> insert it.
     DCHECK_EQ(name, p->key);
     Variable* variable = new (zone) Variable(
         scope, name, mode, kind, initialization_flag, maybe_assigned_flag);
-    if (variable_list) variable_list->Add(variable);
     p->value = variable;
   }
   return reinterpret_cast<Variable*>(p->value);
@@ -247,9 +247,11 @@ Scope::Scope(Zone* zone, const AstRawString* catch_variable_name,
   // Cache the catch variable, even though it's also available via the
   // scope_info, as the parser expects that a catch scope always has the catch
   // variable as first and only variable.
+  bool was_added;
   Variable* variable =
       Declare(zone, catch_variable_name, VariableMode::kVar, NORMAL_VARIABLE,
-              kCreatedInitialized, maybe_assigned);
+              kCreatedInitialized, maybe_assigned, &was_added);
+  DCHECK(was_added);
   AllocateHeapSlot(variable);
 }
 
@@ -562,16 +564,18 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
       DCHECK(!is_being_lazily_parsed_);
       VariableProxy* proxy = factory->NewVariableProxy(name, NORMAL_VARIABLE);
       auto declaration = factory->NewVariableDeclaration(kNoSourcePosition);
+      bool was_added;
       // Based on the preceding checks, it doesn't matter what we pass as
       // sloppy_mode_block_scope_function_redefinition.
       bool ok = true;
       DeclareVariable(declaration, proxy, VariableMode::kVar, NORMAL_VARIABLE,
                       Variable::DefaultInitializationFlag(VariableMode::kVar),
-                      nullptr, &ok);
+                      &was_added, nullptr, &ok);
       DCHECK(ok);
     } else {
       DCHECK(is_being_lazily_parsed_);
-      Variable* var = DeclareVariableName(name, VariableMode::kVar);
+      bool was_added;
+      Variable* var = DeclareVariableName(name, VariableMode::kVar, &was_added);
       var->set_maybe_assigned();
     }
   }
@@ -634,11 +638,14 @@ void DeclarationScope::DeclareThis(AstValueFactory* ast_value_factory) {
   DCHECK(has_this_declaration());
 
   bool derived_constructor = IsDerivedConstructor(function_kind_);
+  bool was_added;
   Variable* var =
       Declare(zone(), ast_value_factory->this_string(),
               derived_constructor ? VariableMode::kConst : VariableMode::kVar,
               THIS_VARIABLE,
-              derived_constructor ? kNeedsInitialization : kCreatedInitialized);
+              derived_constructor ? kNeedsInitialization : kCreatedInitialized,
+              kNotAssigned, &was_added);
+  DCHECK(was_added);
   receiver_ = var;
 }
 
@@ -646,14 +653,14 @@ void DeclarationScope::DeclareArguments(AstValueFactory* ast_value_factory) {
   DCHECK(is_function_scope());
   DCHECK(!is_arrow_scope());
 
-  arguments_ = LookupLocal(ast_value_factory->arguments_string());
-  if (arguments_ == nullptr) {
-    // Declare 'arguments' variable which exists in all non arrow functions.
-    // Note that it might never be accessed, in which case it won't be
-    // allocated during variable allocation.
-    arguments_ = Declare(zone(), ast_value_factory->arguments_string(),
-                         VariableMode::kVar);
-  } else if (IsLexicalVariableMode(arguments_->mode())) {
+  // Declare 'arguments' variable which exists in all non arrow functions.  Note
+  // that it might never be accessed, in which case it won't be allocated during
+  // variable allocation.
+  bool was_added;
+  arguments_ =
+      Declare(zone(), ast_value_factory->arguments_string(), VariableMode::kVar,
+              NORMAL_VARIABLE, kCreatedInitialized, kNotAssigned, &was_added);
+  if (!was_added && IsLexicalVariableMode(arguments_->mode())) {
     // Check if there's lexically declared variable named arguments to avoid
     // redeclaration. See ES#sec-functiondeclarationinstantiation, step 20.
     arguments_ = nullptr;
@@ -666,14 +673,18 @@ void DeclarationScope::DeclareDefaultFunctionVariables(
   DCHECK(!is_arrow_scope());
 
   DeclareThis(ast_value_factory);
+  bool was_added;
   new_target_ = Declare(zone(), ast_value_factory->new_target_string(),
-                        VariableMode::kConst);
+                        VariableMode::kConst, NORMAL_VARIABLE,
+                        kCreatedInitialized, kNotAssigned, &was_added);
+  DCHECK(was_added);
 
   if (IsConciseMethod(function_kind_) || IsClassConstructor(function_kind_) ||
       IsAccessorFunction(function_kind_)) {
-    EnsureRareData()->this_function =
-        Declare(zone(), ast_value_factory->this_function_string(),
-                VariableMode::kConst);
+    EnsureRareData()->this_function = Declare(
+        zone(), ast_value_factory->this_function_string(), VariableMode::kConst,
+        NORMAL_VARIABLE, kCreatedInitialized, kNotAssigned, &was_added);
+    DCHECK(was_added);
   }
 }
 
@@ -771,7 +782,6 @@ void Scope::Snapshot::Reparent(DeclarationScope* new_parent) {
   DCHECK_EQ(new_parent, new_parent->GetClosureScope());
   DCHECK_NULL(new_parent->inner_scope_);
   DCHECK(new_parent->unresolved_list_.is_empty());
-  DCHECK(new_parent->locals_.is_empty());
   Scope* inner_scope = new_parent->sibling_;
   if (inner_scope != top_inner_scope_) {
     for (; inner_scope->sibling() != top_inner_scope_;
@@ -799,13 +809,14 @@ void Scope::Snapshot::Reparent(DeclarationScope* new_parent) {
 
   // Move temporaries allocated for complex parameter initializers.
   DeclarationScope* outer_closure = outer_scope_->GetClosureScope();
-  new_parent->locals_.MoveTail(outer_closure->locals(), top_local_);
-  for (Variable* local : new_parent->locals_) {
+  for (auto it = top_local_; it != outer_closure->locals()->end(); ++it) {
+    Variable* local = *it;
     DCHECK_EQ(VariableMode::kTemporary, local->mode());
     DCHECK_EQ(local->scope(), local->scope()->GetClosureScope());
     DCHECK_NE(local->scope(), new_parent);
     local->set_scope(new_parent);
   }
+  new_parent->locals_.MoveTail(outer_closure->locals(), top_local_);
   outer_closure->locals_.Rewind(top_local_);
 
   // Move eval calls since Snapshot's creation into new_parent.
@@ -877,8 +888,11 @@ Variable* Scope::LookupInScopeInfo(const AstRawString* name, Scope* cache) {
   // TODO(marja, rossberg): Correctly declare FUNCTION, CLASS, NEW_TARGET, and
   // ARGUMENTS bindings as their corresponding VariableKind.
 
-  Variable* var = cache->variables_.Declare(zone(), this, name, mode, kind,
-                                            init_flag, maybe_assigned_flag);
+  bool was_added;
+  Variable* var =
+      cache->variables_.Declare(zone(), this, name, mode, kind, init_flag,
+                                maybe_assigned_flag, &was_added);
+  DCHECK(was_added);
   var->AllocateTo(location, index);
   return var;
 }
@@ -898,8 +912,9 @@ Variable* DeclarationScope::DeclareParameter(const AstRawString* name,
   if (mode == VariableMode::kTemporary) {
     var = NewTemporary(name);
   } else {
+    var = LookupLocal(name);
     DCHECK_EQ(mode, VariableMode::kVar);
-    var = Declare(zone(), name, mode, PARAMETER_VARIABLE);
+    DCHECK(var->is_parameter());
   }
   has_rest_ = is_rest;
   var->set_initializer_position(position);
@@ -925,22 +940,9 @@ void DeclarationScope::RecordParameter(bool is_rest) {
   if (!is_rest) ++num_parameters_;
 }
 
-void DeclarationScope::DeclareParameterName(const AstRawString* name) {
-  DCHECK(!already_resolved_);
-  DCHECK(is_function_scope() || is_module_scope());
-  DCHECK(is_being_lazily_parsed_);
-  // The resulting variable isn't added to params. In the case of non-simple
-  // params, a dummy temp variable is added in AddNonSimpleParameterTemp.
-  Variable* var = Declare(zone(), name, VariableMode::kVar, PARAMETER_VARIABLE);
-  // 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();
-}
-
 Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
-                              VariableKind kind, InitializationFlag init_flag) {
+                              VariableKind kind, bool* was_added,
+                              InitializationFlag init_flag) {
   DCHECK(!already_resolved_);
   // This function handles VariableMode::kVar, VariableMode::kLet, and
   // VariableMode::kConst modes.  VariableMode::kDynamic variables are
@@ -951,16 +953,15 @@ Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
                  mode == VariableMode::kVar || mode == VariableMode::kLet ||
                      mode == VariableMode::kConst);
   DCHECK(!GetDeclarationScope()->was_lazily_parsed());
-  return Declare(zone(), name, mode, kind, init_flag);
+  return Declare(zone(), name, mode, kind, init_flag, kNotAssigned, was_added);
 }
 
 // TODO(leszeks): Avoid passing the proxy into here, passing the raw_name alone
 // instead.
-void Scope::DeclareVariable(Declaration* declaration, VariableProxy* proxy,
-                            VariableMode mode, VariableKind kind,
-                            InitializationFlag init,
-                            bool* sloppy_mode_block_scope_function_redefinition,
-                            bool* ok) {
+Variable* Scope::DeclareVariable(
+    Declaration* declaration, VariableProxy* proxy, VariableMode mode,
+    VariableKind kind, InitializationFlag init, bool* was_added,
+    bool* sloppy_mode_block_scope_function_redefinition, bool* ok) {
   DCHECK(IsDeclaredVariableMode(mode));
   DCHECK(!already_resolved_);
   DCHECK(!GetDeclarationScope()->is_being_lazily_parsed());
@@ -968,7 +969,7 @@ void Scope::DeclareVariable(Declaration* declaration, VariableProxy* proxy,
 
   if (mode == VariableMode::kVar && !is_declaration_scope()) {
     return GetDeclarationScope()->DeclareVariable(
-        declaration, proxy, mode, kind, init,
+        declaration, proxy, mode, kind, init, was_added,
         sloppy_mode_block_scope_function_redefinition, ok);
   }
   DCHECK(!is_catch_scope());
@@ -992,7 +993,8 @@ void Scope::DeclareVariable(Declaration* declaration, VariableProxy* proxy,
 
   Variable* var = LookupLocal(name);
   // Declare the variable in the declaration scope.
-  if (V8_LIKELY(var == nullptr)) {
+  *was_added = var == nullptr;
+  if (V8_LIKELY(*was_added)) {
     if (V8_UNLIKELY(is_eval_scope() && is_sloppy(language_mode()) &&
                     mode == VariableMode::kVar)) {
       // In a var binding in a sloppy direct eval, pollute the enclosing scope
@@ -1003,7 +1005,8 @@ void Scope::DeclareVariable(Declaration* declaration, VariableProxy* proxy,
       var = NonLocal(proxy->raw_name(), VariableMode::kDynamic);
     } else {
       // Declare the name.
-      var = DeclareLocal(name, mode, kind, init);
+      var = DeclareLocal(name, mode, kind, was_added, init);
+      DCHECK(*was_added);
     }
   } else {
     var->set_maybe_assigned();
@@ -1053,16 +1056,19 @@ void Scope::DeclareVariable(Declaration* declaration, VariableProxy* proxy,
   decls_.Add(declaration);
   declaration->set_var(var);
   proxy->BindTo(var);
+  return var;
 }
 
 Variable* Scope::DeclareVariableName(const AstRawString* name,
-                                     VariableMode mode) {
+                                     VariableMode mode, bool* was_added,
+                                     VariableKind kind) {
   DCHECK(IsDeclaredVariableMode(mode));
   DCHECK(!already_resolved_);
   DCHECK(GetDeclarationScope()->is_being_lazily_parsed());
 
   if (mode == VariableMode::kVar && !is_declaration_scope()) {
-    return GetDeclarationScope()->DeclareVariableName(name, mode);
+    return GetDeclarationScope()->DeclareVariableName(name, mode, was_added,
+                                                      kind);
   }
   DCHECK(!is_with_scope());
   DCHECK(!is_eval_scope());
@@ -1070,17 +1076,15 @@ Variable* Scope::DeclareVariableName(const AstRawString* name,
   DCHECK(scope_info_.is_null());
 
   // Declare the variable in the declaration scope.
-  Variable* var = LookupLocal(name);
-  if (var == nullptr) {
-    var = DeclareLocal(name, mode);
-  } else if (IsLexicalVariableMode(mode) ||
-             IsLexicalVariableMode(var->mode())) {
+  Variable* var = DeclareLocal(name, mode, kind, was_added);
+  if (!*was_added) {
+    if (IsLexicalVariableMode(mode) || IsLexicalVariableMode(var->mode())) {
       // Duplicate functions are allowed in the sloppy mode, but if this is not
       // a function declaration, it's an error. This is an error PreParser
       // hasn't previously detected.
       return nullptr;
-  } else if (mode == VariableMode::kVar) {
-    var->set_maybe_assigned();
+    }
+    if (mode == VariableMode::kVar) var->set_maybe_assigned();
   }
   var->set_is_used();
   return var;
@@ -1091,7 +1095,11 @@ Variable* Scope::DeclareCatchVariableName(const AstRawString* name) {
   DCHECK(is_catch_scope());
   DCHECK(scope_info_.is_null());
 
-  return Declare(zone(), name, VariableMode::kVar);
+  bool was_added;
+  Variable* result = Declare(zone(), name, VariableMode::kVar, NORMAL_VARIABLE,
+                             kCreatedInitialized, kNotAssigned, &was_added);
+  DCHECK(was_added);
+  return result;
 }
 
 void Scope::AddUnresolved(VariableProxy* proxy) {
@@ -1104,8 +1112,10 @@ Variable* DeclarationScope::DeclareDynamicGlobal(const AstRawString* name,
                                                  VariableKind kind,
                                                  Scope* cache) {
   DCHECK(is_script_scope());
-  return cache->variables_.Declare(zone(), this, name,
-                                   VariableMode::kDynamicGlobal, kind);
+  bool was_added;
+  return cache->variables_.Declare(
+      zone(), this, name, VariableMode::kDynamicGlobal, kind,
+      kCreatedInitialized, kNotAssigned, &was_added);
   // TODO(neis): Mark variable as maybe-assigned?
 }
 
@@ -1702,7 +1712,10 @@ void Scope::CheckZones() {
 Variable* Scope::NonLocal(const AstRawString* name, VariableMode mode) {
   // Declare a new non-local.
   DCHECK(IsDynamicVariableMode(mode));
-  Variable* var = variables_.Declare(zone(), this, name, mode);
+  bool was_added;
+  Variable* var =
+      variables_.Declare(zone(), this, name, mode, NORMAL_VARIABLE,
+                         kCreatedInitialized, kNotAssigned, &was_added);
   // Allocate it by giving it a dynamic lookup.
   var->AllocateTo(VariableLocation::LOOKUP, -1);
   return var;

src/ast/scopes.h

@@ -33,12 +33,10 @@ class VariableMap: public ZoneHashMap {
  public:
   explicit VariableMap(Zone* zone);
 
-  Variable* Declare(
-      Zone* zone, Scope* scope, const AstRawString* name, VariableMode mode,
-      VariableKind kind = NORMAL_VARIABLE,
-      InitializationFlag initialization_flag = kCreatedInitialized,
-      MaybeAssignedFlag maybe_assigned_flag = kNotAssigned,
-      base::ThreadedList<Variable>* variable_list = nullptr);
+  Variable* Declare(Zone* zone, Scope* scope, const AstRawString* name,
+                    VariableMode mode, VariableKind kind,
+                    InitializationFlag initialization_flag,
+                    MaybeAssignedFlag maybe_assigned_flag, bool* was_added);
 
   Variable* Lookup(const AstRawString* name);
   void Remove(Variable* var);
@@ -224,17 +222,19 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // Declare a local variable in this scope. If the variable has been
   // declared before, the previously declared variable is returned.
   Variable* DeclareLocal(const AstRawString* name, VariableMode mode,
-                         VariableKind kind = NORMAL_VARIABLE,
+                         VariableKind kind, bool* was_added,
                          InitializationFlag init_flag = kCreatedInitialized);
 
-  void DeclareVariable(Declaration* declaration, VariableProxy* proxy,
+  Variable* DeclareVariable(Declaration* declaration, VariableProxy* proxy,
                             VariableMode mode, VariableKind kind,
-                       InitializationFlag init,
+                            InitializationFlag init, bool* was_added,
                             bool* sloppy_mode_block_scope_function_redefinition,
                             bool* ok);
 
-  // The return value is meaningful only if FLAG_preparser_scope_analysis is on.
-  Variable* DeclareVariableName(const AstRawString* name, VariableMode mode);
+  // Returns nullptr if there was a declaration conflict.
+  Variable* DeclareVariableName(const AstRawString* name, VariableMode mode,
+                                bool* was_added,
+                                VariableKind kind = NORMAL_VARIABLE);
   Variable* DeclareCatchVariableName(const AstRawString* name);
 
   // Declarations list.
@@ -552,13 +552,14 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   }
 
  private:
-  Variable* Declare(
-      Zone* zone, const AstRawString* name, VariableMode mode,
-      VariableKind kind = NORMAL_VARIABLE,
-      InitializationFlag initialization_flag = kCreatedInitialized,
-      MaybeAssignedFlag maybe_assigned_flag = kNotAssigned) {
-    return variables_.Declare(zone, this, name, mode, kind, initialization_flag,
-                              maybe_assigned_flag, &locals_);
+  Variable* Declare(Zone* zone, const AstRawString* name, VariableMode mode,
+                    VariableKind kind, InitializationFlag initialization_flag,
+                    MaybeAssignedFlag maybe_assigned_flag, bool* was_added) {
+    Variable* result =
+        variables_.Declare(zone, this, name, mode, kind, initialization_flag,
+                           maybe_assigned_flag, was_added);
+    if (*was_added) locals_.Add(result);
+    return result;
   }
 
   // This method should only be invoked on scopes created during parsing (i.e.,
@@ -817,8 +818,6 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
                              bool is_optional, bool is_rest,
                              AstValueFactory* ast_value_factory, int position);
 
-  // Declares that a parameter with the name exists. Creates a Variable.
-  void DeclareParameterName(const AstRawString* name);
   // Makes sure that num_parameters_ and has_rest is correct for the preparser.
   void RecordParameter(bool is_rest);
 

src/ast/variables.h

@@ -176,6 +176,13 @@ class Variable final : public ZoneObject {
     index_ = index;
   }
 
+  void MakeNonSimpleParameter() {
+    DCHECK(is_parameter());
+    bit_field_ = VariableModeField::update(bit_field_, VariableMode::kLet);
+    bit_field_ =
+        InitializationFlagField::update(bit_field_, kNeedsInitialization);
+  }
+
   static InitializationFlag DefaultInitializationFlag(VariableMode mode) {
     DCHECK(IsDeclaredVariableMode(mode));
     return mode == VariableMode::kVar ? kCreatedInitialized

src/parsing/expression-scope.h

@@ -19,6 +19,10 @@ template <typename Types>
 class AccumulationScope;
 template <typename Types>
 class ArrowHeadParsingScope;
+template <typename Types>
+class ParameterDeclarationParsingScope;
+template <typename Types>
+class VariableDeclarationParsingScope;
 class VariableProxy;
 
 // ExpressionScope is used in a stack fashion, and is used to specialize
@@ -44,25 +48,10 @@ class ExpressionScope {
     VariableProxy* result = parser_->NewRawVariable(name, pos);
     if (CanBeExpression()) {
       AsExpressionParsingScope()->TrackVariable(result);
-    } else if (type_ == kVarDeclaration && parser_->loop_nesting_depth() > 0) {
-      // Due to hoisting, the value of a 'var'-declared variable may actually
-      // change even if the code contains only the "initial" assignment, namely
-      // when that assignment occurs inside a loop.  For example:
-      //
-      //   let i = 10;
-      //   do { var x = i } while (i--):
-      //
-      // Note that non-lexical variables include temporaries, which may also get
-      // assigned inside a loop due to the various rewritings that the parser
-      // performs.
-      //
-      // Pessimistically mark all vars in loops as assigned. This
-      // overapproximates the actual assigned vars due to unassigned var without
-      // initializer, but that's unlikely anyway.
-      //
-      // This also handles marking of loop variables in for-in and for-of loops,
-      // as determined by loop-nesting-depth.
-      result->set_is_assigned();
+    } else if (type_ == kParameterDeclaration) {
+      AsParameterDeclarationParsingScope()->Declare(result);
+    } else {
+      return AsVariableDeclarationParsingScope()->Declare(result);
     }
     return result;
   }
@@ -220,6 +209,7 @@ class ExpressionScope {
   bool IsVariableDeclaration() const {
     return IsInRange(type_, kVarDeclaration, kLexicalDeclaration);
   }
+  bool IsLexicalDeclaration() const { return type_ == kLexicalDeclaration; }
   bool IsAsyncArrowHeadParsingScope() const {
     return type_ == kMaybeAsyncArrowParameterDeclaration;
   }
@@ -233,6 +223,17 @@ class ExpressionScope {
     return static_cast<ArrowHeadParsingScope<Types>*>(this);
   }
 
+  ParameterDeclarationParsingScope<Types>*
+  AsParameterDeclarationParsingScope() {
+    DCHECK(IsCertainlyParameterDeclaration());
+    return static_cast<ParameterDeclarationParsingScope<Types>*>(this);
+  }
+
+  VariableDeclarationParsingScope<Types>* AsVariableDeclarationParsingScope() {
+    DCHECK(IsVariableDeclaration());
+    return static_cast<VariableDeclarationParsingScope<Types>*>(this);
+  }
+
   bool IsArrowHeadParsingScope() const {
     return IsInRange(type_, kMaybeArrowParameterDeclaration,
                      kMaybeAsyncArrowParameterDeclaration);
@@ -245,7 +246,6 @@ class ExpressionScope {
   bool IsCertainlyParameterDeclaration() const {
     return type_ == kParameterDeclaration;
   }
-  bool IsLexicalDeclaration() const { return type_ == kLexicalDeclaration; }
 
   ParserT* parser_;
   ExpressionScope<Types>* parent_;
@@ -269,6 +269,45 @@ class VariableDeclarationParsingScope : public ExpressionScope<Types> {
                                      : ExpressionScopeT::kVarDeclaration),
         mode_(mode) {}
 
+  VariableProxy* Declare(VariableProxy* proxy) {
+    VariableKind kind = NORMAL_VARIABLE;
+    bool added;
+    this->parser()->DeclareVariable(
+        proxy, kind, mode_, Variable::DefaultInitializationFlag(mode_),
+        this->parser()->scope(), &added, proxy->position());
+    if (!this->IsLexicalDeclaration()) {
+      if (this->parser()->loop_nesting_depth() > 0) {
+        // Due to hoisting, the value of a 'var'-declared variable may actually
+        // change even if the code contains only the "initial" assignment,
+        // namely when that assignment occurs inside a loop.  For example:
+        //
+        //   let i = 10;
+        //   do { var x = i } while (i--):
+        //
+        // Note that non-lexical variables include temporaries, which may also
+        // get assigned inside a loop due to the various rewritings that the
+        // parser performs.
+        //
+        // Pessimistically mark all vars in loops as assigned. This
+        // overapproximates the actual assigned vars due to unassigned var
+        // without initializer, but that's unlikely anyway.
+        //
+        // This also handles marking of loop variables in for-in and for-of
+        // loops, as determined by loop-nesting-depth.
+        proxy->set_is_assigned();
+      }
+
+      // Make sure we'll properly resolve the variable since we might be in a
+      // with or catch scope. In those cases the assignment isn't guaranteed to
+      // write to the variable declared above.
+      if (!this->parser()->scope()->is_declaration_scope()) {
+        proxy =
+            this->parser()->NewUnresolved(proxy->raw_name(), proxy->position());
+      }
+    }
+    return proxy;
+  }
+
  private:
   VariableMode mode_;
 
@@ -285,7 +324,24 @@ class ParameterDeclarationParsingScope : public ExpressionScope<Types> {
   explicit ParameterDeclarationParsingScope(ParserT* parser)
       : ExpressionScopeT(parser, ExpressionScopeT::kParameterDeclaration) {}
 
+  void Declare(VariableProxy* proxy) {
+    VariableKind kind = PARAMETER_VARIABLE;
+    VariableMode mode = VariableMode::kVar;
+    bool added;
+    this->parser()->DeclareVariable(
+        proxy, kind, mode, Variable::DefaultInitializationFlag(mode),
+        this->parser()->scope(), &added, proxy->position());
+    if (!has_duplicate() && !added) {
+      duplicate_loc_ = proxy->location();
+    }
+  }
+
+  bool has_duplicate() const { return duplicate_loc_.IsValid(); }
+
+  const Scanner::Location& duplicate_location() const { return duplicate_loc_; }
+
  private:
+  Scanner::Location duplicate_loc_ = Scanner::Location::invalid();
   DISALLOW_COPY_AND_ASSIGN(ParameterDeclarationParsingScope);
 };
 
@@ -590,7 +646,20 @@ class ArrowHeadParsingScope : public ExpressionParsingScope<Types> {
 
     DeclarationScope* result = this->parser()->NewFunctionScope(kind());
     if (!has_simple_parameter_list_) result->SetHasNonSimpleParameters();
-    // TODO(verwaest): Add declarations.
+    VariableKind kind = PARAMETER_VARIABLE;
+    VariableMode mode =
+        has_simple_parameter_list_ ? VariableMode::kVar : VariableMode::kLet;
+    for (int i = 0; i < this->variable_list()->length(); i++) {
+      VariableProxy* proxy = this->variable_list()->at(i);
+      bool added;
+      this->parser()->DeclareVariable(proxy, kind, mode,
+                                      Variable::DefaultInitializationFlag(mode),
+                                      result, &added, proxy->position());
+      if (!added) {
+        ExpressionScope<Types>::Report(proxy->location(),
+                                       MessageTemplate::kParamDupe);
+      }
+    }
     return result;
   }
 

src/parsing/parser-base.h

@@ -706,6 +706,17 @@ class ParserBase {
         name, NORMAL_VARIABLE, pos);
   }
 
+  VariableProxy* NewUnresolved(const AstRawString* name) {
+    return scope()->NewUnresolved(factory()->ast_node_factory(), name,
+                                  scanner()->location().beg_pos);
+  }
+
+  VariableProxy* NewUnresolved(const AstRawString* name, int begin_pos,
+                               VariableKind kind = NORMAL_VARIABLE) {
+    return scope()->NewUnresolved(factory()->ast_node_factory(), name,
+                                  begin_pos, kind);
+  }
+
   Scanner* scanner() const { return scanner_; }
   AstValueFactory* ast_value_factory() const { return ast_value_factory_; }
   int position() const { return scanner_->location().beg_pos; }
@@ -1620,10 +1631,10 @@ ParserBase<Impl>::ParsePrimaryExpression() {
       ArrowHeadParsingScope parsing_scope(impl(), kind);
       IdentifierT name = ParseAndClassifyIdentifier(token);
       ClassifyParameter(name, beg_pos, end_position());
+      ExpressionT result =
+          impl()->ExpressionFromIdentifier(name, beg_pos, InferName::kNo);
       next_arrow_function_info_.scope = parsing_scope.ValidateAndCreateScope();
-      FunctionState function_state(&function_state_, &scope_,
-                                   next_arrow_function_info_.scope);
-      return impl()->ExpressionFromIdentifier(name, beg_pos, InferName::kNo);
+      return result;
     }
 
     IdentifierT name = ParseAndClassifyIdentifier(token);
@@ -3771,9 +3782,8 @@ void ParserBase<Impl>::ParseFunctionBody(
     if (is_sloppy(function_scope->language_mode())) {
       impl()->InsertSloppyBlockFunctionVarBindings(function_scope);
     }
-    allow_duplicate_parameters = is_sloppy(function_scope->language_mode()) &&
-                                 !IsConciseMethod(kind) &&
-                                 !IsArrowFunction(kind);
+    allow_duplicate_parameters =
+        is_sloppy(function_scope->language_mode()) && !IsConciseMethod(kind);
   } else {
     DCHECK_NOT_NULL(inner_scope);
     DCHECK_EQ(function_scope, scope());
@@ -4000,8 +4010,6 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
       CheckStrictOctalLiteral(formal_parameters.scope->start_position(),
                               end_position());
     }
-    impl()->CheckConflictingVarDeclarations(formal_parameters.scope);
-
     suspend_count = function_state.suspend_count();
   }
 

src/parsing/parser.cc

@@ -561,6 +561,11 @@ FunctionLiteral* Parser::DoParseProgram(Isolate* isolate, ParseInfo* info) {
       auto name = ast_value_factory()->empty_string();
       bool is_rest = false;
       bool is_optional = false;
+      VariableMode mode = VariableMode::kVar;
+      bool added;
+      scope->DeclareLocal(name, mode, PARAMETER_VARIABLE, &added,
+                          Variable::DefaultInitializationFlag(mode));
+      DCHECK(added);
       auto var = scope->DeclareParameter(name, VariableMode::kVar, is_optional,
                                          is_rest, ast_value_factory(), beg_pos);
       var->AllocateTo(VariableLocation::PARAMETER, 0);
@@ -802,6 +807,7 @@ FunctionLiteral* Parser::DoParseFunction(Isolate* isolate, ParseInfo* info,
           ParseFormalParameter(&formals);
           DeclareFormalParameters(&formals);
         }
+        formals.duplicate_loc = formals_scope.duplicate_location();
       }
 
       if (GetLastFunctionLiteralId() != info->function_literal_id() - 1) {
@@ -1342,15 +1348,6 @@ Statement* Parser::ParseExportDeclaration() {
   return result;
 }
 
-VariableProxy* Parser::NewUnresolved(const AstRawString* name, int begin_pos,
-                                     VariableKind kind) {
-  return scope()->NewUnresolved(factory(), name, begin_pos, kind);
-}
-
-VariableProxy* Parser::NewUnresolved(const AstRawString* name) {
-  return scope()->NewUnresolved(factory(), name, scanner()->location().beg_pos);
-}
-
 VariableProxy* Parser::DeclareVariable(const AstRawString* name,
                                        VariableMode mode, int pos) {
   return DeclareVariable(name, mode, Variable::DefaultInitializationFlag(mode),
@@ -1363,14 +1360,15 @@ VariableProxy* Parser::DeclareVariable(const AstRawString* name,
   DCHECK_NOT_NULL(name);
   VariableProxy* proxy =
       factory()->NewVariableProxy(name, NORMAL_VARIABLE, position());
-  DeclareVariable(proxy, NORMAL_VARIABLE, mode, init, scope(), pos,
+  bool added;
+  DeclareVariable(proxy, NORMAL_VARIABLE, mode, init, scope(), &added, pos,
                   end_position());
   return proxy;
 }
 
 void Parser::DeclareVariable(VariableProxy* proxy, VariableKind kind,
                              VariableMode mode, InitializationFlag init,
-                             Scope* scope, int begin, int end) {
+                             Scope* scope, bool* added, int begin, int end) {
   Declaration* declaration;
   if (mode == VariableMode::kVar && !scope->is_declaration_scope()) {
     DCHECK(scope->is_block_scope() || scope->is_with_scope());
@@ -1378,15 +1376,16 @@ void Parser::DeclareVariable(VariableProxy* proxy, VariableKind kind,
   } else {
     declaration = factory()->NewVariableDeclaration(begin);
   }
-  Declare(declaration, proxy, kind, mode, init, scope, end);
+  return Declare(declaration, proxy, kind, mode, init, scope, added, end);
 }
 
 void Parser::Declare(Declaration* declaration, VariableProxy* proxy,
                      VariableKind variable_kind, VariableMode mode,
-                     InitializationFlag init, Scope* scope, int var_end_pos) {
+                     InitializationFlag init, Scope* scope, bool* added,
+                     int var_end_pos) {
   bool local_ok = true;
   bool sloppy_mode_block_scope_function_redefinition = false;
-  scope->DeclareVariable(declaration, proxy, mode, variable_kind, init,
+  scope->DeclareVariable(declaration, proxy, mode, variable_kind, init, added,
                          &sloppy_mode_block_scope_function_redefinition,
                          &local_ok);
   if (!local_ok) {
@@ -1412,7 +1411,7 @@ Statement* Parser::BuildInitializationBlock(
     ZonePtrList<const AstRawString>* names) {
   ScopedPtrList<Statement> statements(pointer_buffer());
   for (const auto& declaration : parsing_result->declarations) {
-    InitializeVariables(&statements, &(parsing_result->descriptor),
+    InitializeVariables(&statements, parsing_result->descriptor.kind,
                         &declaration, names);
   }
   return factory()->NewBlock(true, statements);
@@ -1427,8 +1426,9 @@ Statement* Parser::DeclareFunction(const AstRawString* variable_name,
       factory()->NewVariableProxy(variable_name, NORMAL_VARIABLE, beg_pos);
   Declaration* declaration = factory()->NewFunctionDeclaration(
       function, is_sloppy_block_function, beg_pos);
+  bool added;
   Declare(declaration, proxy, NORMAL_VARIABLE, mode, kCreatedInitialized,
-          scope());
+          scope(), &added);
   if (names) names->Add(variable_name, zone());
   if (is_sloppy_block_function) {
     SloppyBlockFunctionStatement* statement =
@@ -1594,11 +1594,6 @@ Statement* Parser::RewriteSwitchStatement(SwitchStatement* switch_statement,
 
 Block* Parser::RewriteCatchPattern(CatchInfo* catch_info) {
   DCHECK_NOT_NULL(catch_info->pattern);
-  DeclarationDescriptor descriptor;
-  descriptor.kind = NORMAL_VARIABLE;
-  descriptor.mode = VariableMode::kLet;
-  descriptor.declaration_pos = catch_info->pattern->position();
-  descriptor.initialization_pos = catch_info->pattern->position();
 
   // Initializer position for variables declared by the pattern.
   const int initializer_position = position();
@@ -1608,7 +1603,7 @@ Block* Parser::RewriteCatchPattern(CatchInfo* catch_info) {
       factory()->NewVariableProxy(catch_info->variable));
 
   ScopedPtrList<Statement> init_statements(pointer_buffer());
-  InitializeVariables(&init_statements, &descriptor, &decl, nullptr);
+  InitializeVariables(&init_statements, NORMAL_VARIABLE, &decl, nullptr);
   return factory()->NewBlock(true, init_statements);
 }
 
@@ -1831,9 +1826,6 @@ void Parser::DesugarBindingInForEachStatement(ForInfo* for_info,
   Variable* temp = NewTemporary(ast_value_factory()->dot_for_string());
   ScopedPtrList<Statement> each_initialization_statements(pointer_buffer());
   {
-    auto descriptor = for_info->parsing_result.descriptor;
-    descriptor.declaration_pos = kNoSourcePosition;
-    descriptor.initialization_pos = kNoSourcePosition;
     decl.initializer = factory()->NewVariableProxy(temp);
 
     bool is_for_var_of =
@@ -1843,7 +1835,7 @@ void Parser::DesugarBindingInForEachStatement(ForInfo* for_info,
         IsLexicalVariableMode(for_info->parsing_result.descriptor.mode) ||
         is_for_var_of;
 
-    InitializeVariables(&each_initialization_statements, &descriptor, &decl,
+    InitializeVariables(&each_initialization_statements, NORMAL_VARIABLE, &decl,
                         collect_names ? &for_info->bound_names : nullptr);
 
     // Annex B.3.5 prohibits the form
@@ -2619,16 +2611,6 @@ Block* Parser::BuildParameterInitializationBlock(
   ScopedPtrList<Statement> init_statements(pointer_buffer());
   int index = 0;
   for (auto parameter : parameters.params) {
-    DeclarationDescriptor descriptor;
-    descriptor.kind = PARAMETER_VARIABLE;
-    descriptor.mode = VariableMode::kLet;
-    descriptor.declaration_pos = parameter->pattern->position();
-    // The position that will be used by the AssignmentExpression
-    // which copies from the temp parameter to the pattern.
-    //
-    // TODO(adamk): Should this be kNoSourcePosition, since
-    // it's just copying from a temp var to the real param var?
-    descriptor.initialization_pos = parameter->pattern->position();
     Expression* initial_value =
         factory()->NewVariableProxy(parameters.scope->parameter(index));
     if (parameter->initializer() != nullptr) {
@@ -2654,7 +2636,6 @@ Block* Parser::BuildParameterInitializationBlock(
       initial_value =
           factory()->NewConditional(condition, parameter->initializer(),
                                     initial_value, kNoSourcePosition);
-      descriptor.initialization_pos = parameter->initializer()->position();
     }
 
     Scope* param_scope = scope();
@@ -2664,7 +2645,7 @@ Block* Parser::BuildParameterInitializationBlock(
     if (!parameter->is_simple() &&
         scope()->AsDeclarationScope()->calls_sloppy_eval()) {
       param_scope = NewVarblockScope();
-      param_scope->set_start_position(descriptor.initialization_pos);
+      param_scope->set_start_position(parameter->pattern->position());
       param_scope->set_end_position(parameter->initializer_end_position);
       param_scope->RecordEvalCall();
       non_simple_param_init_statements.emplace(pointer_buffer());
@@ -2677,7 +2658,8 @@ Block* Parser::BuildParameterInitializationBlock(
     DeclarationParsingResult::Declaration decl(
         parameter->pattern, parameter->initializer_end_position, initial_value);
 
-    InitializeVariables(param_init_statements, &descriptor, &decl, nullptr);
+    InitializeVariables(param_init_statements, PARAMETER_VARIABLE, &decl,
+                        nullptr);
 
     if (param_init_statements != &init_statements) {
       DCHECK_EQ(param_init_statements,
@@ -2687,9 +2669,6 @@ Block* Parser::BuildParameterInitializationBlock(
       non_simple_param_init_statements.reset();
       param_block->set_scope(param_scope);
       param_scope = param_scope->FinalizeBlockScope();
-      if (param_scope != nullptr) {
-        CheckConflictingVarDeclarations(param_scope);
-      }
       init_statements.Add(param_block);
     }
     ++index;
@@ -2699,8 +2678,10 @@ Block* Parser::BuildParameterInitializationBlock(
 
 Scope* Parser::NewHiddenCatchScope() {
   Scope* catch_scope = NewScopeWithParent(scope(), CATCH_SCOPE);
+  bool added;
   catch_scope->DeclareLocal(ast_value_factory()->dot_catch_string(),
-                            VariableMode::kVar);
+                            VariableMode::kVar, NORMAL_VARIABLE, &added);
+  DCHECK(added);
   catch_scope->set_is_hidden();
   return catch_scope;
 }
@@ -2817,6 +2798,7 @@ void Parser::ParseFunction(
                              formals_end_position);
       Expect(Token::LBRACE);
     }
+    formals.duplicate_loc = formals_scope.duplicate_location();
   }
 
   *num_parameters = formals.num_parameters();

src/parsing/parser.h

@@ -206,6 +206,9 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   friend class ParserBase<Parser>;
   friend struct ParserFormalParameters;
   friend class i::ExpressionScope<ParserTypes<Parser>>;
+  friend class i::VariableDeclarationParsingScope<ParserTypes<Parser>>;
+  friend class i::ParameterDeclarationParsingScope<ParserTypes<Parser>>;
+  friend class i::ArrowHeadParsingScope<ParserTypes<Parser>>;
   friend bool v8::internal::parsing::ParseProgram(ParseInfo*, Isolate*);
   friend bool v8::internal::parsing::ParseFunction(
       ParseInfo*, Handle<SharedFunctionInfo> shared_info, Isolate*);
@@ -377,8 +380,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   // PatternRewriter and associated methods defined in pattern-rewriter.cc.
   friend class PatternRewriter;
   void InitializeVariables(
-      ScopedPtrList<Statement>* statements,
-      const DeclarationDescriptor* declaration_descriptor,
+      ScopedPtrList<Statement>* statements, VariableKind kind,
       const DeclarationParsingResult::Declaration* declaration,
       ZonePtrList<const AstRawString>* names);
 
@@ -423,20 +425,18 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   // Implement sloppy block-scoped functions, ES2015 Annex B 3.3
   void InsertSloppyBlockFunctionVarBindings(DeclarationScope* scope);
 
-  VariableProxy* NewUnresolved(const AstRawString* name, int begin_pos,
-                               VariableKind kind = NORMAL_VARIABLE);
-  VariableProxy* NewUnresolved(const AstRawString* name);
   VariableProxy* DeclareVariable(const AstRawString* name, VariableMode mode,
                                  int pos);
   VariableProxy* DeclareVariable(const AstRawString* name, VariableMode mode,
                                  InitializationFlag init, int pos);
   void DeclareVariable(VariableProxy* proxy, VariableKind kind,
                        VariableMode mode, InitializationFlag init,
-                       Scope* declaration_scope, int begin,
+                       Scope* declaration_scope, bool* added, int begin,
                        int end = kNoSourcePosition);
   void Declare(Declaration* declaration, VariableProxy* proxy,
                VariableKind kind, VariableMode mode, InitializationFlag init,
-               Scope* declaration_scope, int var_end_pos = kNoSourcePosition);
+               Scope* declaration_scope, bool* added,
+               int var_end_pos = kNoSourcePosition);
 
   bool TargetStackContainsLabel(const AstRawString* label);
   BreakableStatement* LookupBreakTarget(const AstRawString* label);
@@ -890,12 +890,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
       // their names. If the parameter list is not simple, declare a temporary
       // for each parameter - the corresponding named variable is declared by
       // BuildParamerterInitializationBlock.
-      if (is_simple && !parameters->has_duplicate() &&
-          scope->LookupLocal(parameter->name())) {
-        parameters->duplicate_loc = Scanner::Location(
-            parameter->position,
-            parameter->position + parameter->name()->length());
-      }
       scope->DeclareParameter(
           is_simple ? parameter->name() : ast_value_factory()->empty_string(),
           is_simple ? VariableMode::kVar : VariableMode::kTemporary,

src/parsing/pattern-rewriter.cc

@@ -39,20 +39,19 @@ class PatternRewriter final : public AstVisitor<PatternRewriter> {
   typedef Parser::DeclarationDescriptor DeclarationDescriptor;
 
   static void InitializeVariables(
-      Parser* parser, const DeclarationDescriptor* declaration_descriptor,
+      Parser* parser, VariableKind kind,
       const Parser::DeclarationParsingResult::Declaration* declaration,
       ZonePtrList<const AstRawString>* names);
 
  private:
-  PatternRewriter(Parser* parser, const DeclarationDescriptor* descriptor,
-                  ZonePtrList<const AstRawString>* names, bool has_initializer,
+  PatternRewriter(Parser* parser, VariableKind kind,
+                  ZonePtrList<const AstRawString>* names,
                   int initializer_position = kNoSourcePosition,
                   bool declares_parameter_containing_sloppy_eval = false)
       : parser_(parser),
-        descriptor_(descriptor),
+        kind_(kind),
         names_(names),
         initializer_position_(initializer_position),
-        has_initializer_(has_initializer),
         declares_parameter_containing_sloppy_eval_(
             declares_parameter_containing_sloppy_eval) {}
 
@@ -90,23 +89,20 @@ class PatternRewriter final : public AstVisitor<PatternRewriter> {
   Scope* scope() const { return parser_->scope(); }
 
   Parser* const parser_;
-  const DeclarationDescriptor* descriptor_;
+  VariableKind kind_;
   ZonePtrList<const AstRawString>* names_;
   const int initializer_position_;
-  const bool has_initializer_;
   const bool declares_parameter_containing_sloppy_eval_;
 
   DEFINE_AST_VISITOR_MEMBERS_WITHOUT_STACKOVERFLOW()
 };
 
 void Parser::InitializeVariables(
-    ScopedPtrList<Statement>* statements,
-    const DeclarationDescriptor* declaration_descriptor,
+    ScopedPtrList<Statement>* statements, VariableKind kind,
     const DeclarationParsingResult::Declaration* declaration,
     ZonePtrList<const AstRawString>* names) {
   if (has_error()) return;
-  PatternRewriter::InitializeVariables(this, declaration_descriptor,
-                                       declaration, names);
+  PatternRewriter::InitializeVariables(this, kind, declaration, names);
 
   if (declaration->initializer) {
     int pos = declaration->value_beg_position;
@@ -120,88 +116,37 @@ void Parser::InitializeVariables(
 }
 
 void PatternRewriter::InitializeVariables(
-    Parser* parser, const DeclarationDescriptor* declaration_descriptor,
+    Parser* parser, VariableKind kind,
     const Parser::DeclarationParsingResult::Declaration* declaration,
     ZonePtrList<const AstRawString>* names) {
-  PatternRewriter rewriter(parser, declaration_descriptor, names,
-                           declaration->initializer != nullptr,
-                           declaration->initializer_position,
-                           declaration_descriptor->kind == PARAMETER_VARIABLE &&
-                               parser->scope()->is_block_scope());
+  PatternRewriter rewriter(
+      parser, kind, names, declaration->initializer_position,
+      kind == PARAMETER_VARIABLE && parser->scope()->is_block_scope());
 
   rewriter.RecurseIntoSubpattern(declaration->pattern);
 }
 
 void PatternRewriter::VisitVariableProxy(VariableProxy* proxy) {
-  DCHECK_NOT_NULL(descriptor_);
-
   Scope* target_scope = scope();
   if (declares_parameter_containing_sloppy_eval_) {
     // 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.
-    DCHECK_EQ(descriptor_->mode, VariableMode::kLet);
     target_scope = target_scope->outer_scope();
   }
-  Scope* var_init_scope = scope();
-
-#ifdef DEBUG
-  // Calculate the scope we expect the variable to be declared in, for DCHECKs.
-  Scope* expected_declaration_scope =
-      declares_parameter_containing_sloppy_eval_
-          ? scope()->outer_scope()
-          : (IsLexicalVariableMode(descriptor_->mode)
-                 ? scope()
-                 : scope()->GetDeclarationScope());
-#endif
-
-  // Declare variable.
-  // Note that we *always* must treat the initial value via a separate init
-  // assignment for variables and constants because the value must be assigned
-  // when the variable is encountered in the source. But the variable/constant
-  // is declared (and set to 'undefined') upon entering the function within
-  // which the variable or constant is declared. Only function variables have
-  // an initial value in the declaration (because they are initialized upon
-  // entering the function).
-
-  // A declaration of the form:
-  //
-  //    var v = x;
-  //
-  // is syntactic sugar for:
-  //
-  //    var v; v = x;
-  //
-  // In particular, we need to re-lookup 'v' if it may be a different 'v' than
-  // the 'v' in the declaration (e.g., if we are inside a 'with' statement or
-  // 'catch' block).
-  //
-  // For 'let' and 'const' declared variables the initialization always assigns
-  // to the declared variable. But for var initializations that are declared in
-  // a different scope we need to do a new lookup, so clone the variable for the
-  // declaration and don't consider the original variable resolved.
-  if (has_initializer_ && descriptor_->mode == VariableMode::kVar &&
-      !var_init_scope->is_declaration_scope()) {
-    DCHECK_EQ(target_scope->GetDeclarationScope(), expected_declaration_scope);
-    // The cloned variable is not added to the unresolved list of the target
-    // scope, as it is about to be resolved by the declaration. The original
-    // variable will be left unresolved for now.
-    var_init_scope->AddUnresolved(proxy);
-    proxy = factory()->NewVariableProxy(proxy->raw_name(), NORMAL_VARIABLE,
-                                        proxy->position());
-  }
 
-  parser_->DeclareVariable(
-      proxy, descriptor_->kind, descriptor_->mode,
-      Variable::DefaultInitializationFlag(descriptor_->mode), target_scope,
-      descriptor_->declaration_pos);
+  DCHECK(!parser_->has_error());
+  Variable* var =
+      proxy->is_resolved()
+          ? 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();
 
-  if (parser_->has_error()) return;
-  Variable* var = proxy->var();
   DCHECK_NOT_NULL(var);
-  DCHECK(proxy->is_resolved());
-  DCHECK_EQ(var->scope(), expected_declaration_scope);
   DCHECK_NE(initializer_position_, kNoSourcePosition);
   var->set_initializer_position(initializer_position_);
 

src/parsing/preparser.cc

@@ -151,6 +151,7 @@ PreParser::PreParseResult PreParser::PreParseFunction(
     // We return kPreParseSuccess in failure cases too - errors are retrieved
     // separately by Parser::SkipLazyFunctionBody.
     ParseFormalParameterList(&formals);
+    if (formals_scope.has_duplicate()) formals.set_has_duplicate();
     if (!formals.is_simple) {
       BuildParameterInitializationBlock(formals);
     }
@@ -183,9 +184,8 @@ PreParser::PreParseResult PreParser::PreParseFunction(
       function_scope->HoistSloppyBlockFunctions(nullptr);
     }
 
-    allow_duplicate_parameters = is_sloppy(function_scope->language_mode()) &&
-                                 !IsConciseMethod(kind) &&
-                                 !IsArrowFunction(kind);
+    allow_duplicate_parameters =
+        is_sloppy(function_scope->language_mode()) && !IsConciseMethod(kind);
   } else {
     if (is_sloppy(inner_scope->language_mode())) {
       inner_scope->HoistSloppyBlockFunctions(nullptr);
@@ -309,6 +309,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
     {
       ParameterDeclarationParsingScope formals_scope(this);
       ParseFormalParameterList(&formals);
+      if (formals_scope.has_duplicate()) formals.set_has_duplicate();
     }
     Expect(Token::RPAREN);
     int formals_end_position = scanner()->location().end_pos;
@@ -419,26 +420,16 @@ PreParserExpression PreParser::ExpressionFromIdentifier(
 }
 
 void PreParser::InitializeVariables(
-    PreParserScopedStatementList* statements,
-    const DeclarationDescriptor* declaration_descriptor,
     const DeclarationParsingResult::Declaration* declaration,
     ZonePtrList<const AstRawString>* names) {
-  if (declaration->pattern.variables_ != nullptr) {
+  if (names && declaration->pattern.variables_ != nullptr) {
     for (auto variable : *(declaration->pattern.variables_)) {
-      Variable* var = scope()->DeclareVariableName(
-          variable->raw_name(), declaration_descriptor->mode);
-      if (var == nullptr) {
-        ReportUnidentifiableError();
-        return;
-      }
       // This is only necessary if there is an initializer, but we don't have
       // that information here.  Consequently, the preparser sometimes says
       // maybe-assigned where the parser (correctly) says never-assigned.
-      if (names) {
       names->Add(variable->raw_name(), zone());
     }
   }
-  }
 }
 
 }  // namespace internal

src/parsing/preparser.h

@@ -1028,6 +1028,9 @@ class PreParser : public ParserBase<PreParser> {
 
  private:
   friend class i::ExpressionScope<ParserTypes<PreParser>>;
+  friend class i::VariableDeclarationParsingScope<ParserTypes<PreParser>>;
+  friend class i::ParameterDeclarationParsingScope<ParserTypes<PreParser>>;
+  friend class i::ArrowHeadParsingScope<ParserTypes<PreParser>>;
   friend class PreParserFormalParameters;
   // These types form an algebra over syntactic categories that is just
   // rich enough to let us recognize and propagate the constructs that
@@ -1107,8 +1110,6 @@ class PreParser : public ParserBase<PreParser> {
       const PreParserExpression& return_value) {}
 
   void InitializeVariables(
-      PreParserScopedStatementList* statements,
-      const DeclarationDescriptor* declaration_descriptor,
       const DeclarationParsingResult::Declaration* declaration,
       ZonePtrList<const AstRawString>* names);
 
@@ -1134,12 +1135,21 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserStatement::Default();
   }
 
-  V8_INLINE PreParserBlock RewriteCatchPattern(CatchInfo* catch_info) {
-    if (catch_info->pattern.variables_ != nullptr) {
-      for (auto variable : *catch_info->pattern.variables_) {
-        scope()->DeclareVariableName(variable->raw_name(), VariableMode::kLet);
+  void DeclareVariable(VariableProxy* proxy, VariableKind kind,
+                       VariableMode mode, InitializationFlag init, Scope* scope,
+                       bool* added, int position) {
+    DeclareVariableName(proxy->raw_name(), mode, scope, added, kind);
+  }
+
+  void DeclareVariableName(const AstRawString* name, VariableMode mode,
+                           Scope* scope, bool* added,
+                           VariableKind kind = NORMAL_VARIABLE) {
+    if (scope->DeclareVariableName(name, mode, added, kind) == nullptr) {
+      ReportUnidentifiableError();
     }
   }
+
+  V8_INLINE PreParserBlock RewriteCatchPattern(CatchInfo* catch_info) {
     return PreParserBlock::Default();
   }
 
@@ -1207,7 +1217,12 @@ class PreParser : public ParserBase<PreParser> {
                   ZonePtrList<const AstRawString>* names) {
     DCHECK_NULL(names);
     if (variable_name.string_ != nullptr) {
-      scope()->DeclareVariableName(variable_name.string_, mode);
+      bool added;
+      if (is_strict(language_mode())) {
+        DeclareVariableName(variable_name.string_, mode, scope(), &added);
+      } else {
+        scope()->DeclareVariableName(variable_name.string_, mode, &added);
+      }
       if (is_sloppy_block_function) {
         GetDeclarationScope()->DeclareSloppyBlockFunction(variable_name.string_,
                                                           scope());
@@ -1223,7 +1238,9 @@ class PreParser : public ParserBase<PreParser> {
     // Preparser shouldn't be used in contexts where we need to track the names.
     DCHECK_NULL(names);
     if (variable_name.string_ != nullptr) {
-      scope()->DeclareVariableName(variable_name.string_, VariableMode::kLet);
+      bool added;
+      DeclareVariableName(variable_name.string_, VariableMode::kLet, scope(),
+                          &added);
     }
     return PreParserStatement::Default();
   }
@@ -1231,7 +1248,8 @@ class PreParser : public ParserBase<PreParser> {
                                       ClassInfo* class_info,
                                       int class_token_pos) {
     if (name.string_ != nullptr) {
-      scope()->DeclareVariableName(name.string_, VariableMode::kConst);
+      bool added;
+      DeclareVariableName(name.string_, VariableMode::kConst, scope(), &added);
     }
   }
   V8_INLINE void DeclareClassProperty(const PreParserIdentifier& class_name,
@@ -1245,15 +1263,15 @@ class PreParser : public ParserBase<PreParser> {
                                    bool is_private, ClassInfo* class_info) {
     DCHECK_IMPLIES(is_computed_name, !is_private);
     if (is_computed_name) {
-      scope()->DeclareVariableName(
+      bool added;
+      DeclareVariableName(
           ClassFieldVariableName(ast_value_factory(),
                                  class_info->computed_field_count),
-          VariableMode::kConst);
+          VariableMode::kConst, scope(), &added);
     } else if (is_private && property_name.string_ != nullptr) {
-      if (scope()->DeclareVariableName(property_name.string_,
-                                       VariableMode::kConst) == nullptr) {
-        ReportUnidentifiableError();
-      }
+      bool added;
+      DeclareVariableName(property_name.string_, VariableMode::kConst, scope(),
+                          &added);
     }
   }
 
@@ -1411,8 +1429,7 @@ class PreParser : public ParserBase<PreParser> {
   BuildInitializationBlock(DeclarationParsingResult* parsing_result,
                            ZonePtrList<const AstRawString>* names) {
     for (auto declaration : parsing_result->declarations) {
-      InitializeVariables(nullptr, &(parsing_result->descriptor), &declaration,
-                          names);
+      InitializeVariables(&declaration, names);
     }
     return PreParserStatement::Default();
   }
@@ -1432,8 +1449,7 @@ class PreParser : public ParserBase<PreParser> {
         IsLexicalVariableMode(for_info->parsing_result.descriptor.mode) ||
         is_for_var_of;
 
-    InitializeVariables(nullptr, &for_info->parsing_result.descriptor,
-                        &for_info->parsing_result.declarations[0],
+    InitializeVariables(&for_info->parsing_result.declarations[0],
                         collect_names ? &for_info->bound_names : nullptr);
   }
 
@@ -1441,7 +1457,8 @@ class PreParser : public ParserBase<PreParser> {
                                                      const ForInfo& for_info) {
     if (IsLexicalVariableMode(for_info.parsing_result.descriptor.mode)) {
       for (auto name : for_info.bound_names) {
-        scope()->DeclareVariableName(name, VariableMode::kLet);
+        bool added;
+        DeclareVariableName(name, VariableMode::kLet, scope(), &added);
       }
       return PreParserBlock::Default();
     }
@@ -1454,8 +1471,9 @@ class PreParser : public ParserBase<PreParser> {
       PreParserStatement body, Scope* inner_scope, const ForInfo& for_info) {
     // See Parser::DesugarLexicalBindingsInForStatement.
     for (auto name : for_info.bound_names) {
-      inner_scope->DeclareVariableName(name,
-                                       for_info.parsing_result.descriptor.mode);
+      bool added;
+      DeclareVariableName(name, for_info.parsing_result.descriptor.mode,
+                          inner_scope, &added);
     }
     return loop;
   }
@@ -1632,13 +1650,6 @@ class PreParser : public ParserBase<PreParser> {
                                     bool is_rest) {
     DeclarationScope* scope = parameters->scope;
     scope->RecordParameter(is_rest);
-    if (pattern.variables_) {
-      for (VariableProxy* param : *pattern.variables_) {
-        const AstRawString* name = param->raw_name();
-        if (scope->LookupLocal(name)) parameters->set_has_duplicate();
-        scope->DeclareParameterName(name);
-      }
-    }
     parameters->UpdateArityAndFunctionLength(!initializer.IsNull(), is_rest);
   }
 
@@ -1650,14 +1661,6 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE void DeclareArrowFunctionFormalParameters(
       PreParserFormalParameters* parameters, const PreParserExpression& params,
       const Scanner::Location& params_loc) {
-    if (params.variables_ != nullptr) {
-      DeclarationScope* scope = parameters->scope;
-      for (auto param : *params.variables_) {
-        const AstRawString* name = param->raw_name();
-        if (scope->LookupLocal(name)) parameters->set_has_duplicate();
-        scope->DeclareParameterName(name);
-      }
-    }
   }
 
   V8_INLINE PreParserExpression

test/cctest/interpreter/bytecode_expectations/CreateRestParameter.golden

@@ -16,9 +16,9 @@ parameter count: 1
 bytecode array length: 7
 bytecodes: [
                 B(CreateRestParameter),
-                B(Star), R(0),
+                B(Star), R(1),
   /*   10 E> */ B(StackCheck),
-  /*   22 S> */ B(Star), R(1),
+  /*   22 S> */ B(Star), R(0),
   /*   42 S> */ B(Return),
 ]
 constant pool: [
@@ -36,11 +36,11 @@ parameter count: 2
 bytecode array length: 13
 bytecodes: [
                 B(CreateRestParameter),
-                B(Star), R(0),
+                B(Star), R(2),
   /*   10 E> */ B(StackCheck),
-  /*   12 S> */ B(Mov), R(arg0), R(1),
-  /*   25 S> */ B(Mov), R(0), R(2),
-  /*   29 S> */ B(Ldar), R(2),
+  /*   12 S> */ B(Mov), R(arg0), R(0),
+  /*   25 S> */ B(Mov), R(2), R(1),
+  /*   29 S> */ B(Ldar), R(1),
   /*   45 S> */ B(Return),
 ]
 constant pool: [
@@ -58,12 +58,12 @@ parameter count: 2
 bytecode array length: 15
 bytecodes: [
                 B(CreateRestParameter),
-                B(Star), R(0),
+                B(Star), R(2),
   /*   10 E> */ B(StackCheck),
-  /*   12 S> */ B(Mov), R(arg0), R(1),
-  /*   25 S> */ B(Mov), R(0), R(2),
+  /*   12 S> */ B(Mov), R(arg0), R(0),
+  /*   25 S> */ B(Mov), R(2), R(1),
   /*   29 S> */ B(LdaZero),
-  /*   44 E> */ B(LdaKeyedProperty), R(2), U8(0),
+  /*   44 E> */ B(LdaKeyedProperty), R(1), U8(0),
   /*   48 S> */ B(Return),
 ]
 constant pool: [
@@ -83,12 +83,12 @@ bytecodes: [
                 B(CreateUnmappedArguments),
                 B(Star), R(3),
                 B(CreateRestParameter),
-                B(Star), R(0),
+                B(Star), R(2),
   /*   10 E> */ B(StackCheck),
-  /*   12 S> */ B(Mov), R(arg0), R(1),
-  /*   25 S> */ B(Mov), R(0), R(2),
+  /*   12 S> */ B(Mov), R(arg0), R(0),
+  /*   25 S> */ B(Mov), R(2), R(1),
   /*   29 S> */ B(LdaZero),
-  /*   44 E> */ B(LdaKeyedProperty), R(2), U8(1),
+  /*   44 E> */ B(LdaKeyedProperty), R(1), U8(1),
                 B(Star), R(4),
                 B(LdaZero),
   /*   59 E> */ B(LdaKeyedProperty), R(3), U8(3),

test/cctest/interpreter/bytecode_expectations/ForIn.golden

@@ -79,9 +79,9 @@ bytecodes: [
                 B(JumpIfFalse), U8(22),
                 B(ForInNext), R(3), R(7), R(4), U8(0),
                 B(JumpIfUndefined), U8(8),
-                B(Star), R(1),
+                B(Star), R(2),
   /*   54 E> */ B(StackCheck),
-  /*   63 S> */ B(Star), R(2),
+  /*   63 S> */ B(Star), R(1),
   /*   82 S> */ B(Return),
                 B(ForInStep), R(7),
                 B(Star), R(7),
@@ -119,10 +119,10 @@ bytecodes: [
                 B(JumpIfFalse), U8(31),
                 B(ForInNext), R(3), R(7), R(4), U8(0),
                 B(JumpIfUndefined), U8(17),
-                B(Star), R(1),
+                B(Star), R(2),
   /*   45 E> */ B(StackCheck),
-  /*   54 S> */ B(Star), R(2),
-  /*   70 S> */ B(Ldar), R(1),
+  /*   54 S> */ B(Star), R(1),
+  /*   70 S> */ B(Ldar), R(2),
   /*   75 E> */ B(Add), R(0), U8(2),
                 B(Mov), R(0), R(8),
                 B(Star), R(0),

test/cctest/interpreter/bytecode_expectations/ForOf.golden

@@ -39,9 +39,9 @@ bytecodes: [
                 B(Star), R(10),
                 B(LdaFalse),
                 B(Star), R(6),
-                B(Mov), R(10), R(0),
+                B(Mov), R(10), R(1),
   /*   34 E> */ B(StackCheck),
-  /*   43 S> */ B(Mov), R(0), R(1),
+  /*   43 S> */ B(Mov), R(1), R(0),
                 B(Ldar), R(10),
                 B(JumpLoop), U8(40), I8(0),
                 B(LdaSmi), I8(-1),
@@ -140,9 +140,9 @@ bytecodes: [
                 B(Star), R(11),
                 B(LdaFalse),
                 B(Star), R(7),
-                B(Mov), R(11), R(1),
+                B(Mov), R(11), R(2),
   /*   54 E> */ B(StackCheck),
-  /*   63 S> */ B(Mov), R(1), R(2),
+  /*   63 S> */ B(Mov), R(2), R(1),
   /*   73 S> */ B(LdaSmi), I8(1),
                 B(Mov), R(11), R(9),
                 B(Star), R(8),
@@ -248,15 +248,15 @@ bytecodes: [
                 B(Star), R(10),
                 B(LdaFalse),
                 B(Star), R(6),
-                B(Mov), R(10), R(0),
+                B(Mov), R(10), R(1),
   /*   34 E> */ B(StackCheck),
-  /*   43 S> */ B(Mov), R(0), R(1),
+  /*   43 S> */ B(Mov), R(1), R(0),
   /*   66 S> */ B(LdaSmi), I8(10),
-  /*   72 E> */ B(TestEqual), R(1), U8(13),
+  /*   72 E> */ B(TestEqual), R(0), U8(13),
                 B(JumpIfFalse), U8(4),
   /*   79 S> */ B(Jump), U8(11),
   /*   91 S> */ B(LdaSmi), I8(20),
-  /*   97 E> */ B(TestEqual), R(1), U8(14),
+  /*   97 E> */ B(TestEqual), R(0), U8(14),
                 B(JumpIfFalse), U8(4),
   /*  104 S> */ B(Jump), U8(5),
                 B(JumpLoop), U8(56), I8(0),

test/cctest/interpreter/bytecode_expectations/SuperCallAndSpread.golden

@@ -54,16 +54,16 @@ parameter count: 1
 bytecode array length: 40
 bytecodes: [
                 B(CreateRestParameter),
-                B(Star), R(2),
+                B(Star), R(3),
                 B(Mov), R(closure), R(1),
   /*  128 E> */ B(StackCheck),
-  /*  136 S> */ B(Mov), R(2), R(3),
+  /*  136 S> */ B(Mov), R(3), R(2),
   /*  140 S> */ B(Ldar), R(closure),
                 B(GetSuperConstructor), R(5),
                 B(LdaSmi), I8(1),
                 B(Star), R(6),
                 B(Ldar), R(0),
-                B(Mov), R(2), R(7),
+                B(Mov), R(3), R(7),
   /*  140 E> */ B(ConstructWithSpread), R(5), R(6), U8(2), U8(0),
                 B(Star), R(8),
                 B(Ldar), R(this),
@@ -96,10 +96,10 @@ parameter count: 1
 bytecode array length: 128
 bytecodes: [
                 B(CreateRestParameter),
-                B(Star), R(2),
+                B(Star), R(3),
                 B(Mov), R(closure), R(1),
   /*  128 E> */ B(StackCheck),
-  /*  136 S> */ B(Mov), R(2), R(3),
+  /*  136 S> */ B(Mov), R(3), R(2),
   /*  140 S> */ B(Ldar), R(closure),
                 B(GetSuperConstructor), R(5),
                 B(CreateEmptyArrayLiteral), U8(0),
@@ -111,10 +111,10 @@ bytecodes: [
                 B(Ldar), R(6),
                 B(Inc), U8(3),
   /*  152 S> */ B(Star), R(6),
-                B(LdaNamedProperty), R(2), U8(0), U8(4),
+                B(LdaNamedProperty), R(3), U8(0), U8(4),
                 B(Star), R(11),
-                B(CallProperty0), R(11), R(2), U8(6),
-                B(Mov), R(2), R(10),
+                B(CallProperty0), R(11), R(3), U8(6),
+                B(Mov), R(3), R(10),
                 B(Mov), R(1), R(4),
                 B(JumpIfJSReceiver), U8(7),
                 B(CallRuntime), U16(Runtime::kThrowSymbolIteratorInvalid), R(0), U8(0),