[parser] Skip TDZ Checks in more cases of let and const

The parser can now skip TDZ checks for cases when a reference is in,
or nested in, a scope that's both a sibling of the declaration and
created by a function expression.

Bug: v8:7331
Change-Id: Ia9748b5a8faa3037873efe5081837f5d0aa74115
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1542042
Commit-Queue: Suraj Sharma <surshar@microsoft.com>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60853}

src/ast/scopes.cc

@@ -116,8 +116,10 @@ DeclarationScope::DeclarationScope(Zone* zone,
 
 DeclarationScope::DeclarationScope(Zone* zone, Scope* outer_scope,
                                    ScopeType scope_type,
-                                   FunctionKind function_kind)
+                                   FunctionKind function_kind,
+                                   NormalFunctionType type)
     : Scope(zone, outer_scope, scope_type),
+      is_function_expression_(type == NormalFunctionType::kExpression),
       function_kind_(function_kind),
       params_(4, zone) {
   DCHECK_NE(scope_type, SCRIPT_SCOPE);
@@ -1949,12 +1951,13 @@ void UpdateNeedsHoleCheck(Variable* var, VariableProxy* proxy, Scope* scope) {
   }
 
   // Check if the binding really needs an initialization check. The check
-  // can be skipped in the following situation: we have a VariableMode::kLet or
-  // VariableMode::kConst binding, both the Variable and the VariableProxy have
-  // the same declaration scope (i.e. they are both in global code, in the same
-  // function or in the same eval code), the VariableProxy is in the source
-  // physically located after the initializer of the variable, and that the
-  // initializer cannot be skipped due to a nonlinear scope.
+  // can be skipped in the following situation:
+  // 1. We have a VariableMode::kLet or VariableMode::kConst binding.
+  // 2. The outermost closure scope inner to the scope in which the variable
+  //    is declared is not hoisted.
+  // 3. The VariableProxy is in the source physically located after the
+  //    initializer of the variable.
+  // 4. The initializer does not contain a nonlinear scope.
   //
   // The condition on the closure scopes is a conservative check for
   // nested functions that access a binding and are called before the
@@ -1966,8 +1969,26 @@ void UpdateNeedsHoleCheck(Variable* var, VariableProxy* proxy, Scope* scope) {
   //   switch (1) { case 0: let x = 2; case 1: f(x); }
   // The scope of the variable needs to be checked, in case the use is
   // in a sub-block which may be linear.
-  if (var->scope()->GetClosureScope() != scope->GetClosureScope()) {
-    return SetNeedsHoleCheck(var, proxy);
+  DeclarationScope* target = scope->GetClosureScope();
+  DeclarationScope* var_closure = var->scope()->GetClosureScope();
+
+  if (target != var_closure) {
+    // If the Variable doesn't have a valid source position the check
+    // cannot be skipped.
+    if (var->initializer_position() == kNoSourcePosition) {
+      return SetNeedsHoleCheck(var, proxy);
+    }
+
+    DeclarationScope* test_scope = target->outer_scope()->GetClosureScope();
+    while (test_scope != var_closure) {
+      target = test_scope;
+      test_scope = test_scope->outer_scope()->GetClosureScope();
+    }
+
+    // If the target scope is not hoisted Hole Check can be skipped.
+    if (target->is_hoisted_declaration_scope()) {
+      return SetNeedsHoleCheck(var, proxy);
+    }
   }
 
   // We should always have valid source positions.

src/ast/scopes.h

@@ -724,7 +724,8 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
 class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
  public:
   DeclarationScope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
-                   FunctionKind function_kind = kNormalFunction);
+                   FunctionKind function_kind = kNormalFunction,
+                   NormalFunctionType type = NormalFunctionType::kExpression);
   DeclarationScope(Zone* zone, ScopeType scope_type,
                    Handle<ScopeInfo> scope_info);
   // Creates a script scope.
@@ -736,6 +737,10 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
     return is_function_scope() && IsArrowFunction(function_kind_);
   }
 
+  bool is_hoisted_declaration_scope() const {
+    return !is_function_expression_ && !is_arrow_scope() && !is_eval_scope();
+  }
+
   // Inform the scope that the corresponding code uses "super".
   void RecordSuperPropertyUsage() {
     DCHECK(IsConciseMethod(function_kind()) ||
@@ -1075,6 +1080,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   bool has_checked_syntax_ : 1;
   bool has_this_reference_ : 1;
   bool has_this_declaration_ : 1;
+  bool is_function_expression_ : 1;
 
   // If the scope is a function scope, this is the function kind.
   const FunctionKind function_kind_;

src/function-kind.h

@@ -11,6 +11,8 @@
 namespace v8 {
 namespace internal {
 
+enum class NormalFunctionType { kDeclaration, kExpression };
+
 enum FunctionKind : uint8_t {
   // BEGIN constructable functions
   kNormalFunction,

src/parsing/parser-base.h

@@ -680,12 +680,13 @@ class ParserBase {
   // Creates a function scope that always allocates in zone(). The function
   // scope itself is either allocated in zone() or in target_zone if one is
   // passed in.
-  DeclarationScope* NewFunctionScope(FunctionKind kind,
-                                     Zone* parse_zone = nullptr) const {
+  DeclarationScope* NewFunctionScope(
+      FunctionKind kind, Zone* parse_zone = nullptr,
+      NormalFunctionType type = NormalFunctionType::kDeclaration) const {
     DCHECK(ast_value_factory());
     if (parse_zone == nullptr) parse_zone = zone();
     DeclarationScope* result = new (zone())
-        DeclarationScope(parse_zone, scope(), FUNCTION_SCOPE, kind);
+        DeclarationScope(parse_zone, scope(), FUNCTION_SCOPE, kind, type);
 
     // Record presence of an inner function scope
     function_state_->RecordFunctionOrEvalCall();

src/parsing/parser.cc

@@ -2375,7 +2375,12 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
 
   // This Scope lives in the main zone. We'll migrate data into that zone later.
   Zone* parse_zone = should_preparse ? &preparser_zone_ : zone();
-  DeclarationScope* scope = NewFunctionScope(kind, parse_zone);
+  NormalFunctionType type =
+      (function_type != FunctionLiteral::kDeclaration &&
+       function_type != FunctionLiteral::kAccessorOrMethod)
+          ? NormalFunctionType::kExpression
+          : NormalFunctionType::kDeclaration;
+  DeclarationScope* scope = NewFunctionScope(kind, parse_zone, type);
   SetLanguageMode(scope, language_mode);
 #ifdef DEBUG
   scope->SetScopeName(function_name);