[class] Fix private name scope chain

Expressions in class heritage position do not have access to the
inheriting class's private names, only its lexical bindings. The parser
currently uses the same scope chain for both.

This CL makes scopes in class heritage position skip their outer class
when resolving private names. Whether a scope needs to skip is kept as a
bit on various scope-related data structures.

See implementation doc at
https://docs.google.com/document/d/1d3o_SQqcICxfjLMw53OOaiIQux0ppNHQJnjZHtCQLwA

Bug: v8:9177
Change-Id: I77e491a9d4a261131274f12ddf052af7ac31a921
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1769486
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63586}

src/ast/ast.cc

@@ -293,6 +293,10 @@ bool FunctionLiteral::requires_brand_initialization() const {
   return outer->AsClassScope()->brand() != nullptr;
 }
 
+bool FunctionLiteral::private_name_lookup_skips_outer_class() const {
+  return scope()->private_name_lookup_skips_outer_class();
+}
+
 ObjectLiteralProperty::ObjectLiteralProperty(Expression* key, Expression* value,
                                              Kind kind, bool is_computed_name)
     : LiteralProperty(key, value, is_computed_name),

src/ast/ast.h

@@ -2350,6 +2350,8 @@ class FunctionLiteral final : public Expression {
 
   bool requires_brand_initialization() const;
 
+  bool private_name_lookup_skips_outer_class() const;
+
   ProducedPreparseData* produced_preparse_data() const {
     return produced_preparse_data_;
   }

src/ast/scopes.cc

@@ -102,6 +102,9 @@ Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type)
   DCHECK_NE(SCRIPT_SCOPE, scope_type);
   SetDefaults();
   set_language_mode(outer_scope->language_mode());
+  private_name_lookup_skips_outer_class_ =
+      outer_scope->is_class_scope() &&
+      outer_scope->AsClassScope()->IsParsingHeritage();
   outer_scope_->AddInnerScope(this);
 }
 
@@ -141,13 +144,15 @@ ModuleScope::ModuleScope(Isolate* isolate, Handle<ScopeInfo> scope_info,
 }
 
 ClassScope::ClassScope(Zone* zone, Scope* outer_scope)
-    : Scope(zone, outer_scope, CLASS_SCOPE) {
+    : Scope(zone, outer_scope, CLASS_SCOPE),
+      rare_data_and_is_parsing_heritage_(nullptr) {
   set_language_mode(LanguageMode::kStrict);
 }
 
 ClassScope::ClassScope(Zone* zone, AstValueFactory* ast_value_factory,
                        Handle<ScopeInfo> scope_info)
-    : Scope(zone, CLASS_SCOPE, scope_info) {
+    : Scope(zone, CLASS_SCOPE, scope_info),
+      rare_data_and_is_parsing_heritage_(nullptr) {
   set_language_mode(LanguageMode::kStrict);
   if (scope_info->HasClassBrand()) {
     Variable* brand =
@@ -171,6 +176,8 @@ Scope::Scope(Zone* zone, ScopeType scope_type, Handle<ScopeInfo> scope_info)
   set_language_mode(scope_info->language_mode());
   num_heap_slots_ = scope_info->ContextLength();
   DCHECK_LE(Context::MIN_CONTEXT_SLOTS, num_heap_slots_);
+  private_name_lookup_skips_outer_class_ =
+      scope_info->PrivateNameLookupSkipsOuterClass();
   // We don't really need to use the preparsed scope data; this is just to
   // shorten the recursion in SetMustUsePreparseData.
   must_use_preparsed_scope_data_ = true;
@@ -222,6 +229,7 @@ void DeclarationScope::SetDefaults() {
   has_this_reference_ = false;
   has_this_declaration_ =
       (is_function_scope() && !is_arrow_scope()) || is_module_scope();
+  needs_private_name_context_chain_recalc_ = false;
   has_rest_ = false;
   receiver_ = nullptr;
   new_target_ = nullptr;
@@ -270,6 +278,8 @@ void Scope::SetDefaults() {
 
   is_declaration_scope_ = false;
 
+  private_name_lookup_skips_outer_class_ = false;
+
   must_use_preparsed_scope_data_ = false;
 }
 
@@ -1165,9 +1175,9 @@ bool DeclarationScope::AllocateVariables(ParseInfo* info) {
   // to ensure that UpdateNeedsHoleCheck() can detect import variables.
   if (is_module_scope()) AsModuleScope()->AllocateModuleVariables();
 
-  ClassScope* closest_class_scope = GetClassScope();
-  if (closest_class_scope != nullptr &&
-      !closest_class_scope->ResolvePrivateNames(info)) {
+  PrivateNameScopeIterator private_name_scope_iter(this);
+  if (!private_name_scope_iter.Done() &&
+      !private_name_scope_iter.GetScope()->ResolvePrivateNames(info)) {
     DCHECK(info->pending_error_handler()->has_pending_error());
     return false;
   }
@@ -1177,7 +1187,7 @@ bool DeclarationScope::AllocateVariables(ParseInfo* info) {
     return false;
   }
 
-  // // Don't allocate variables of preparsed scopes.
+  // Don't allocate variables of preparsed scopes.
   if (!was_lazily_parsed()) AllocateVariablesRecursively();
 
   return true;
@@ -1254,17 +1264,6 @@ int Scope::ContextChainLengthUntilOutermostSloppyEval() const {
   return result;
 }
 
-ClassScope* Scope::GetClassScope() {
-  Scope* scope = this;
-  while (scope != nullptr && !scope->is_class_scope()) {
-    scope = scope->outer_scope();
-  }
-  if (scope != nullptr && scope->is_class_scope()) {
-    return scope->AsClassScope();
-  }
-  return nullptr;
-}
-
 DeclarationScope* Scope::GetDeclarationScope() {
   Scope* scope = this;
   while (!scope->is_declaration_scope()) {
@@ -1683,11 +1682,17 @@ void Scope::Print(int n) {
   if (is_declaration_scope() && AsDeclarationScope()->NeedsHomeObject()) {
     Indent(n1, "// scope needs home object\n");
   }
+  if (private_name_lookup_skips_outer_class()) {
+    Indent(n1, "// scope skips outer class for #-names\n");
+  }
   if (inner_scope_calls_eval_) Indent(n1, "// inner scope calls 'eval'\n");
   if (is_declaration_scope()) {
     DeclarationScope* scope = AsDeclarationScope();
     if (scope->was_lazily_parsed()) Indent(n1, "// lazily parsed\n");
     if (scope->ShouldEagerCompile()) Indent(n1, "// will be compiled\n");
+    if (scope->needs_private_name_context_chain_recalc()) {
+      Indent(n1, "// needs #-name context chain recalc\n");
+    }
   }
   if (num_stack_slots_ > 0) {
     Indent(n1, "// ");
@@ -1724,9 +1729,9 @@ void Scope::Print(int n) {
 
   if (is_class_scope()) {
     ClassScope* class_scope = AsClassScope();
-    if (class_scope->rare_data_ != nullptr) {
+    if (class_scope->GetRareData() != nullptr) {
       PrintMap(n1, "// private name vars:\n",
-               &(class_scope->rare_data_->private_name_map), true, function);
+               &(class_scope->GetRareData()->private_name_map), true, function);
       Variable* brand = class_scope->brand();
       if (brand != nullptr) {
         Indent(n1, "// brand var:\n");
@@ -2303,6 +2308,47 @@ void Scope::AllocateScopeInfosRecursively(Isolate* isolate,
   }
 }
 
+void DeclarationScope::RecalcPrivateNameContextChain() {
+  // The outermost scope in a class heritage expression is marked to skip the
+  // class scope during private name resolution. It is possible, however, that
+  // either the class scope won't require a Context and ScopeInfo, or the
+  // outermost scope in the heritage position won't. Simply copying the bit from
+  // full parse into the ScopeInfo will break lazy compilation. In the former
+  // case the scope that is marked to skip its outer scope will incorrectly skip
+  // a different class scope than the one we intended to skip. In the latter
+  // case variables resolved through an inner scope will incorrectly check the
+  // class scope since we lost the skip bit from the outermost heritage scope.
+  //
+  // This method fixes both cases by, in outermost to innermost order, copying
+  // the value of the skip bit from outer scopes that don't require a Context.
+  DCHECK(needs_private_name_context_chain_recalc_);
+  this->ForEach([](Scope* scope) {
+    Scope* outer = scope->outer_scope();
+    if (!outer) return Iteration::kDescend;
+    if (!outer->NeedsContext()) {
+      scope->private_name_lookup_skips_outer_class_ =
+          outer->private_name_lookup_skips_outer_class();
+    }
+    if (!scope->is_function_scope() ||
+        scope->AsDeclarationScope()->ShouldEagerCompile()) {
+      return Iteration::kDescend;
+    }
+    return Iteration::kContinue;
+  });
+}
+
+void DeclarationScope::RecordNeedsPrivateNameContextChainRecalc() {
+  DCHECK_EQ(GetClosureScope(), this);
+  DeclarationScope* scope;
+  for (scope = this; scope != nullptr;
+       scope = scope->outer_scope() != nullptr
+                   ? scope->outer_scope()->GetClosureScope()
+                   : nullptr) {
+    if (scope->needs_private_name_context_chain_recalc_) return;
+    scope->needs_private_name_context_chain_recalc_ = true;
+  }
+}
+
 // static
 void DeclarationScope::AllocateScopeInfos(ParseInfo* info, Isolate* isolate) {
   DeclarationScope* scope = info->literal()->scope();
@@ -2313,6 +2359,9 @@ void DeclarationScope::AllocateScopeInfos(ParseInfo* info, Isolate* isolate) {
     outer_scope = scope->outer_scope_->scope_info_;
   }
 
+  if (scope->needs_private_name_context_chain_recalc()) {
+    scope->RecalcPrivateNameContextChain();
+  }
   scope->AllocateScopeInfosRecursively(isolate, outer_scope);
 
   // The debugger expects all shared function infos to contain a scope info.
@@ -2370,38 +2419,42 @@ Variable* ClassScope::DeclarePrivateName(const AstRawString* name,
 }
 
 Variable* ClassScope::LookupLocalPrivateName(const AstRawString* name) {
-  if (rare_data_ == nullptr) {
+  RareData* rare_data = GetRareData();
+  if (rare_data == nullptr) {
     return nullptr;
   }
-  return rare_data_->private_name_map.Lookup(name);
+  return rare_data->private_name_map.Lookup(name);
 }
 
 UnresolvedList::Iterator ClassScope::GetUnresolvedPrivateNameTail() {
-  if (rare_data_ == nullptr) {
+  RareData* rare_data = GetRareData();
+  if (rare_data == nullptr) {
     return UnresolvedList::Iterator();
   }
-  return rare_data_->unresolved_private_names.end();
+  return rare_data->unresolved_private_names.end();
 }
 
 void ClassScope::ResetUnresolvedPrivateNameTail(UnresolvedList::Iterator tail) {
-  if (rare_data_ == nullptr ||
-      rare_data_->unresolved_private_names.end() == tail) {
+  RareData* rare_data = GetRareData();
+  if (rare_data == nullptr ||
+      rare_data->unresolved_private_names.end() == tail) {
     return;
   }
 
   bool tail_is_empty = tail == UnresolvedList::Iterator();
   if (tail_is_empty) {
     // If the saved tail is empty, the list used to be empty, so clear it.
-    rare_data_->unresolved_private_names.Clear();
+    rare_data->unresolved_private_names.Clear();
   } else {
-    rare_data_->unresolved_private_names.Rewind(tail);
+    rare_data->unresolved_private_names.Rewind(tail);
   }
 }
 
 void ClassScope::MigrateUnresolvedPrivateNameTail(
     AstNodeFactory* ast_node_factory, UnresolvedList::Iterator tail) {
-  if (rare_data_ == nullptr ||
-      rare_data_->unresolved_private_names.end() == tail) {
+  RareData* rare_data = GetRareData();
+  if (rare_data == nullptr ||
+      rare_data->unresolved_private_names.end() == tail) {
     return;
   }
   UnresolvedList migrated_names;
@@ -2410,9 +2463,9 @@ void ClassScope::MigrateUnresolvedPrivateNameTail(
   // migrate everything after the head.
   bool tail_is_empty = tail == UnresolvedList::Iterator();
   UnresolvedList::Iterator it =
-      tail_is_empty ? rare_data_->unresolved_private_names.begin() : tail;
+      tail_is_empty ? rare_data->unresolved_private_names.begin() : tail;
 
-  for (; it != rare_data_->unresolved_private_names.end(); ++it) {
+  for (; it != rare_data->unresolved_private_names.end(); ++it) {
     VariableProxy* proxy = *it;
     VariableProxy* copy = ast_node_factory->CopyVariableProxy(proxy);
     migrated_names.Add(copy);
@@ -2420,20 +2473,11 @@ void ClassScope::MigrateUnresolvedPrivateNameTail(
 
   // Replace with the migrated copies.
   if (tail_is_empty) {
-    rare_data_->unresolved_private_names.Clear();
+    rare_data->unresolved_private_names.Clear();
   } else {
-    rare_data_->unresolved_private_names.Rewind(tail);
+    rare_data->unresolved_private_names.Rewind(tail);
   }
-  rare_data_->unresolved_private_names.Append(std::move(migrated_names));
-}
-
-void ClassScope::AddUnresolvedPrivateName(VariableProxy* proxy) {
-  // During a reparse, already_resolved_ may be true here, because
-  // the class scope is deserialized while the function scope inside may
-  // be new.
-  DCHECK(!proxy->is_resolved());
-  DCHECK(proxy->IsPrivateName());
-  EnsureRareData()->unresolved_private_names.Add(proxy);
+  rare_data->unresolved_private_names.Append(std::move(migrated_names));
 }
 
 Variable* ClassScope::LookupPrivateNameInScopeInfo(const AstRawString* name) {
@@ -2467,15 +2511,14 @@ Variable* ClassScope::LookupPrivateNameInScopeInfo(const AstRawString* name) {
 Variable* ClassScope::LookupPrivateName(VariableProxy* proxy) {
   DCHECK(!proxy->is_resolved());
 
-  for (Scope* scope = this; !scope->is_script_scope();
-       scope = scope->outer_scope_) {
-    if (!scope->is_class_scope()) continue;  // Only search in class scopes
-    ClassScope* class_scope = scope->AsClassScope();
+  for (PrivateNameScopeIterator scope_iter(this); !scope_iter.Done();
+       scope_iter.Next()) {
+    ClassScope* scope = scope_iter.GetScope();
     // Try finding it in the private name map first, if it can't be found,
     // try the deseralized scope info.
-    Variable* var = class_scope->LookupLocalPrivateName(proxy->raw_name());
-    if (var == nullptr && !class_scope->scope_info_.is_null()) {
-      var = class_scope->LookupPrivateNameInScopeInfo(proxy->raw_name());
+    Variable* var = scope->LookupLocalPrivateName(proxy->raw_name());
+    if (var == nullptr && !scope->scope_info_.is_null()) {
+      var = scope->LookupPrivateNameInScopeInfo(proxy->raw_name());
     }
     if (var != nullptr) {
       return var;
@@ -2485,12 +2528,12 @@ Variable* ClassScope::LookupPrivateName(VariableProxy* proxy) {
 }
 
 bool ClassScope::ResolvePrivateNames(ParseInfo* info) {
-  if (rare_data_ == nullptr ||
-      rare_data_->unresolved_private_names.is_empty()) {
+  RareData* rare_data = GetRareData();
+  if (rare_data == nullptr || rare_data->unresolved_private_names.is_empty()) {
     return true;
   }
 
-  UnresolvedList& list = rare_data_->unresolved_private_names;
+  UnresolvedList& list = rare_data->unresolved_private_names;
   for (VariableProxy* proxy : list) {
     Variable* var = LookupPrivateName(proxy);
     if (var == nullptr) {
@@ -2512,20 +2555,20 @@ bool ClassScope::ResolvePrivateNames(ParseInfo* info) {
 }
 
 VariableProxy* ClassScope::ResolvePrivateNamesPartially() {
-  if (rare_data_ == nullptr ||
-      rare_data_->unresolved_private_names.is_empty()) {
+  RareData* rare_data = GetRareData();
+  if (rare_data == nullptr || rare_data->unresolved_private_names.is_empty()) {
     return nullptr;
   }
 
-  ClassScope* outer_class_scope =
-      outer_scope_ == nullptr ? nullptr : outer_scope_->GetClassScope();
-  UnresolvedList& unresolved = rare_data_->unresolved_private_names;
-  bool has_private_names = rare_data_->private_name_map.capacity() > 0;
+  PrivateNameScopeIterator private_name_scope_iter(this);
+  private_name_scope_iter.Next();
+  UnresolvedList& unresolved = rare_data->unresolved_private_names;
+  bool has_private_names = rare_data->private_name_map.capacity() > 0;
 
   // If the class itself does not have private names, nor does it have
-  // an outer class scope, then we are certain any private name access
+  // an outer private name scope, then we are certain any private name access
   // inside cannot be resolved.
-  if (!has_private_names && outer_class_scope == nullptr &&
+  if (!has_private_names && private_name_scope_iter.Done() &&
       !unresolved.is_empty()) {
     return unresolved.first();
   }
@@ -2549,15 +2592,15 @@ VariableProxy* ClassScope::ResolvePrivateNamesPartially() {
     // If the current scope does not have declared private names,
     // try looking from the outer class scope later.
     if (var == nullptr) {
-      // There's no outer class scope so we are certain that the variable
+      // There's no outer private name scope so we are certain that the variable
       // cannot be resolved later.
-      if (outer_class_scope == nullptr) {
+      if (private_name_scope_iter.Done()) {
         return proxy;
       }
 
-      // The private name may be found later in the outer class scope,
-      // so push it to the outer sopce.
-      outer_class_scope->AddUnresolvedPrivateName(proxy);
+      // The private name may be found later in the outer private name scope, so
+      // push it to the outer sopce.
+      private_name_scope_iter.AddUnresolvedPrivateName(proxy);
     }
 
     proxy = next;
@@ -2569,7 +2612,7 @@ VariableProxy* ClassScope::ResolvePrivateNamesPartially() {
 
 Variable* ClassScope::DeclareBrandVariable(AstValueFactory* ast_value_factory,
                                            int class_token_pos) {
-  DCHECK_IMPLIES(rare_data_ != nullptr, rare_data_->brand == nullptr);
+  DCHECK_IMPLIES(GetRareData() != nullptr, GetRareData()->brand == nullptr);
   bool was_added;
   Variable* brand = Declare(zone(), ast_value_factory->dot_brand_string(),
                             VariableMode::kConst, NORMAL_VARIABLE,
@@ -2583,5 +2626,46 @@ Variable* ClassScope::DeclareBrandVariable(AstValueFactory* ast_value_factory,
   return brand;
 }
 
+PrivateNameScopeIterator::PrivateNameScopeIterator(Scope* start)
+    : start_scope_(start), current_scope_(start) {
+  if (!start->is_class_scope() || start->AsClassScope()->IsParsingHeritage()) {
+    Next();
+  }
+}
+
+void PrivateNameScopeIterator::Next() {
+  DCHECK(!Done());
+  Scope* inner = current_scope_;
+  Scope* scope = inner->outer_scope();
+  while (scope != nullptr) {
+    if (scope->is_class_scope()) {
+      if (!inner->private_name_lookup_skips_outer_class()) {
+        current_scope_ = scope;
+        return;
+      }
+      skipped_any_scopes_ = true;
+    }
+    inner = scope;
+    scope = scope->outer_scope();
+  }
+  current_scope_ = nullptr;
+}
+
+void PrivateNameScopeIterator::AddUnresolvedPrivateName(VariableProxy* proxy) {
+  // During a reparse, current_scope_->already_resolved_ may be true here,
+  // because the class scope is deserialized while the function scope inside may
+  // be new.
+  DCHECK(!proxy->is_resolved());
+  DCHECK(proxy->IsPrivateName());
+  GetScope()->EnsureRareData()->unresolved_private_names.Add(proxy);
+  // Any closure scope that contain uses of private names that skips over a
+  // class scope due to heritage expressions need private name context chain
+  // recalculation, since not all scopes require a Context or ScopeInfo. See
+  // comment in DeclarationScope::RecalcPrivateNameContextChain.
+  if (V8_UNLIKELY(skipped_any_scopes_)) {
+    start_scope_->GetClosureScope()->RecordNeedsPrivateNameContextChainRecalc();
+  }
+}
+
 }  // namespace internal
 }  // namespace v8

src/ast/scopes.h

@@ -360,6 +360,9 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   bool is_class_scope() const { return scope_type_ == CLASS_SCOPE; }
 
   bool inner_scope_calls_eval() const { return inner_scope_calls_eval_; }
+  bool private_name_lookup_skips_outer_class() const {
+    return private_name_lookup_skips_outer_class_;
+  }
   bool IsAsmModule() const;
   // Returns true if this scope or any inner scopes that might be eagerly
   // compiled are asm modules.
@@ -464,10 +467,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // sloppy eval call. One if this->sloppy_eval_can_extend_vars().
   int ContextChainLengthUntilOutermostSloppyEval() const;
 
-  // Find the closest class scope in the current scope and outer scopes. If no
-  // class scope is found, nullptr will be returned.
-  ClassScope* GetClassScope();
-
   // Find the first function, script, eval or (declaration) block scope. This is
   // the scope where var declarations will be hoisted to in the implementation.
   DeclarationScope* GetDeclarationScope();
@@ -712,7 +711,8 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // This scope's declarations might not be executed in order (e.g., switch).
   bool scope_nonlinear_ : 1;
   bool is_hidden_ : 1;
-  // Temporary workaround that allows masking of 'this' in debug-evalute scopes.
+  // Temporary workaround that allows masking of 'this' in debug-evaluate
+  // scopes.
   bool is_debug_evaluate_scope_ : 1;
 
   // True if one of the inner scopes or the scope itself calls eval.
@@ -722,6 +722,11 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // True if it holds 'var' declarations.
   bool is_declaration_scope_ : 1;
 
+  // True if the outer scope is a class scope and should be skipped when
+  // resolving private names, i.e. if the scope is in a class heritage
+  // expression.
+  bool private_name_lookup_skips_outer_class_ : 1;
+
   bool must_use_preparsed_scope_data_ : 1;
 };
 
@@ -1080,6 +1085,11 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
     GetReceiverScope()->receiver()->ForceContextAllocation();
   }
 
+  bool needs_private_name_context_chain_recalc() const {
+    return needs_private_name_context_chain_recalc_;
+  }
+  void RecordNeedsPrivateNameContextChainRecalc();
+
  private:
   V8_INLINE void AllocateParameter(Variable* var, int index);
 
@@ -1097,6 +1107,12 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
 
   void SetDefaults();
 
+  // Recalculate the private name context chain from the existing skip bit in
+  // preparation for AllocateScopeInfos. Because the private name scope is
+  // implemented with a skip bit for scopes in heritage position, that bit may
+  // need to be recomputed due scopes that do not need contexts.
+  void RecalcPrivateNameContextChain();
+
   bool has_simple_parameters_ : 1;
   // This scope contains an "use asm" annotation.
   bool is_asm_module_ : 1;
@@ -1118,6 +1134,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   bool has_checked_syntax_ : 1;
   bool has_this_reference_ : 1;
   bool has_this_declaration_ : 1;
+  bool needs_private_name_context_chain_recalc_ : 1;
 
   // If the scope is a function scope, this is the function kind.
   const FunctionKind function_kind_;
@@ -1223,13 +1240,22 @@ class V8_EXPORT_PRIVATE ClassScope : public Scope {
   ClassScope(Zone* zone, AstValueFactory* ast_value_factory,
              Handle<ScopeInfo> scope_info);
 
+  struct HeritageParsingScope {
+    explicit HeritageParsingScope(ClassScope* class_scope)
+        : class_scope_(class_scope) {
+      class_scope_->SetIsParsingHeritage(true);
+    }
+    ~HeritageParsingScope() { class_scope_->SetIsParsingHeritage(false); }
+
+   private:
+    ClassScope* class_scope_;
+  };
+
   // Declare a private name in the private name map and add it to the
   // local variables of this scope.
   Variable* DeclarePrivateName(const AstRawString* name, VariableMode mode,
                                bool* was_added);
 
-  void AddUnresolvedPrivateName(VariableProxy* proxy);
-
   // Try resolving all unresolved private names found in the current scope.
   // Called from DeclarationScope::AllocateVariables() when reparsing a
   // method to generate code or when eval() is called to access private names.
@@ -1261,11 +1287,17 @@ class V8_EXPORT_PRIVATE ClassScope : public Scope {
   Variable* DeclareBrandVariable(AstValueFactory* ast_value_factory,
                                  int class_token_pos);
   Variable* brand() {
-    return rare_data_ == nullptr ? nullptr : rare_data_->brand;
+    return GetRareData() == nullptr ? nullptr : GetRareData()->brand;
+  }
+
+  V8_INLINE bool IsParsingHeritage() {
+    return rare_data_and_is_parsing_heritage_.GetPayload();
   }
 
  private:
   friend class Scope;
+  friend class PrivateNameScopeIterator;
+
   // Find the private name declared in the private name map first,
   // if it cannot be found there, try scope info if there is any.
   // Returns nullptr if it cannot be found.
@@ -1283,14 +1315,44 @@ class V8_EXPORT_PRIVATE ClassScope : public Scope {
     Variable* brand = nullptr;
   };
 
+  V8_INLINE RareData* GetRareData() {
+    return rare_data_and_is_parsing_heritage_.GetPointer();
+  }
   V8_INLINE RareData* EnsureRareData() {
-    if (rare_data_ == nullptr) {
-      rare_data_ = new (zone_) RareData(zone_);
+    if (GetRareData() == nullptr) {
+      rare_data_and_is_parsing_heritage_.SetPointer(new (zone_)
+                                                        RareData(zone_));
     }
-    return rare_data_;
+    return GetRareData();
+  }
+  V8_INLINE void SetIsParsingHeritage(bool v) {
+    rare_data_and_is_parsing_heritage_.SetPayload(v);
   }
 
-  RareData* rare_data_ = nullptr;
+  PointerWithPayload<RareData, bool, 1> rare_data_and_is_parsing_heritage_;
+};
+
+// Iterate over the private name scope chain. The iteration proceeds from the
+// innermost private name scope outwards.
+class PrivateNameScopeIterator {
+ public:
+  explicit PrivateNameScopeIterator(Scope* start);
+
+  bool Done() const { return current_scope_ == nullptr; }
+  void Next();
+
+  // Add an unresolved private name to the current scope.
+  void AddUnresolvedPrivateName(VariableProxy* proxy);
+
+  ClassScope* GetScope() const {
+    DCHECK(!Done());
+    return current_scope_->AsClassScope();
+  }
+
+ private:
+  bool skipped_any_scopes_ = false;
+  Scope* start_scope_;
+  Scope* current_scope_;
 };
 
 }  // namespace internal

src/objects/objects.cc

@@ -5354,6 +5354,8 @@ void SharedFunctionInfo::InitFromFunctionLiteral(
     Scope* outer_scope = lit->scope()->GetOuterScopeWithContext();
     if (outer_scope) {
       shared_info->set_outer_scope_info(*outer_scope->scope_info());
+      shared_info->set_private_name_lookup_skips_outer_class(
+          lit->scope()->private_name_lookup_skips_outer_class());
     }
   }
 

src/objects/scope-info.cc

@@ -146,6 +146,7 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone, Scope* scope,
           ? scope->AsDeclarationScope()->num_parameters()
           : 0;
   const bool has_outer_scope_info = !outer_scope.is_null();
+
   const int length = kVariablePartIndex + 2 * context_local_count +
                      (has_receiver ? 1 : 0) +
                      (has_function_name ? kFunctionNameEntries : 0) +
@@ -196,7 +197,9 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone, Scope* scope,
         HasOuterScopeInfoField::encode(has_outer_scope_info) |
         IsDebugEvaluateScopeField::encode(scope->is_debug_evaluate_scope()) |
         ForceContextAllocationField::encode(
-            scope->ForceContextForLanguageMode());
+            scope->ForceContextForLanguageMode()) |
+        PrivateNameLookupSkipsOuterClassField::encode(
+            scope->private_name_lookup_skips_outer_class());
     scope_info.SetFlags(flags);
 
     scope_info.SetParameterCount(parameter_count);
@@ -366,7 +369,9 @@ Handle<ScopeInfo> ScopeInfo::CreateForWithScope(
       IsAsmModuleField::encode(false) | HasSimpleParametersField::encode(true) |
       FunctionKindField::encode(kNormalFunction) |
       HasOuterScopeInfoField::encode(has_outer_scope_info) |
-      IsDebugEvaluateScopeField::encode(false);
+      IsDebugEvaluateScopeField::encode(false) |
+      ForceContextAllocationField::encode(false) |
+      PrivateNameLookupSkipsOuterClassField::encode(false);
   scope_info->SetFlags(flags);
 
   scope_info->SetParameterCount(0);
@@ -431,7 +436,9 @@ Handle<ScopeInfo> ScopeInfo::CreateForBootstrapping(Isolate* isolate,
       IsAsmModuleField::encode(false) | HasSimpleParametersField::encode(true) |
       FunctionKindField::encode(FunctionKind::kNormalFunction) |
       HasOuterScopeInfoField::encode(false) |
-      IsDebugEvaluateScopeField::encode(false);
+      IsDebugEvaluateScopeField::encode(false) |
+      ForceContextAllocationField::encode(false) |
+      PrivateNameLookupSkipsOuterClassField::encode(false);
   scope_info->SetFlags(flags);
   scope_info->SetParameterCount(parameter_count);
   scope_info->SetContextLocalCount(context_local_count);
@@ -608,6 +615,11 @@ void ScopeInfo::SetIsDebugEvaluateScope() {
   }
 }
 
+bool ScopeInfo::PrivateNameLookupSkipsOuterClass() const {
+  if (length() == 0) return false;
+  return PrivateNameLookupSkipsOuterClassField::decode(Flags());
+}
+
 bool ScopeInfo::HasContext() const { return ContextLength() > 0; }
 
 Object ScopeInfo::FunctionName() const {

src/objects/scope-info.h

@@ -176,6 +176,10 @@ class ScopeInfo : public FixedArray {
   // Return the outer ScopeInfo if present.
   ScopeInfo OuterScopeInfo() const;
 
+  // Returns true if this ScopeInfo was created for a scope that skips the
+  // closest outer class when resolving private names.
+  bool PrivateNameLookupSkipsOuterClass() const;
+
 #ifdef DEBUG
   bool Equals(ScopeInfo other) const;
 #endif
@@ -240,6 +244,8 @@ class ScopeInfo : public FixedArray {
   using HasOuterScopeInfoField = FunctionKindField::Next<bool, 1>;
   using IsDebugEvaluateScopeField = HasOuterScopeInfoField::Next<bool, 1>;
   using ForceContextAllocationField = IsDebugEvaluateScopeField::Next<bool, 1>;
+  using PrivateNameLookupSkipsOuterClassField =
+      ForceContextAllocationField::Next<bool, 1>;
 
   STATIC_ASSERT(kLastFunctionKind <= FunctionKindField::kMax);
 

src/objects/shared-function-info-inl.h

@@ -212,6 +212,9 @@ BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags,
 BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags,
                     is_safe_to_skip_arguments_adaptor,
                     SharedFunctionInfo::IsSafeToSkipArgumentsAdaptorBit)
+BIT_FIELD_ACCESSORS(SharedFunctionInfo, flags,
+                    private_name_lookup_skips_outer_class,
+                    SharedFunctionInfo::PrivateNameLookupSkipsOuterClassBit)
 
 bool SharedFunctionInfo::optimization_disabled() const {
   return disable_optimization_reason() != BailoutReason::kNoReason;

src/objects/shared-function-info.h

@@ -448,6 +448,10 @@ class SharedFunctionInfo : public HeapObject {
   // Indicates that the function has been reported for binary code coverage.
   DECL_BOOLEAN_ACCESSORS(has_reported_binary_coverage)
 
+  // Indicates that the private name lookups inside the function skips the
+  // closest outer class scope.
+  DECL_BOOLEAN_ACCESSORS(private_name_lookup_skips_outer_class)
+
   inline FunctionKind kind() const;
 
   // Defines the index in a native context of closure's map instantiated using
@@ -649,7 +653,8 @@ class SharedFunctionInfo : public HeapObject {
   V(HasReportedBinaryCoverageBit, bool, 1, _)                \
   V(IsTopLevelBit, bool, 1, _)                               \
   V(IsOneshotIIFEOrPropertiesAreFinalBit, bool, 1, _)        \
-  V(IsSafeToSkipArgumentsAdaptorBit, bool, 1, _)
+  V(IsSafeToSkipArgumentsAdaptorBit, bool, 1, _)             \
+  V(PrivateNameLookupSkipsOuterClassBit, bool, 1, _)
   DEFINE_BIT_FIELDS(FLAGS_BIT_FIELDS)
 #undef FLAGS_BIT_FIELDS
 

src/parsing/parser-base.h

@@ -1577,16 +1577,17 @@ ParserBase<Impl>::ParsePropertyOrPrivatePropertyName() {
     //
     // Here, we check if this is a new private name reference in a top
     // level function and throw an error if so.
-    ClassScope* class_scope = scope()->GetClassScope();
+    PrivateNameScopeIterator private_name_scope_iter(scope());
     // Parse the identifier so that we can display it in the error message
     name = impl()->GetIdentifier();
-    if (class_scope == nullptr) {
+    if (private_name_scope_iter.Done()) {
       impl()->ReportMessageAt(Scanner::Location(pos, pos + 1),
                               MessageTemplate::kInvalidPrivateFieldResolution,
                               impl()->GetRawNameFromIdentifier(name));
       return impl()->FailureExpression();
     }
-    key = impl()->ExpressionFromPrivateName(class_scope, name, pos);
+    key =
+        impl()->ExpressionFromPrivateName(&private_name_scope_iter, name, pos);
   } else {
     ReportUnexpectedToken(next);
     return impl()->FailureExpression();
@@ -4360,6 +4361,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
 
   scope()->set_start_position(end_position());
   if (Check(Token::EXTENDS)) {
+    ClassScope::HeritageParsingScope heritage(class_scope);
     FuncNameInferrerState fni_state(&fni_);
     ExpressionParsingScope scope(impl());
     class_info.extends = ParseLeftHandSideExpression();

src/parsing/parser.cc

@@ -705,8 +705,17 @@ FunctionLiteral* Parser::ParseFunction(Isolate* isolate, ParseInfo* info,
   info->set_function_name(ast_value_factory()->GetString(name));
   scanner_.Initialize();
 
-  FunctionLiteral* result =
-      DoParseFunction(isolate, info, info->function_name());
+  FunctionLiteral* result;
+  if (V8_UNLIKELY(shared_info->private_name_lookup_skips_outer_class() &&
+                  original_scope_->is_class_scope())) {
+    // If the function skips the outer class and the outer scope is a class, the
+    // function is in heritage position. Otherwise the function scope's skip bit
+    // will be correctly inherited from the outer scope.
+    ClassScope::HeritageParsingScope heritage(original_scope_->AsClassScope());
+    result = DoParseFunction(isolate, info, info->function_name());
+  } else {
+    result = DoParseFunction(isolate, info, info->function_name());
+  }
   MaybeResetCharacterStream(info, result);
   MaybeProcessSourceRanges(info, result, stack_limit_);
   if (result != nullptr) {
@@ -2493,10 +2502,10 @@ bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
   bookmark.Set(function_scope->start_position());
 
   UnresolvedList::Iterator unresolved_private_tail;
-  ClassScope* closest_class_scope = function_scope->GetClassScope();
-  if (closest_class_scope != nullptr) {
+  PrivateNameScopeIterator private_name_scope_iter(function_scope);
+  if (!private_name_scope_iter.Done()) {
     unresolved_private_tail =
-        closest_class_scope->GetUnresolvedPrivateNameTail();
+        private_name_scope_iter.GetScope()->GetUnresolvedPrivateNameTail();
   }
 
   // With no cached data, we partially parse the function, without building an
@@ -2520,8 +2529,8 @@ bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
     // the state before preparsing. The caller may then fully parse the function
     // to identify the actual error.
     bookmark.Apply();
-    if (closest_class_scope != nullptr) {
-      closest_class_scope->ResetUnresolvedPrivateNameTail(
+    if (!private_name_scope_iter.Done()) {
+      private_name_scope_iter.GetScope()->ResetUnresolvedPrivateNameTail(
           unresolved_private_tail);
     }
     function_scope->ResetAfterPreparsing(ast_value_factory_, true);
@@ -2542,8 +2551,8 @@ bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
     *num_parameters = logger->num_parameters();
     *function_length = logger->function_length();
     SkipFunctionLiterals(logger->num_inner_functions());
-    if (closest_class_scope != nullptr) {
-      closest_class_scope->MigrateUnresolvedPrivateNameTail(
+    if (!private_name_scope_iter.Done()) {
+      private_name_scope_iter.GetScope()->MigrateUnresolvedPrivateNameTail(
           factory(), unresolved_private_tail);
     }
     function_scope->AnalyzePartially(this, factory());

src/parsing/parser.h

@@ -778,12 +778,12 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   Expression* ExpressionFromLiteral(Token::Value token, int pos);
 
-  V8_INLINE VariableProxy* ExpressionFromPrivateName(ClassScope* class_scope,
-                                                     const AstRawString* name,
-                                                     int start_position) {
+  V8_INLINE VariableProxy* ExpressionFromPrivateName(
+      PrivateNameScopeIterator* private_name_scope, const AstRawString* name,
+      int start_position) {
     VariableProxy* proxy = factory()->ast_node_factory()->NewVariableProxy(
         name, NORMAL_VARIABLE, start_position);
-    class_scope->AddUnresolvedPrivateName(proxy);
+    private_name_scope->AddUnresolvedPrivateName(proxy);
     return proxy;
   }
 

src/parsing/preparse-data.cc

@@ -24,6 +24,8 @@ namespace {
 using ScopeSloppyEvalCanExtendVarsField = BitField8<bool, 0, 1>;
 using InnerScopeCallsEvalField =
     ScopeSloppyEvalCanExtendVarsField::Next<bool, 1>;
+using NeedsPrivateNameContextChainRecalcField =
+    InnerScopeCallsEvalField::Next<bool, 1>;
 
 using VariableMaybeAssignedField = BitField8<bool, 0, 1>;
 using VariableContextAllocatedField = VariableMaybeAssignedField::Next<bool, 1>;
@@ -322,7 +324,7 @@ void PreparseDataBuilder::SaveScopeAllocationData(DeclarationScope* scope,
     if (SaveDataForSkippableFunction(builder)) num_inner_with_data_++;
   }
 
-  // Don't save imcoplete scope information when bailed out.
+  // Don't save incomplete scope information when bailed out.
   if (!bailed_out_) {
 #ifdef DEBUG
   // function data items, kSkippableMinFunctionDataSize each.
@@ -352,13 +354,17 @@ void PreparseDataBuilder::SaveDataForScope(Scope* scope) {
   byte_data_.WriteUint8(scope->scope_type());
 #endif
 
-  uint8_t eval =
+  uint8_t eval_and_private_recalc =
       ScopeSloppyEvalCanExtendVarsField::encode(
           scope->is_declaration_scope() &&
           scope->AsDeclarationScope()->sloppy_eval_can_extend_vars()) |
-      InnerScopeCallsEvalField::encode(scope->inner_scope_calls_eval());
+      InnerScopeCallsEvalField::encode(scope->inner_scope_calls_eval()) |
+      NeedsPrivateNameContextChainRecalcField::encode(
+          scope->is_function_scope() &&
+          scope->AsDeclarationScope()
+              ->needs_private_name_context_chain_recalc());
   byte_data_.Reserve(kUint8Size);
-  byte_data_.WriteUint8(eval);
+  byte_data_.WriteUint8(eval_and_private_recalc);
 
   if (scope->is_function_scope()) {
     Variable* function = scope->AsDeclarationScope()->function_var();
@@ -599,9 +605,17 @@ void BaseConsumedPreparseData<Data>::RestoreDataForScope(Scope* scope) {
   DCHECK_EQ(scope_data_->ReadUint8(), scope->scope_type());
 
   CHECK(scope_data_->HasRemainingBytes(ByteData::kUint8Size));
-  uint32_t eval = scope_data_->ReadUint8();
-  if (ScopeSloppyEvalCanExtendVarsField::decode(eval)) scope->RecordEvalCall();
-  if (InnerScopeCallsEvalField::decode(eval)) scope->RecordInnerScopeEvalCall();
+  uint32_t eval_and_private_recalc = scope_data_->ReadUint8();
+  if (ScopeSloppyEvalCanExtendVarsField::decode(eval_and_private_recalc)) {
+    scope->RecordEvalCall();
+  }
+  if (InnerScopeCallsEvalField::decode(eval_and_private_recalc)) {
+    scope->RecordInnerScopeEvalCall();
+  }
+  if (NeedsPrivateNameContextChainRecalcField::decode(
+          eval_and_private_recalc)) {
+    scope->AsDeclarationScope()->RecordNeedsPrivateNameContextChainRecalc();
+  }
 
   if (scope->is_function_scope()) {
     Variable* function = scope->AsDeclarationScope()->function_var();

src/parsing/preparser.h

@@ -1595,12 +1595,12 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserExpression::StringLiteral();
   }
 
-  PreParserExpression ExpressionFromPrivateName(ClassScope* class_scope,
-                                                const PreParserIdentifier& name,
-                                                int start_position) {
+  PreParserExpression ExpressionFromPrivateName(
+      PrivateNameScopeIterator* private_name_scope,
+      const PreParserIdentifier& name, int start_position) {
     VariableProxy* proxy = factory()->ast_node_factory()->NewVariableProxy(
         name.string_, NORMAL_VARIABLE, start_position);
-    class_scope->AddUnresolvedPrivateName(proxy);
+    private_name_scope->AddUnresolvedPrivateName(proxy);
     return PreParserExpression::FromIdentifier(name);
   }
 

test/mjsunit/harmony/private-name-scopes.js

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+{
+    let heritageFn;
+    class O {
+      #f = "O.#f";
+      static C = class C extends (heritageFn = function () {
+                   return class D {
+                     exfil(obj) { return obj.#f; }
+                     exfilEval(obj) { return eval("obj.#f"); }
+                   };
+                 }) {
+                   #f = "C.#f";
+                 };
+    }
+
+    const o = new O;
+    const c = new O.C;
+    const D = heritageFn();
+    const d = new D;
+    assertEquals(d.exfil(o), "O.#f");
+    assertEquals(d.exfilEval(o), "O.#f");
+    assertThrows(() => d.exfil(c), TypeError);
+    assertThrows(() => d.exfilEval(c), TypeError);
+}
+
+// Early errors
+
+assertThrows(() => eval("new class extends " +
+                        "(class { m() { let x = this.#f; } }) " +
+                        "{ #f }"), SyntaxError);
+
+assertThrows(() => eval("new class extends this.#foo { #foo }"), SyntaxError);
+
+// Runtime errors
+
+{
+  // Test private name context chain recalc.
+  let heritageFn;
+  class O {
+    #f = "O.#f";
+    static C = class C extends (heritageFn = function () {
+                 return class D { exfil(obj) { return obj.#f; } }
+               }) {
+                 #f = "C.#f";
+               };
+  }
+
+  const o = new O;
+  const c = new O.C;
+  const D = heritageFn();
+  const d = new D;
+  assertEquals(d.exfil(o), "O.#f");
+  assertThrows(() => d.exfil(c), TypeError);
+}
+
+{
+  // Test private name context chain recalc with nested closures with context.
+  let heritageFn;
+  class O {
+    #f = "O.#f";
+    static C = class C extends (heritageFn = function () {
+                 let forceContext = 1;
+                 return () => {
+                   assertEquals(forceContext, 1);
+                   return class D { exfil(obj) { return obj.#f; } }
+                 };
+               }) {
+                 #f = "C.#f";
+               };
+  }
+
+  const o = new O;
+  const c = new O.C;
+  const D = heritageFn()();
+  const d = new D;
+  assertEquals(d.exfil(o), "O.#f");
+  assertThrows(() => d.exfil(c), TypeError);
+}
+
+{
+  // Test private name context chain recalc where skipped class has no context.
+  let heritageFn;
+  class O {
+    #f = "O.#f";
+    static C = class C0 extends (class C1 extends (heritageFn = function (obj) {
+                 if (obj) { return obj.#f; }
+               }) {}) {
+                 #f = "C0.#f"
+               }
+  }
+
+  const o = new O;
+  const c = new O.C;
+  assertEquals(heritageFn(o), "O.#f");
+  assertThrows(() => heritageFn(c), TypeError);
+}
+
+{
+  // Test private name context chain recalc where skipping function has no
+  // context.
+  let heritageFn;
+  class O {
+    #f = "O.#f";
+    static C = class C extends (heritageFn = function () {
+                 return (obj) => { return obj.#f; }
+               }) {
+                 #f = "C.#f";
+               }
+  }
+
+  const o = new O;
+  const c = new O.C;
+  assertEquals(heritageFn()(o), "O.#f");
+  assertThrows(() => heritageFn()(c), TypeError);
+}
+
+{
+  // Test private name context chain recalc where neither skipped class nor
+  // skipping function has contexts.
+  let heritageFn;
+  class O {
+    #f = "O.#f";
+    static C = class C0 extends (class C1 extends (heritageFn = function () {
+                 return (obj) => { return obj.#f; }
+               }) {}) {
+                 #f = "C0.#f";
+               }
+  }
+
+  const o = new O;
+  const c = new O.C;
+  assertEquals(heritageFn()(o), "O.#f");
+  assertThrows(() => heritageFn()(c), TypeError);
+}

test/test262/test262.status

@@ -484,16 +484,6 @@
   # https://bugs.chromium.org/p/v8/issues/detail?id=9049
   'language/comments/hashbang/use-strict': [SKIP],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=9229
-  'language/expressions/class/elements/syntax/early-errors/grammar-private-environment-on-class-heritage': [FAIL],
-  'language/expressions/class/elements/syntax/early-errors/grammar-private-environment-on-class-heritage-chained-usage': [FAIL],
-  'language/expressions/class/elements/syntax/early-errors/grammar-private-environment-on-class-heritage-function-expression': [FAIL],
-  'language/expressions/class/elements/syntax/early-errors/grammar-private-environment-on-class-heritage-recursive': [FAIL],
-  'language/statements/class/elements/syntax/early-errors/grammar-private-environment-on-class-heritage': [FAIL],
-  'language/statements/class/elements/syntax/early-errors/grammar-private-environment-on-class-heritage-chained-usage': [FAIL],
-  'language/statements/class/elements/syntax/early-errors/grammar-private-environment-on-class-heritage-function-expression': [FAIL],
-  'language/statements/class/elements/syntax/early-errors/grammar-private-environment-on-class-heritage-recursive': [FAIL],
-
   # https://bugs.chromium.org/p/v8/issues/detail?id=8179
   #
   # These tests require exception handling support which is currently