[ast] Refactor resolution of private names

- Add a new ClassScope for block scopes created for classes.
- Add a VariableMap in the class scope for private name resolution,
  and a separate UnresolvedList for private names that will be resolved
  only using ClassScopes. These are stored in RareData and will only be
  allocated when there are private name declaration or access in the
  class.

Design: https://docs.google.com/document/d/1l-D70uaHzXU8QVgQZ3ACikb3FLO6LTAfQVdGDXsh5mw/edit?usp=sharing

TBR: hpayer@chromium.org
Bug: v8:8330
Bug: v8:7468
Change-Id: I78191fc075f7f195f6c56c959773c382346cce8a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1488271
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60726}

src/ast/scopes.h

@@ -30,6 +30,9 @@ class Statement;
 class StringSet;
 class VariableProxy;
 
+typedef base::ThreadedList<VariableProxy, VariableProxy::UnresolvedNext>
+    UnresolvedList;
+
 // A hash map to support fast variable declaration and lookup.
 class VariableMap : public ZoneHashMap {
  public:
@@ -79,13 +82,12 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   }
 #endif
 
-  typedef base::ThreadedList<VariableProxy, VariableProxy::UnresolvedNext>
-      UnresolvedList;
-
   DeclarationScope* AsDeclarationScope();
   const DeclarationScope* AsDeclarationScope() const;
   ModuleScope* AsModuleScope();
   const ModuleScope* AsModuleScope() const;
+  ClassScope* AsClassScope();
+  const ClassScope* AsClassScope() const;
 
   class Snapshot final {
    public:
@@ -347,9 +349,12 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   bool is_module_scope() const { return scope_type_ == MODULE_SCOPE; }
   bool is_script_scope() const { return scope_type_ == SCRIPT_SCOPE; }
   bool is_catch_scope() const { return scope_type_ == CATCH_SCOPE; }
-  bool is_block_scope() const { return scope_type_ == BLOCK_SCOPE; }
+  bool is_block_scope() const {
+    return scope_type_ == BLOCK_SCOPE || scope_type_ == CLASS_SCOPE;
+  }
   bool is_with_scope() const { return scope_type_ == WITH_SCOPE; }
   bool is_declaration_scope() const { return is_declaration_scope_; }
+  bool is_class_scope() const { return scope_type_ == CLASS_SCOPE; }
 
   bool inner_scope_calls_eval() const { return inner_scope_calls_eval_; }
   bool IsAsmModule() const;
@@ -456,6 +461,10 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // sloppy eval call. One if this->calls_sloppy_eval().
   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();
@@ -589,11 +598,10 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   static Variable* LookupSloppyEval(VariableProxy* proxy, Scope* scope,
                                     Scope* outer_scope_end, Scope* entry_point,
                                     bool force_context_allocation);
-  static bool ResolvePreparsedVariable(VariableProxy* proxy, Scope* scope,
+  static void ResolvePreparsedVariable(VariableProxy* proxy, Scope* scope,
                                        Scope* end);
   void ResolveTo(ParseInfo* info, VariableProxy* proxy, Variable* var);
-  V8_WARN_UNUSED_RESULT bool ResolveVariable(ParseInfo* info,
-                                             VariableProxy* proxy);
+  void ResolveVariable(ParseInfo* info, VariableProxy* proxy);
   V8_WARN_UNUSED_RESULT bool ResolveVariablesRecursively(ParseInfo* info);
 
   // Finds free variables of this scope. This mutates the unresolved variables
@@ -638,6 +646,7 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   void SetDefaults();
 
   friend class DeclarationScope;
+  friend class ClassScope;
   friend class ScopeTestHelper;
 
   Zone* zone_;
@@ -1156,6 +1165,75 @@ class ModuleScope final : public DeclarationScope {
   ModuleDescriptor* const module_descriptor_;
 };
 
+class V8_EXPORT_PRIVATE ClassScope : public Scope {
+ public:
+  ClassScope(Zone* zone, Scope* outer_scope);
+  // Deserialization.
+  ClassScope(Zone* zone, Handle<ScopeInfo> scope_info);
+
+  // Declare a private name in the private name map and add it to the
+  // local variables of this scope.
+  Variable* DeclarePrivateName(const AstRawString* name, 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.
+  // If there are any private names that cannot be resolved, returns false.
+  V8_WARN_UNUSED_RESULT bool ResolvePrivateNames(ParseInfo* info);
+
+  // Called after the entire class literal is parsed.
+  // - If we are certain a private name cannot be resolve, return that
+  //   variable proxy.
+  // - If we find the private name in the scope chain, return nullptr.
+  //   If the name is found in the current class scope, resolve it
+  //   immediately.
+  // - If we are not sure if the private name can be resolved or not yet,
+  //   return nullptr.
+  VariableProxy* ResolvePrivateNamesPartially();
+
+  // Get the current tail of unresolved private names to be used to
+  // reset the tail.
+  UnresolvedList::Iterator GetUnresolvedPrivateNameTail();
+
+  // Reset the tail of unresolved private names, discard everything
+  // between the tail passed into this method and the current tail.
+  void ResetUnresolvedPrivateNameTail(UnresolvedList::Iterator tail);
+
+  // Migrate private names added between the tail passed into this method
+  // and the current tail.
+  void MigrateUnresolvedPrivateNameTail(AstNodeFactory* ast_node_factory,
+                                        UnresolvedList::Iterator tail);
+
+ private:
+  friend class Scope;
+  // 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.
+  Variable* LookupPrivateName(VariableProxy* proxy);
+  // Lookup a private name from the local private name map of the current
+  // scope.
+  Variable* LookupLocalPrivateName(const AstRawString* name);
+  // Lookup a private name from the scope info of the current scope.
+  Variable* LookupPrivateNameInScopeInfo(const AstRawString* name);
+
+  struct RareData : public ZoneObject {
+    explicit RareData(Zone* zone) : private_name_map(zone) {}
+    UnresolvedList unresolved_private_names;
+    VariableMap private_name_map;
+  };
+
+  V8_INLINE RareData* EnsureRareData() {
+    if (rare_data_ == nullptr) {
+      rare_data_ = new (zone_) RareData(zone_);
+    }
+    return rare_data_;
+  }
+
+  RareData* rare_data_ = nullptr;
+};
+
 }  // namespace internal
 }  // namespace v8
 

src/debug/debug-scopes.cc

@@ -329,6 +329,7 @@ ScopeIterator::ScopeType ScopeIterator::Type() const {
         DCHECK(context_->IsCatchContext());
         return ScopeTypeCatch;
       case BLOCK_SCOPE:
+      case CLASS_SCOPE:
         DCHECK_IMPLIES(current_scope_->NeedsContext(),
                        context_->IsBlockContext());
         return ScopeTypeBlock;

src/globals.h

@@ -547,6 +547,7 @@ constexpr uint32_t kQuietNaNHighBitsMask = 0xfff << (51 - 32);
 class AccessorInfo;
 class Arguments;
 class Assembler;
+class ClassScope;
 class Code;
 class CodeSpace;
 class Context;
@@ -989,6 +990,7 @@ inline std::ostream& operator<<(std::ostream& os, CreateArgumentsType type) {
 }
 
 enum ScopeType : uint8_t {
+  CLASS_SCOPE,     // The scope introduced by a class.
   EVAL_SCOPE,      // The top-level scope for an eval source.
   FUNCTION_SCOPE,  // The top-level scope for a function.
   MODULE_SCOPE,    // The scope introduced by a module literal
@@ -1012,6 +1014,8 @@ inline std::ostream& operator<<(std::ostream& os, ScopeType type) {
       return os << "CATCH_SCOPE";
     case ScopeType::BLOCK_SCOPE:
       return os << "BLOCK_SCOPE";
+    case ScopeType::CLASS_SCOPE:
+      return os << "CLASS_SCOPE";
     case ScopeType::WITH_SCOPE:
       return os << "WITH_SCOPE";
   }

src/heap/factory.cc

@@ -1604,7 +1604,8 @@ Handle<Context> Factory::NewWithContext(Handle<Context> previous,
 
 Handle<Context> Factory::NewBlockContext(Handle<Context> previous,
                                          Handle<ScopeInfo> scope_info) {
-  DCHECK_EQ(scope_info->scope_type(), BLOCK_SCOPE);
+  DCHECK_IMPLIES(scope_info->scope_type() != BLOCK_SCOPE,
+                 scope_info->scope_type() == CLASS_SCOPE);
   int variadic_part_length = scope_info->ContextLength();
   Handle<Context> context = NewContext(
       RootIndex::kBlockContextMap, Context::SizeFor(variadic_part_length),

src/objects/scope-info.cc

@@ -508,14 +508,14 @@ int ScopeInfo::ContextLength() const {
     bool function_name_context_slot =
         FunctionVariableField::decode(Flags()) == CONTEXT;
     bool force_context = ForceContextAllocationField::decode(Flags());
-    bool has_context = context_locals > 0 || force_context ||
-                       function_name_context_slot ||
-                       scope_type() == WITH_SCOPE ||
-                       (scope_type() == BLOCK_SCOPE && CallsSloppyEval() &&
-                        is_declaration_scope()) ||
-                       (scope_type() == FUNCTION_SCOPE && CallsSloppyEval()) ||
-                       (scope_type() == FUNCTION_SCOPE && IsAsmModule()) ||
-                       scope_type() == MODULE_SCOPE;
+    bool has_context =
+        context_locals > 0 || force_context || function_name_context_slot ||
+        scope_type() == WITH_SCOPE || scope_type() == CLASS_SCOPE ||
+        (scope_type() == BLOCK_SCOPE && CallsSloppyEval() &&
+         is_declaration_scope()) ||
+        (scope_type() == FUNCTION_SCOPE && CallsSloppyEval()) ||
+        (scope_type() == FUNCTION_SCOPE && IsAsmModule()) ||
+        scope_type() == MODULE_SCOPE;
 
     if (has_context) {
       return Context::MIN_CONTEXT_SLOTS + context_locals +

src/objects/scope-info.h

@@ -47,6 +47,9 @@ class ScopeInfo : public FixedArray {
   // True if this scope is a (var) declaration scope.
   bool is_declaration_scope() const;
 
+  // True if this scope is a class scope.
+  bool is_class_scope() const;
+
   // Does this scope make a sloppy eval call?
   bool CallsSloppyEval() const;
 

src/parsing/parser-base.h

@@ -656,6 +656,10 @@ class ParserBase {
     return new (zone()) DeclarationScope(zone(), parent, EVAL_SCOPE);
   }
 
+  ClassScope* NewClassScope(Scope* parent) const {
+    return new (zone()) ClassScope(zone(), parent);
+  }
+
   Scope* NewScope(ScopeType scope_type) const {
     return NewScopeWithParent(scope(), scope_type);
   }
@@ -1573,14 +1577,14 @@ ParserBase<Impl>::ParsePropertyOrPrivatePropertyName() {
     //
     // Bug(v8:7468): This hack will go away once we refactor private
     // name resolution to happen independently from scope resolution.
-    if (scope()->scope_type() == FUNCTION_SCOPE &&
-        scope()->outer_scope() != nullptr &&
-        scope()->outer_scope()->scope_type() == SCRIPT_SCOPE) {
+    ClassScope* class_scope = scope()->GetClassScope();
+    if (class_scope == nullptr) {
       ReportMessage(MessageTemplate::kInvalidPrivateFieldResolution);
+      return impl()->FailureExpression();
     }
 
     name = impl()->GetIdentifier();
-    key = impl()->ExpressionFromIdentifier(name, pos, InferName::kNo);
+    key = impl()->ExpressionFromPrivateName(class_scope, name, pos);
   } else {
     ReportUnexpectedToken(next);
     return impl()->FailureExpression();
@@ -4232,8 +4236,8 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
     }
   }
 
-  Scope* block_scope = NewScope(BLOCK_SCOPE);
-  BlockState block_state(&scope_, block_scope);
+  ClassScope* class_scope = NewClassScope(scope());
+  BlockState block_state(&scope_, class_scope);
   RaiseLanguageMode(LanguageMode::kStrict);
 
   ClassInfo class_info(this);
@@ -4278,19 +4282,30 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
         class_info.computed_field_count++;
       }
 
-      impl()->DeclareClassField(property, prop_info.name, prop_info.is_static,
-                                prop_info.is_computed_name,
+      impl()->DeclareClassField(class_scope, property, prop_info.name,
+                                prop_info.is_static, prop_info.is_computed_name,
                                 prop_info.is_private, &class_info);
     } else {
-      impl()->DeclareClassProperty(name, property, is_constructor, &class_info);
+      impl()->DeclareClassProperty(class_scope, name, property, is_constructor,
+                                   &class_info);
     }
     impl()->InferFunctionName();
   }
 
   Expect(Token::RBRACE);
   int end_pos = end_position();
-  block_scope->set_end_position(end_pos);
-  return impl()->RewriteClassLiteral(block_scope, name, &class_info,
+  class_scope->set_end_position(end_pos);
+
+  VariableProxy* unresolvable = class_scope->ResolvePrivateNamesPartially();
+  if (unresolvable != nullptr) {
+    impl()->ReportMessageAt(Scanner::Location(unresolvable->position(),
+                                              unresolvable->position() + 1),
+                            MessageTemplate::kInvalidPrivateFieldResolution,
+                            unresolvable->raw_name(), kSyntaxError);
+    return impl()->FailureExpression();
+  }
+
+  return impl()->RewriteClassLiteral(class_scope, name, &class_info,
                                      class_token_pos, end_pos);
 }
 

src/parsing/parser.cc

@@ -2506,6 +2506,13 @@ bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
   Scanner::BookmarkScope bookmark(scanner());
   bookmark.Set(function_scope->start_position());
 
+  UnresolvedList::Iterator unresolved_private_tail;
+  ClassScope* closest_class_scope = function_scope->GetClassScope();
+  if (closest_class_scope != nullptr) {
+    unresolved_private_tail =
+        closest_class_scope->GetUnresolvedPrivateNameTail();
+  }
+
   // With no cached data, we partially parse the function, without building an
   // AST. This gathers the data needed to build a lazy function.
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"), "V8.PreParse");
@@ -2527,7 +2534,11 @@ 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();
-    function_scope->ResetAfterPreparsing(ast_value_factory(), true);
+    if (closest_class_scope != nullptr) {
+      closest_class_scope->ResetUnresolvedPrivateNameTail(
+          unresolved_private_tail);
+    }
+    function_scope->ResetAfterPreparsing(ast_value_factory_, true);
     pending_error_handler()->clear_unidentifiable_error();
     return false;
   } else if (pending_error_handler()->has_pending_error()) {
@@ -2544,6 +2555,10 @@ bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
         function_scope->end_position() - function_scope->start_position();
     *num_parameters = logger->num_parameters();
     SkipFunctionLiterals(logger->num_inner_functions());
+    if (closest_class_scope != nullptr) {
+      closest_class_scope->MigrateUnresolvedPrivateNameTail(
+          factory(), unresolved_private_tail);
+    }
     function_scope->AnalyzePartially(this, factory());
   }
 
@@ -2770,7 +2785,23 @@ Variable* Parser::CreateSyntheticContextVariable(const AstRawString* name) {
   return proxy->var();
 }
 
-void Parser::DeclareClassField(ClassLiteralProperty* property,
+Variable* Parser::CreatePrivateNameVariable(ClassScope* scope,
+                                            const AstRawString* name) {
+  DCHECK_NOT_NULL(name);
+  int begin = position();
+  int end = end_position();
+  bool was_added = false;
+  Variable* var = scope->DeclarePrivateName(name, &was_added);
+  if (!was_added) {
+    Scanner::Location loc(begin, end);
+    ReportMessageAt(loc, MessageTemplate::kVarRedeclaration, var->raw_name());
+  }
+  VariableProxy* proxy = factory()->NewVariableProxy(var, begin);
+  return proxy->var();
+}
+
+void Parser::DeclareClassField(ClassScope* scope,
+                               ClassLiteralProperty* property,
                                const AstRawString* property_name,
                                bool is_static, bool is_computed_name,
                                bool is_private, ClassInfo* class_info) {
@@ -2792,8 +2823,13 @@ void Parser::DeclareClassField(ClassLiteralProperty* property,
     property->set_computed_name_var(computed_name_var);
     class_info->properties->Add(property, zone());
   } else if (is_private) {
-    Variable* private_name_var = CreateSyntheticContextVariable(property_name);
-    private_name_var->set_initializer_position(property->value()->position());
+    Variable* private_name_var =
+        CreatePrivateNameVariable(scope, property_name);
+    int pos = property->value()->position();
+    if (pos == kNoSourcePosition) {
+      pos = property->key()->position();
+    }
+    private_name_var->set_initializer_position(pos);
     property->set_private_name_var(private_name_var);
     class_info->properties->Add(property, zone());
   }
@@ -2803,7 +2839,8 @@ void Parser::DeclareClassField(ClassLiteralProperty* property,
 // following fields of class_info, as appropriate:
 //   - constructor
 //   - properties
-void Parser::DeclareClassProperty(const AstRawString* class_name,
+void Parser::DeclareClassProperty(ClassScope* scope,
+                                  const AstRawString* class_name,
                                   ClassLiteralProperty* property,
                                   bool is_constructor, ClassInfo* class_info) {
   if (is_constructor) {
@@ -2845,12 +2882,12 @@ FunctionLiteral* Parser::CreateInitializerFunction(
 //   - properties
 //   - has_name_static_property
 //   - has_static_computed_names
-Expression* Parser::RewriteClassLiteral(Scope* block_scope,
+Expression* Parser::RewriteClassLiteral(ClassScope* block_scope,
                                         const AstRawString* name,
                                         ClassInfo* class_info, int pos,
                                         int end_pos) {
   DCHECK_NOT_NULL(block_scope);
-  DCHECK_EQ(block_scope->scope_type(), BLOCK_SCOPE);
+  DCHECK_EQ(block_scope->scope_type(), CLASS_SCOPE);
   DCHECK_EQ(block_scope->language_mode(), LanguageMode::kStrict);
 
   bool has_extends = class_info->extends != nullptr;

src/parsing/parser.h

@@ -344,6 +344,8 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
                              VariableKind kind, int beg_pos, int end_pos,
                              ZonePtrList<const AstRawString>* names);
   Variable* CreateSyntheticContextVariable(const AstRawString* synthetic_name);
+  Variable* CreatePrivateNameVariable(ClassScope* scope,
+                                      const AstRawString* name);
   FunctionLiteral* CreateInitializerFunction(
       const char* name, DeclarationScope* scope,
       ZonePtrList<ClassLiteral::Property>* fields);
@@ -358,14 +360,15 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
                           int class_token_pos, int end_pos);
   void DeclareClassVariable(const AstRawString* name, ClassInfo* class_info,
                             int class_token_pos);
-  void DeclareClassProperty(const AstRawString* class_name,
+  void DeclareClassProperty(ClassScope* scope, const AstRawString* class_name,
                             ClassLiteralProperty* property, bool is_constructor,
                             ClassInfo* class_info);
-  void DeclareClassField(ClassLiteralProperty* property,
+  void DeclareClassField(ClassScope* scope, ClassLiteralProperty* property,
                          const AstRawString* property_name, bool is_static,
                          bool is_computed_name, bool is_private,
                          ClassInfo* class_info);
-  Expression* RewriteClassLiteral(Scope* block_scope, const AstRawString* name,
+  Expression* RewriteClassLiteral(ClassScope* block_scope,
+                                  const AstRawString* name,
                                   ClassInfo* class_info, int pos, int end_pos);
   Statement* DeclareNative(const AstRawString* name, int pos);
 
@@ -801,6 +804,15 @@ 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) {
+    VariableProxy* proxy = factory()->ast_node_factory()->NewVariableProxy(
+        name, NORMAL_VARIABLE, start_position);
+    class_scope->AddUnresolvedPrivateName(proxy);
+    return proxy;
+  }
+
   V8_INLINE VariableProxy* ExpressionFromIdentifier(
       const AstRawString* name, int start_position,
       InferName infer = InferName::kYes) {

src/parsing/preparser.h

@@ -1103,6 +1103,11 @@ class PreParser : public ParserBase<PreParser> {
     // Don't bother actually binding the proxy.
   }
 
+  Variable* DeclarePrivateVariableName(const AstRawString* name,
+                                       ClassScope* scope, bool* was_added) {
+    return scope->DeclarePrivateName(name, was_added);
+  }
+
   Variable* DeclareVariableName(const AstRawString* name, VariableMode mode,
                                 Scope* scope, bool* was_added,
                                 int position = kNoSourcePosition,
@@ -1225,12 +1230,14 @@ class PreParser : public ParserBase<PreParser> {
                           &was_added);
     }
   }
-  V8_INLINE void DeclareClassProperty(const PreParserIdentifier& class_name,
+  V8_INLINE void DeclareClassProperty(ClassScope* scope,
+                                      const PreParserIdentifier& class_name,
                                       const PreParserExpression& property,
                                       bool is_constructor,
                                       ClassInfo* class_info) {}
 
-  V8_INLINE void DeclareClassField(const PreParserExpression& property,
+  V8_INLINE void DeclareClassField(ClassScope* scope,
+                                   const PreParserExpression& property,
                                    const PreParserIdentifier& property_name,
                                    bool is_static, bool is_computed_name,
                                    bool is_private, ClassInfo* class_info) {
@@ -1240,16 +1247,20 @@ class PreParser : public ParserBase<PreParser> {
       DeclareVariableName(
           ClassFieldVariableName(ast_value_factory(),
                                  class_info->computed_field_count),
-          VariableMode::kConst, scope(), &was_added);
+          VariableMode::kConst, scope, &was_added);
     } else if (is_private) {
       bool was_added;
-      DeclareVariableName(property_name.string_, VariableMode::kConst, scope(),
-                          &was_added);
+      DeclarePrivateVariableName(property_name.string_, scope, &was_added);
+      if (!was_added) {
+        Scanner::Location loc(property.position(), property.position() + 1);
+        ReportMessageAt(loc, MessageTemplate::kVarRedeclaration,
+                        property_name.string_);
+      }
     }
   }
 
   V8_INLINE PreParserExpression
-  RewriteClassLiteral(Scope* scope, const PreParserIdentifier& name,
+  RewriteClassLiteral(ClassScope* scope, const PreParserIdentifier& name,
                       ClassInfo* class_info, int pos, int end_pos) {
     bool has_default_constructor = !class_info->has_seen_constructor;
     // Account for the default constructor.
@@ -1577,6 +1588,15 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserExpression::StringLiteral();
   }
 
+  PreParserExpression ExpressionFromPrivateName(ClassScope* class_scope,
+                                                const PreParserIdentifier& name,
+                                                int start_position) {
+    VariableProxy* proxy = factory()->ast_node_factory()->NewVariableProxy(
+        name.string_, NORMAL_VARIABLE, start_position);
+    class_scope->AddUnresolvedPrivateName(proxy);
+    return PreParserExpression::FromIdentifier(name);
+  }
+
   PreParserExpression ExpressionFromIdentifier(
       const PreParserIdentifier& name, int start_position,
       InferName infer = InferName::kYes) {

test/cctest/interpreter/bytecode_expectations/PrivateClassFields.golden

@@ -25,14 +25,12 @@ snippet: "
 "
 frame size: 10
 parameter count: 1
-bytecode array length: 143
+bytecode array length: 137
 bytecodes: [
   /*   30 E> */ B(StackCheck),
                 B(CreateBlockContext), U8(0),
                 B(PushContext), R(4),
                 B(LdaTheHole),
-                B(StaCurrentContextSlot), U8(4),
-                B(LdaTheHole),
                 B(Star), R(8),
                 B(CreateClosure), U8(2), U8(0), U8(2),
                 B(Star), R(5),
@@ -56,8 +54,6 @@ bytecodes: [
   /*   38 E> */ B(CreateBlockContext), U8(6),
                 B(PushContext), R(4),
                 B(LdaTheHole),
-                B(StaCurrentContextSlot), U8(4),
-                B(LdaTheHole),
                 B(Star), R(8),
                 B(CreateClosure), U8(8), U8(2), U8(2),
                 B(Star), R(5),
@@ -137,14 +133,12 @@ snippet: "
 "
 frame size: 15
 parameter count: 1
-bytecode array length: 289
+bytecode array length: 277
 bytecodes: [
   /*   30 E> */ B(StackCheck),
                 B(CreateBlockContext), U8(0),
                 B(PushContext), R(6),
                 B(LdaTheHole),
-                B(StaCurrentContextSlot), U8(4),
-                B(LdaTheHole),
                 B(Star), R(14),
                 B(CreateClosure), U8(3), U8(0), U8(2),
                 B(Star), R(11),
@@ -176,10 +170,6 @@ bytecodes: [
   /*   38 E> */ B(CreateBlockContext), U8(8),
                 B(PushContext), R(6),
                 B(LdaTheHole),
-                B(StaCurrentContextSlot), U8(4),
-                B(LdaTheHole),
-                B(StaCurrentContextSlot), U8(5),
-                B(LdaTheHole),
                 B(Star), R(14),
                 B(CreateClosure), U8(11), U8(3), U8(2),
                 B(Star), R(11),
@@ -220,8 +210,6 @@ bytecodes: [
                 B(Mov), R(4), R(1),
   /*  140 E> */ B(CreateBlockContext), U8(17),
                 B(PushContext), R(6),
-                B(LdaTheHole),
-                B(StaCurrentContextSlot), U8(4),
   /*  356 E> */ B(CreateClosure), U8(19), U8(8), U8(2),
                 B(Star), R(7),
                 B(LdaConstant), U8(18),

test/cctest/test-parsing.cc

@@ -6072,7 +6072,7 @@ TEST(PrivateStaticClassFieldsErrors) {
                     private_static_fields, arraysize(private_static_fields));
 }
 
-TEST(PrivateNameNoErrors) {
+TEST(PrivateNameResolutionErrors) {
   // clang-format off
   const char* context_data[][2] = {
       {"class X { bar() { ", " } }"},
@@ -6119,7 +6119,7 @@ TEST(PrivateNameNoErrors) {
   RunParserSyncTest(context_data, statement_data, kError);
 
   static const ParserFlag private_fields[] = {kAllowHarmonyPrivateFields};
-  RunParserSyncTest(context_data, statement_data, kSuccess, nullptr, 0,
+  RunParserSyncTest(context_data, statement_data, kError, nullptr, 0,
                     private_fields, arraysize(private_fields));
 }
 
@@ -11291,27 +11291,15 @@ TEST(LexicalLoopVariable) {
   }
 }
 
-TEST(PrivateNamesSyntaxErrorWithScopeAnalysis) {
+TEST(PrivateNamesSyntaxErrorEarly) {
   i::Isolate* isolate = CcTest::i_isolate();
   i::HandleScope scope(isolate);
   LocalContext env;
 
-  auto test = [isolate](const char* program, bool is_lazy) {
-    i::FLAG_harmony_private_fields = true;
-    i::Factory* const factory = isolate->factory();
-    i::Handle<i::String> source =
-        factory->NewStringFromUtf8(i::CStrVector(program)).ToHandleChecked();
-    i::Handle<i::Script> script = factory->NewScript(source);
-    i::ParseInfo info(isolate, script);
-
-    info.set_allow_lazy_parsing(is_lazy);
-    CHECK(i::parsing::ParseProgram(&info, isolate));
-    CHECK(i::Rewriter::Rewrite(&info));
-    CHECK(!i::DeclarationScope::Analyze(&info));
-    return info.pending_error_handler()->has_pending_error();
-  };
+  const char* context_data[][2] = {
+      {"", ""}, {"\"use strict\";", ""}, {nullptr, nullptr}};
 
-  const char* data[] = {
+  const char* statement_data[] = {
       "class A {"
       "  foo() { return this.#bar; }"
       "}",
@@ -11371,12 +11359,12 @@ TEST(PrivateNamesSyntaxErrorWithScopeAnalysis) {
       "function t(){"
       "  return class { getA() { return this.#foo; } }"
       "}",
-  };
 
-  for (const char* source : data) {
-    CHECK(test(source, true));
-    CHECK(test(source, false));
-  }
+      nullptr};
+
+  static const ParserFlag flags[] = {kAllowHarmonyPrivateFields};
+  RunParserSyncTest(context_data, statement_data, kError, nullptr, 0, flags, 1);
+  RunParserSyncTest(context_data, statement_data, kError);
 }
 
 TEST(HashbangSyntax) {

test/message/fail/class-fields-private-class-in-function.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.
+//
+// Flags: --harmony-private-fields
+
+class Y {
+  makeClass() {
+    return class {
+      setA(val) { this.#a = val; }
+      getA() { return this.#a; }
+      getB() { return this.#b; }
+    }
+  }
+  #a;
+}

test/message/fail/class-fields-private-class-in-function.out

+*%(basename)s:12: SyntaxError: Undefined private field #b: must be declared in an enclosing class
+      getB() { return this.#b; }
+                          ^
+SyntaxError: Undefined private field #b: must be declared in an enclosing class

test/message/fail/class-fields-private-throw-in-module.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.
+//
+// MODULE
+// Flags: --harmony-private-fields
+
+class X {
+  constructor() {
+    this.#x = 1;
+  }
+}

test/message/fail/class-fields-private-throw-in-module.out

+*%(basename)s:10: SyntaxError: Undefined private field #x: must be declared in an enclosing class
+    this.#x = 1;
+        ^
+SyntaxError: Undefined private field #x: must be declared in an enclosing class
\ No newline at end of file

test/message/fail/class-fields-private-undefined-inner-class.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.
+//
+// Flags: --harmony-private-fields
+
+class A {
+  fn() {
+    class B {
+      getA() { return this.#b; }
+    }
+  }
+}

test/message/fail/class-fields-private-undefined-inner-class.out

+*%(basename)s:10: SyntaxError: Undefined private field #b: must be declared in an enclosing class
+      getA() { return this.#b; }
+                          ^
+SyntaxError: Undefined private field #b: must be declared in an enclosing class