[class] fix undefined private name access in computed property keys

This patch implements https://github.com/tc39/proposal-class-fields/pull/269
and makes sure we always throw TypeError when there is invalid private
name access in computed property keys.

Before this patch, private name variables of private fields and methods
are initialized together with computed property keys in the order they
are declared. Accessing undefined private names in the computed property
keys thus fail silently.

After this patch, we initialize the private name variables of private
fields before we initialize the computed property keys, so that invalid
access to private fields in the computed keys can be checked in the IC.
We now also initialize the brand early, so that invalid access to private
methods or accessors in the computed keys throw TypeError during brand
checks - and since these accesses are guarded by brand checks, we can
create the private methods and accessors after the class is
defined, and merge the home object setting with the creation
of the closures.

Bug: v8:8330, v8:9611
Change-Id: I01363f7befac6cf9dd28ec229b99a99102bcf012
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1846571
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64225}

src/ast/ast-function-literal-id-reindexer.cc

@@ -46,15 +46,28 @@ void AstFunctionLiteralIdReindexer::VisitClassLiteral(ClassLiteral* expr) {
   if (expr->instance_members_initializer_function() != nullptr) {
     Visit(expr->instance_members_initializer_function());
   }
-  ZonePtrList<ClassLiteral::Property>* props = expr->properties();
+  ZonePtrList<ClassLiteral::Property>* private_members =
+      expr->private_members();
+  for (int i = 0; i < private_members->length(); ++i) {
+    ClassLiteralProperty* prop = private_members->at(i);
+
+    // Private fields have their key and value present in
+    // instance_members_initializer_function, so they will
+    // already have been visited.
+    if (prop->value()->IsFunctionLiteral()) {
+      Visit(prop->value());
+    } else {
+      CheckVisited(prop->value());
+    }
+  }
+  ZonePtrList<ClassLiteral::Property>* props = expr->public_members();
   for (int i = 0; i < props->length(); ++i) {
     ClassLiteralProperty* prop = props->at(i);
 
-    // Private fields and public fields with computed names have both their key
+    // Public fields with computed names have their key
     // and value present in instance_members_initializer_function, so they will
     // already have been visited.
-    if ((prop->is_computed_name() || prop->is_private()) &&
-        !prop->value()->IsFunctionLiteral()) {
+    if (prop->is_computed_name() && !prop->value()->IsFunctionLiteral()) {
       if (!prop->key()->IsLiteral()) {
         CheckVisited(prop->key());
       }

src/ast/ast-traversal-visitor.h

@@ -490,7 +490,13 @@ void AstTraversalVisitor<Subclass>::VisitClassLiteral(ClassLiteral* expr) {
   if (expr->instance_members_initializer_function() != nullptr) {
     RECURSE_EXPRESSION(Visit(expr->instance_members_initializer_function()));
   }
-  ZonePtrList<ClassLiteral::Property>* props = expr->properties();
+  ZonePtrList<ClassLiteral::Property>* private_members =
+      expr->private_members();
+  for (int i = 0; i < private_members->length(); ++i) {
+    ClassLiteralProperty* prop = private_members->at(i);
+    RECURSE_EXPRESSION(Visit(prop->value()));
+  }
+  ZonePtrList<ClassLiteral::Property>* props = expr->public_members();
   for (int i = 0; i < props->length(); ++i) {
     ClassLiteralProperty* prop = props->at(i);
     if (!prop->key()->IsLiteral()) {

src/ast/ast.h

@@ -2492,7 +2492,8 @@ class ClassLiteral final : public Expression {
   ClassScope* scope() const { return scope_; }
   Expression* extends() const { return extends_; }
   FunctionLiteral* constructor() const { return constructor_; }
-  ZonePtrList<Property>* properties() const { return properties_; }
+  ZonePtrList<Property>* public_members() const { return public_members_; }
+  ZonePtrList<Property>* private_members() const { return private_members_; }
   int start_position() const { return position(); }
   int end_position() const { return end_position_; }
   bool has_name_static_property() const {
@@ -2505,6 +2506,9 @@ class ClassLiteral final : public Expression {
   bool is_anonymous_expression() const {
     return IsAnonymousExpression::decode(bit_field_);
   }
+  bool has_private_methods() const {
+    return HasPrivateMethods::decode(bit_field_);
+  }
   bool IsAnonymousFunctionDefinition() const {
     return is_anonymous_expression();
   }
@@ -2521,36 +2525,42 @@ class ClassLiteral final : public Expression {
   friend class AstNodeFactory;
 
   ClassLiteral(ClassScope* scope, Expression* extends,
-               FunctionLiteral* constructor, ZonePtrList<Property>* properties,
+               FunctionLiteral* constructor,
+               ZonePtrList<Property>* public_members,
+               ZonePtrList<Property>* private_members,
                FunctionLiteral* static_fields_initializer,
                FunctionLiteral* instance_members_initializer_function,
                int start_position, int end_position,
                bool has_name_static_property, bool has_static_computed_names,
-               bool is_anonymous)
+               bool is_anonymous, bool has_private_methods)
       : Expression(start_position, kClassLiteral),
         end_position_(end_position),
         scope_(scope),
         extends_(extends),
         constructor_(constructor),
-        properties_(properties),
+        public_members_(public_members),
+        private_members_(private_members),
         static_fields_initializer_(static_fields_initializer),
         instance_members_initializer_function_(
             instance_members_initializer_function) {
     bit_field_ |= HasNameStaticProperty::encode(has_name_static_property) |
                   HasStaticComputedNames::encode(has_static_computed_names) |
-                  IsAnonymousExpression::encode(is_anonymous);
+                  IsAnonymousExpression::encode(is_anonymous) |
+                  HasPrivateMethods::encode(has_private_methods);
   }
 
   int end_position_;
   ClassScope* scope_;
   Expression* extends_;
   FunctionLiteral* constructor_;
-  ZonePtrList<Property>* properties_;
+  ZonePtrList<Property>* public_members_;
+  ZonePtrList<Property>* private_members_;
   FunctionLiteral* static_fields_initializer_;
   FunctionLiteral* instance_members_initializer_function_;
   using HasNameStaticProperty = Expression::NextBitField<bool, 1>;
   using HasStaticComputedNames = HasNameStaticProperty::Next<bool, 1>;
   using IsAnonymousExpression = HasStaticComputedNames::Next<bool, 1>;
+  using HasPrivateMethods = IsAnonymousExpression::Next<bool, 1>;
 };
 
 
@@ -3257,15 +3267,18 @@ class AstNodeFactory final {
 
   ClassLiteral* NewClassLiteral(
       ClassScope* scope, Expression* extends, FunctionLiteral* constructor,
-      ZonePtrList<ClassLiteral::Property>* properties,
+      ZonePtrList<ClassLiteral::Property>* public_members,
+      ZonePtrList<ClassLiteral::Property>* private_members,
       FunctionLiteral* static_fields_initializer,
       FunctionLiteral* instance_members_initializer_function,
       int start_position, int end_position, bool has_name_static_property,
-      bool has_static_computed_names, bool is_anonymous) {
+      bool has_static_computed_names, bool is_anonymous,
+      bool has_private_methods) {
     return new (zone_) ClassLiteral(
-        scope, extends, constructor, properties, static_fields_initializer,
-        instance_members_initializer_function, start_position, end_position,
-        has_name_static_property, has_static_computed_names, is_anonymous);
+        scope, extends, constructor, public_members, private_members,
+        static_fields_initializer, instance_members_initializer_function,
+        start_position, end_position, has_name_static_property,
+        has_static_computed_names, is_anonymous, has_private_methods);
   }
 
   NativeFunctionLiteral* NewNativeFunctionLiteral(const AstRawString* name,

src/ast/prettyprinter.cc

@@ -217,8 +217,11 @@ void CallPrinter::VisitFunctionLiteral(FunctionLiteral* node) {
 
 void CallPrinter::VisitClassLiteral(ClassLiteral* node) {
   if (node->extends()) Find(node->extends());
-  for (int i = 0; i < node->properties()->length(); i++) {
-    Find(node->properties()->at(i)->value());
+  for (int i = 0; i < node->public_members()->length(); i++) {
+    Find(node->public_members()->at(i)->value());
+  }
+  for (int i = 0; i < node->private_members()->length(); i++) {
+    Find(node->private_members()->at(i)->value());
   }
 }
 
@@ -1106,7 +1109,8 @@ void AstPrinter::VisitClassLiteral(ClassLiteral* node) {
     PrintIndentedVisit("INSTANCE MEMBERS INITIALIZER",
                        node->instance_members_initializer_function());
   }
-  PrintClassProperties(node->properties());
+  PrintClassProperties(node->private_members());
+  PrintClassProperties(node->public_members());
 }
 
 void AstPrinter::VisitInitializeClassMembersStatement(

src/interpreter/bytecode-generator.cc

@@ -2042,7 +2042,71 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) {
   VisitDeclarations(expr->scope()->declarations());
   Register class_constructor = register_allocator()->NewRegister();
 
+  // Create the class brand symbol and store it on the context during class
+  // evaluation. This will be stored in the instance later in the constructor.
+  // We do this early so that invalid access to private methods or accessors
+  // in computed property keys throw.
+  if (expr->scope()->brand() != nullptr) {
+    Register brand = register_allocator()->NewRegister();
+    const AstRawString* class_name =
+        expr->scope()->class_variable() != nullptr
+            ? expr->scope()->class_variable()->raw_name()
+            : ast_string_constants()->empty_string();
+    builder()
+        ->LoadLiteral(class_name)
+        .StoreAccumulatorInRegister(brand)
+        .CallRuntime(Runtime::kCreatePrivateNameSymbol, brand);
+    BuildVariableAssignment(expr->scope()->brand(), Token::INIT,
+                            HoleCheckMode::kElided);
+  }
+
   AccessorTable<ClassLiteral::Property> private_accessors(zone());
+  for (int i = 0; i < expr->private_members()->length(); i++) {
+    ClassLiteral::Property* property = expr->private_members()->at(i);
+    DCHECK(property->is_private());
+    switch (property->kind()) {
+      case ClassLiteral::Property::FIELD: {
+        // Initialize the private field variables early.
+        // Create the private name symbols for fields during class
+        // evaluation and store them on the context. These will be
+        // used as keys later during instance or static initialization.
+        RegisterAllocationScope private_name_register_scope(this);
+        Register private_name = register_allocator()->NewRegister();
+        VisitForRegisterValue(property->key(), private_name);
+        builder()
+            ->LoadLiteral(property->key()->AsLiteral()->AsRawPropertyName())
+            .StoreAccumulatorInRegister(private_name)
+            .CallRuntime(Runtime::kCreatePrivateNameSymbol, private_name);
+        DCHECK_NOT_NULL(property->private_name_var());
+        BuildVariableAssignment(property->private_name_var(), Token::INIT,
+                                HoleCheckMode::kElided);
+        break;
+      }
+      case ClassLiteral::Property::METHOD: {
+        // We can initialize the private methods and accessors later so that the
+        // home objects can be assigned right after the creation of the
+        // closures, and those are guarded by the brand checks.
+        break;
+      }
+      // Collect private accessors into a table to merge the creation of
+      // those closures later.
+      case ClassLiteral::Property::GETTER: {
+        Literal* key = property->key()->AsLiteral();
+        DCHECK_NULL(private_accessors.LookupOrInsert(key)->getter);
+        private_accessors.LookupOrInsert(key)->getter = property;
+        break;
+      }
+      case ClassLiteral::Property::SETTER: {
+        Literal* key = property->key()->AsLiteral();
+        DCHECK_NULL(private_accessors.LookupOrInsert(key)->setter);
+        private_accessors.LookupOrInsert(key)->setter = property;
+        break;
+      }
+      default:
+        UNREACHABLE();
+    }
+  }
+
   {
     RegisterAllocationScope register_scope(this);
     RegisterList args = register_allocator()->NewGrowableRegisterList();
@@ -2065,8 +2129,8 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) {
         .StoreAccumulatorInRegister(class_boilerplate);
 
     // Create computed names and method values nodes to store into the literal.
-    for (int i = 0; i < expr->properties()->length(); i++) {
-      ClassLiteral::Property* property = expr->properties()->at(i);
+    for (int i = 0; i < expr->public_members()->length(); i++) {
+      ClassLiteral::Property* property = expr->public_members()->at(i);
       if (property->is_computed_name()) {
         Register key = register_allocator()->GrowRegisterList(&args);
 
@@ -2099,50 +2163,7 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) {
         }
       }
 
-      if (property->is_private()) {
-        // Assign private class member's name variables.
-        switch (property->kind()) {
-          case ClassLiteral::Property::FIELD: {
-            // Create the private name symbols for fields during class
-            // evaluation and store them on the context. These will be
-            // used as keys later during instance or static initialization.
-            RegisterAllocationScope private_name_register_scope(this);
-            Register private_name = register_allocator()->NewRegister();
-            VisitForRegisterValue(property->key(), private_name);
-            builder()
-                ->LoadLiteral(property->key()->AsLiteral()->AsRawPropertyName())
-                .StoreAccumulatorInRegister(private_name)
-                .CallRuntime(Runtime::kCreatePrivateNameSymbol, private_name);
-            DCHECK_NOT_NULL(property->private_name_var());
-            BuildVariableAssignment(property->private_name_var(), Token::INIT,
-                                    HoleCheckMode::kElided);
-            break;
-          }
-          case ClassLiteral::Property::METHOD: {
-            // Create the closures for private methods.
-            VisitForAccumulatorValue(property->value());
-            BuildVariableAssignment(property->private_name_var(), Token::INIT,
-                                    HoleCheckMode::kElided);
-            break;
-          }
-          case ClassLiteral::Property::GETTER: {
-            Literal* key = property->key()->AsLiteral();
-            DCHECK_NULL(private_accessors.LookupOrInsert(key)->getter);
-            private_accessors.LookupOrInsert(key)->getter = property;
-            break;
-          }
-          case ClassLiteral::Property::SETTER: {
-            Literal* key = property->key()->AsLiteral();
-            DCHECK_NULL(private_accessors.LookupOrInsert(key)->setter);
-            private_accessors.LookupOrInsert(key)->setter = property;
-            break;
-          }
-        }
-        // The private fields are initialized in the initializer function and
-        // the private brand for the private methods are initialized in the
-        // constructor instead.
-        continue;
-      }
+      DCHECK(!property->is_private());
 
       if (property->kind() == ClassLiteral::Property::FIELD) {
         // We don't compute field's value here, but instead do it in the
@@ -2168,41 +2189,32 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) {
                             HoleCheckMode::kElided);
   }
 
-  // Create the class brand symbol and store it on the context
-  // during class evaluation. The instance brand will be stored in the
-  // instance later in the constructor.
-  if (expr->scope()->brand() != nullptr) {
-    Register brand = register_allocator()->NewRegister();
-    const AstRawString* class_name =
-        expr->scope()->class_variable() != nullptr
-            ? expr->scope()->class_variable()->raw_name()
-            : ast_string_constants()->empty_string();
-    builder()
-        ->LoadLiteral(class_name)
-        .StoreAccumulatorInRegister(brand)
-        .CallRuntime(Runtime::kCreatePrivateNameSymbol, brand);
-    BuildVariableAssignment(expr->scope()->brand(), Token::INIT,
-                            HoleCheckMode::kElided);
-  }
-  // Store the home object for any private methods that need
-  // them. We do this here once the prototype and brand symbol has
-  // been created. Private accessors have their home object set later
-  // when they are defined.
-  for (int i = 0; i < expr->properties()->length(); i++) {
-    ClassLiteral::Property* property = expr->properties()->at(i);
-    if (property->NeedsHomeObjectOnClassPrototype()) {
+  // Create the closures of private methods, and store the home object for
+  // any private methods that need them.
+  if (expr->has_private_methods()) {
+    for (int i = 0; i < expr->private_members()->length(); i++) {
+      ClassLiteral::Property* property = expr->private_members()->at(i);
+      if (property->kind() != ClassLiteral::Property::METHOD) {
+        continue;
+      }
       RegisterAllocationScope register_scope(this);
-      Register home_object =
-          property->is_static() ? class_constructor : prototype;
-      Register func = register_allocator()->NewRegister();
-      BuildVariableLoad(property->private_name_var(), HoleCheckMode::kElided);
-      builder()->StoreAccumulatorInRegister(func);
-      VisitSetHomeObject(func, home_object, property);
+      VisitForAccumulatorValue(property->value());
+      BuildVariableAssignment(property->private_name_var(), Token::INIT,
+                              HoleCheckMode::kElided);
+      Register home_object = property->private_name_var()->is_static()
+                                 ? class_constructor
+                                 : prototype;
+      if (property->NeedsHomeObjectOnClassPrototype()) {
+        Register func = register_allocator()->NewRegister();
+        builder()->StoreAccumulatorInRegister(func);
+        VisitSetHomeObject(func, home_object, property);
+      }
     }
   }
 
-  // Define accessors, using only a single call to the runtime for each pair
-  // of corresponding getters and setters.
+  // Define private accessors, using only a single call to the runtime for
+  // each pair of corresponding getters and setters, in the order the first
+  // component is declared. Store the home objects if necessary.
   for (auto accessors : private_accessors.ordered_accessors()) {
     RegisterAllocationScope inner_register_scope(this);
     RegisterList accessors_reg = register_allocator()->NewRegisterList(2);

src/objects/literal-objects.cc

@@ -412,8 +412,8 @@ Handle<ClassBoilerplate> ClassBoilerplate::BuildClassBoilerplate(
   ObjectDescriptor static_desc(kMinimumClassPropertiesCount);
   ObjectDescriptor instance_desc(kMinimumPrototypePropertiesCount);
 
-  for (int i = 0; i < expr->properties()->length(); i++) {
-    ClassLiteral::Property* property = expr->properties()->at(i);
+  for (int i = 0; i < expr->public_members()->length(); i++) {
+    ClassLiteral::Property* property = expr->public_members()->at(i);
     ObjectDescriptor& desc =
         property->is_static() ? static_desc : instance_desc;
     if (property->is_computed_name()) {
@@ -477,14 +477,8 @@ Handle<ClassBoilerplate> ClassBoilerplate::BuildClassBoilerplate(
   //
   int dynamic_argument_index = ClassBoilerplate::kFirstDynamicArgumentIndex;
 
-  for (int i = 0; i < expr->properties()->length(); i++) {
-    ClassLiteral::Property* property = expr->properties()->at(i);
-
-    // Private members are not processed using the class boilerplate.
-    if (property->is_private()) {
-      continue;
-    }
-
+  for (int i = 0; i < expr->public_members()->length(); i++) {
+    ClassLiteral::Property* property = expr->public_members()->at(i);
     ClassBoilerplate::ValueKind value_kind;
     switch (property->kind()) {
       case ClassLiteral::Property::METHOD:

src/parsing/expression-scope-reparenter.cc

@@ -54,7 +54,14 @@ void Reparenter::VisitClassLiteral(ClassLiteral* class_literal) {
 #if DEBUG
   // The same goes for the rest of the class, but we do some
   // sanity checking in debug mode.
-  for (ClassLiteralProperty* prop : *class_literal->properties()) {
+  for (ClassLiteralProperty* prop : *class_literal->private_members()) {
+    // No need to visit the values, since all values are functions with
+    // the class scope on their scope chain.
+    DCHECK(prop->value()->IsFunctionLiteral());
+    DCHECK_EQ(prop->value()->AsFunctionLiteral()->scope()->outer_scope(),
+              class_literal->scope());
+  }
+  for (ClassLiteralProperty* prop : *class_literal->public_members()) {
     // No need to visit the values, since all values are functions with
     // the class scope on their scope chain.
     DCHECK(prop->value()->IsFunctionLiteral());

src/parsing/parser-base.h

@@ -530,7 +530,8 @@ class ParserBase {
    public:
     explicit ClassInfo(ParserBase* parser)
         : extends(parser->impl()->NullExpression()),
-          properties(parser->impl()->NewClassPropertyList(4)),
+          public_members(parser->impl()->NewClassPropertyList(4)),
+          private_members(parser->impl()->NewClassPropertyList(4)),
           static_fields(parser->impl()->NewClassPropertyList(4)),
           instance_fields(parser->impl()->NewClassPropertyList(4)),
           constructor(parser->impl()->NullExpression()),
@@ -541,11 +542,13 @@ class ParserBase {
           has_instance_members(false),
           requires_brand(false),
           is_anonymous(false),
+          has_private_methods(false),
           static_fields_scope(nullptr),
           instance_members_scope(nullptr),
           computed_field_count(0) {}
     ExpressionT extends;
-    ClassPropertyListT properties;
+    ClassPropertyListT public_members;
+    ClassPropertyListT private_members;
     ClassPropertyListT static_fields;
     ClassPropertyListT instance_fields;
     FunctionLiteralT constructor;
@@ -557,6 +560,7 @@ class ParserBase {
     bool has_instance_members;
     bool requires_brand;
     bool is_anonymous;
+    bool has_private_methods;
     DeclarationScope* static_fields_scope;
     DeclarationScope* instance_members_scope;
     int computed_field_count;
@@ -4435,6 +4439,8 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
     if (V8_UNLIKELY(prop_info.is_private)) {
       DCHECK(!is_constructor);
       class_info.requires_brand |= (!is_field && !prop_info.is_static);
+      class_info.has_private_methods |=
+          property_kind == ClassLiteralProperty::METHOD;
       impl()->DeclarePrivateClassMember(class_scope, prop_info.name, property,
                                         property_kind, prop_info.is_static,
                                         &class_info);

src/parsing/parser.cc

@@ -2835,7 +2835,7 @@ void Parser::DeclarePublicClassField(ClassScope* scope,
         CreateSyntheticContextVariable(ClassFieldVariableName(
             ast_value_factory(), class_info->computed_field_count));
     property->set_computed_name_var(computed_name_var);
-    class_info->properties->Add(property, zone());
+    class_info->public_members->Add(property, zone());
   }
 }
 
@@ -2865,7 +2865,7 @@ void Parser::DeclarePrivateClassMember(ClassScope* scope,
   }
   private_name_var->set_initializer_position(pos);
   property->set_private_name_var(private_name_var);
-  class_info->properties->Add(property, zone());
+  class_info->private_members->Add(property, zone());
 }
 
 // This method declares a property of the given class.  It updates the
@@ -2886,7 +2886,7 @@ void Parser::DeclarePublicClassMethod(const AstRawString* class_name,
     return;
   }
 
-  class_info->properties->Add(property, zone());
+  class_info->public_members->Add(property, zone());
 }
 
 FunctionLiteral* Parser::CreateInitializerFunction(
@@ -2958,10 +2958,11 @@ Expression* Parser::RewriteClassLiteral(ClassScope* block_scope,
 
   ClassLiteral* class_literal = factory()->NewClassLiteral(
       block_scope, class_info->extends, class_info->constructor,
-      class_info->properties, static_fields_initializer,
-      instance_members_initializer_function, pos, end_pos,
-      class_info->has_name_static_property,
-      class_info->has_static_computed_names, class_info->is_anonymous);
+      class_info->public_members, class_info->private_members,
+      static_fields_initializer, instance_members_initializer_function, pos,
+      end_pos, class_info->has_name_static_property,
+      class_info->has_static_computed_names, class_info->is_anonymous,
+      class_info->has_private_methods);
 
   AddFunctionForNameInference(class_info->constructor);
   return class_literal;

test/cctest/interpreter/bytecode_expectations/PrivateClassFields.golden

@@ -22,25 +22,25 @@ snippet: "
     new B;
   }
 "
-frame size: 8
+frame size: 7
 parameter count: 1
 bytecode array length: 131
 bytecodes: [
   /*   30 E> */ B(StackCheck),
                 B(CreateBlockContext), U8(0),
                 B(PushContext), R(2),
+                B(LdaConstant), U8(2),
+                B(Star), R(4),
+                B(LdaConstant), U8(2),
+                B(Star), R(4),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(StaCurrentContextSlot), U8(4),
                 B(LdaTheHole),
                 B(Star), R(6),
-                B(CreateClosure), U8(2), U8(0), U8(2),
+                B(CreateClosure), U8(3), U8(0), U8(2),
                 B(Star), R(3),
                 B(LdaConstant), U8(1),
                 B(Star), R(4),
-                B(LdaConstant), U8(3),
-                B(Star), R(7),
-                B(LdaConstant), U8(3),
-                B(Star), R(7),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(7), U8(1),
-                B(StaCurrentContextSlot), U8(4),
                 B(Mov), R(3), R(5),
                 B(CallRuntime), U16(Runtime::kDefineClass), R(4), U8(3),
                 B(Star), R(4),
@@ -51,18 +51,18 @@ bytecodes: [
                 B(Mov), R(3), R(0),
   /*   38 E> */ B(CreateBlockContext), U8(6),
                 B(PushContext), R(2),
+                B(LdaConstant), U8(2),
+                B(Star), R(4),
+                B(LdaConstant), U8(2),
+                B(Star), R(4),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(StaCurrentContextSlot), U8(4),
                 B(LdaTheHole),
                 B(Star), R(6),
                 B(CreateClosure), U8(8), U8(2), U8(2),
                 B(Star), R(3),
                 B(LdaConstant), U8(7),
                 B(Star), R(4),
-                B(LdaConstant), U8(3),
-                B(Star), R(7),
-                B(LdaConstant), U8(3),
-                B(Star), R(7),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(7), U8(1),
-                B(StaCurrentContextSlot), U8(4),
                 B(Mov), R(3), R(5),
                 B(CallRuntime), U16(Runtime::kDefineClass), R(4), U8(3),
                 B(Star), R(4),
@@ -81,9 +81,9 @@ bytecodes: [
 constant pool: [
   SCOPE_INFO_TYPE,
   FIXED_ARRAY_TYPE,
-  SHARED_FUNCTION_INFO_TYPE,
   ONE_BYTE_INTERNALIZED_STRING_TYPE ["#a"],
   SHARED_FUNCTION_INFO_TYPE,
+  SHARED_FUNCTION_INFO_TYPE,
   SYMBOL_TYPE,
   SCOPE_INFO_TYPE,
   FIXED_ARRAY_TYPE,
@@ -135,25 +135,25 @@ bytecodes: [
   /*   30 E> */ B(StackCheck),
                 B(CreateBlockContext), U8(0),
                 B(PushContext), R(3),
+                B(LdaConstant), U8(2),
+                B(Star), R(5),
+                B(LdaConstant), U8(2),
+                B(Star), R(5),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(5), U8(1),
+                B(StaCurrentContextSlot), U8(4),
                 B(LdaTheHole),
                 B(Star), R(11),
-                B(CreateClosure), U8(3), U8(0), U8(2),
+                B(CreateClosure), U8(4), U8(0), U8(2),
                 B(Star), R(8),
-                B(LdaConstant), U8(2),
+                B(LdaConstant), U8(3),
                 B(Star), R(9),
                 B(Mov), R(8), R(10),
                 B(CallRuntime), U16(Runtime::kDefineClass), R(9), U8(3),
                 B(Star), R(9),
-                B(CreateClosure), U8(4), U8(1), U8(2),
+                B(CreateClosure), U8(5), U8(1), U8(2),
                 B(Star), R(4),
                 B(LdaConstant), U8(1),
                 B(Star), R(5),
-                B(LdaConstant), U8(5),
-                B(Star), R(8),
-                B(LdaConstant), U8(5),
-                B(Star), R(8),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(8), U8(1),
-                B(StaCurrentContextSlot), U8(4),
                 B(Mov), R(4), R(6),
                 B(Mov), R(10), R(7),
                 B(CallRuntime), U16(Runtime::kDefineClass), R(5), U8(3),
@@ -165,31 +165,31 @@ bytecodes: [
                 B(Mov), R(4), R(0),
   /*   38 E> */ B(CreateBlockContext), U8(8),
                 B(PushContext), R(3),
+                B(LdaConstant), U8(2),
+                B(Star), R(5),
+                B(LdaConstant), U8(2),
+                B(Star), R(5),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(5), U8(1),
+                B(StaCurrentContextSlot), U8(4),
+                B(LdaConstant), U8(10),
+                B(Star), R(5),
+                B(LdaConstant), U8(10),
+                B(Star), R(5),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(5), U8(1),
+                B(StaCurrentContextSlot), U8(5),
                 B(LdaTheHole),
                 B(Star), R(11),
-                B(CreateClosure), U8(11), U8(3), U8(2),
+                B(CreateClosure), U8(12), U8(3), U8(2),
                 B(Star), R(8),
-                B(LdaConstant), U8(10),
+                B(LdaConstant), U8(11),
                 B(Star), R(9),
                 B(Mov), R(8), R(10),
                 B(CallRuntime), U16(Runtime::kDefineClass), R(9), U8(3),
                 B(Star), R(9),
-                B(CreateClosure), U8(12), U8(4), U8(2),
+                B(CreateClosure), U8(13), U8(4), U8(2),
                 B(Star), R(4),
                 B(LdaConstant), U8(9),
                 B(Star), R(5),
-                B(LdaConstant), U8(5),
-                B(Star), R(8),
-                B(LdaConstant), U8(5),
-                B(Star), R(8),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(8), U8(1),
-                B(StaCurrentContextSlot), U8(4),
-                B(LdaConstant), U8(13),
-                B(Star), R(8),
-                B(LdaConstant), U8(13),
-                B(Star), R(8),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(8), U8(1),
-                B(StaCurrentContextSlot), U8(5),
                 B(CreateClosure), U8(14), U8(5), U8(2),
                 B(Star), R(8),
                 B(CreateClosure), U8(15), U8(6), U8(2),
@@ -205,16 +205,16 @@ bytecodes: [
                 B(Mov), R(4), R(1),
   /*  140 E> */ B(CreateBlockContext), U8(17),
                 B(PushContext), R(3),
+                B(LdaConstant), U8(2),
+                B(Star), R(5),
+                B(LdaConstant), U8(2),
+                B(Star), R(5),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(5), U8(1),
+                B(StaCurrentContextSlot), U8(4),
   /*  356 E> */ B(CreateClosure), U8(19), U8(8), U8(2),
                 B(Star), R(4),
                 B(LdaConstant), U8(18),
                 B(Star), R(5),
-                B(LdaConstant), U8(5),
-                B(Star), R(8),
-                B(LdaConstant), U8(5),
-                B(Star), R(8),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(8), U8(1),
-                B(StaCurrentContextSlot), U8(4),
                 B(Mov), R(4), R(6),
                 B(Mov), R(1), R(7),
                 B(CallRuntime), U16(Runtime::kDefineClass), R(5), U8(3),
@@ -236,18 +236,18 @@ bytecodes: [
 constant pool: [
   SCOPE_INFO_TYPE,
   FIXED_ARRAY_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["#a"],
   FIXED_ARRAY_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
-  ONE_BYTE_INTERNALIZED_STRING_TYPE ["#a"],
   SHARED_FUNCTION_INFO_TYPE,
   SYMBOL_TYPE,
   SCOPE_INFO_TYPE,
   FIXED_ARRAY_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["#b"],
   FIXED_ARRAY_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
-  ONE_BYTE_INTERNALIZED_STRING_TYPE ["#b"],
   SHARED_FUNCTION_INFO_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
   SHARED_FUNCTION_INFO_TYPE,

test/cctest/interpreter/bytecode_expectations/PrivateMethodDeclaration.golden

@@ -14,39 +14,39 @@ snippet: "
     }
   }
 "
-frame size: 6
+frame size: 7
 parameter count: 1
 bytecode array length: 52
 bytecodes: [
   /*   30 E> */ B(StackCheck),
                 B(CreateBlockContext), U8(0),
                 B(PushContext), R(1),
+                B(LdaConstant), U8(2),
+                B(Star), R(3),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(3), U8(1),
+                B(StaCurrentContextSlot), U8(5),
                 B(LdaTheHole),
-                B(Star), R(5),
-                B(CreateClosure), U8(2), U8(0), U8(2),
+                B(Star), R(6),
+                B(CreateClosure), U8(3), U8(0), U8(2),
                 B(Star), R(2),
                 B(LdaConstant), U8(1),
-                B(Star), R(3),
-                B(CreateClosure), U8(3), U8(1), U8(2),
-                B(StaCurrentContextSlot), U8(4),
-                B(Mov), R(2), R(4),
-                B(CallRuntime), U16(Runtime::kDefineClass), R(3), U8(3),
-                B(Star), R(3),
-                B(LdaConstant), U8(4),
                 B(Star), R(4),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
-                B(StaCurrentContextSlot), U8(5),
+                B(Mov), R(2), R(5),
+                B(CallRuntime), U16(Runtime::kDefineClass), R(4), U8(3),
+                B(Star), R(4),
+                B(CreateClosure), U8(4), U8(1), U8(2),
+                B(StaCurrentContextSlot), U8(4),
                 B(PopContext), R(1),
-                B(Mov), R(2), R(0),
+                B(Mov), R(5), R(0),
                 B(LdaUndefined),
   /*   77 S> */ B(Return),
 ]
 constant pool: [
   SCOPE_INFO_TYPE,
   FIXED_ARRAY_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["A"],
   SHARED_FUNCTION_INFO_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
-  ONE_BYTE_INTERNALIZED_STRING_TYPE ["A"],
 ]
 handlers: [
 ]
@@ -62,62 +62,62 @@ snippet: "
     }
   }
 "
-frame size: 7
+frame size: 8
 parameter count: 1
 bytecode array length: 101
 bytecodes: [
   /*   30 E> */ B(StackCheck),
                 B(CreateBlockContext), U8(0),
                 B(PushContext), R(2),
+                B(LdaConstant), U8(2),
+                B(Star), R(4),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(StaCurrentContextSlot), U8(5),
                 B(LdaTheHole),
-                B(Star), R(6),
-                B(CreateClosure), U8(2), U8(0), U8(2),
+                B(Star), R(7),
+                B(CreateClosure), U8(3), U8(0), U8(2),
                 B(Star), R(3),
                 B(LdaConstant), U8(1),
-                B(Star), R(4),
-                B(CreateClosure), U8(3), U8(1), U8(2),
-                B(StaCurrentContextSlot), U8(4),
-                B(Mov), R(3), R(5),
-                B(CallRuntime), U16(Runtime::kDefineClass), R(4), U8(3),
-                B(Star), R(4),
-                B(LdaConstant), U8(4),
                 B(Star), R(5),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(5), U8(1),
-                B(StaCurrentContextSlot), U8(5),
+                B(Mov), R(3), R(6),
+                B(CallRuntime), U16(Runtime::kDefineClass), R(5), U8(3),
+                B(Star), R(5),
+                B(CreateClosure), U8(4), U8(1), U8(2),
+                B(StaCurrentContextSlot), U8(4),
                 B(PopContext), R(2),
-                B(Mov), R(3), R(0),
+                B(Mov), R(6), R(0),
   /*   38 E> */ B(CreateBlockContext), U8(5),
                 B(PushContext), R(2),
-  /*   93 E> */ B(CreateClosure), U8(7), U8(2), U8(2),
+                B(LdaConstant), U8(7),
+                B(Star), R(4),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(StaCurrentContextSlot), U8(5),
+  /*   93 E> */ B(CreateClosure), U8(8), U8(2), U8(2),
                 B(Star), R(3),
                 B(LdaConstant), U8(6),
-                B(Star), R(4),
-                B(CreateClosure), U8(8), U8(3), U8(2),
-                B(StaCurrentContextSlot), U8(4),
-                B(Mov), R(3), R(5),
-                B(Mov), R(0), R(6),
-                B(CallRuntime), U16(Runtime::kDefineClass), R(4), U8(3),
-                B(Star), R(4),
-                B(LdaConstant), U8(9),
                 B(Star), R(5),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(5), U8(1),
-                B(StaCurrentContextSlot), U8(5),
+                B(Mov), R(3), R(6),
+                B(Mov), R(0), R(7),
+                B(CallRuntime), U16(Runtime::kDefineClass), R(5), U8(3),
+                B(Star), R(5),
+                B(CreateClosure), U8(9), U8(3), U8(2),
+                B(StaCurrentContextSlot), U8(4),
                 B(PopContext), R(2),
-                B(Mov), R(3), R(1),
+                B(Mov), R(6), R(1),
                 B(LdaUndefined),
   /*  126 S> */ B(Return),
 ]
 constant pool: [
   SCOPE_INFO_TYPE,
   FIXED_ARRAY_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["D"],
   SHARED_FUNCTION_INFO_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
-  ONE_BYTE_INTERNALIZED_STRING_TYPE ["D"],
   SCOPE_INFO_TYPE,
   FIXED_ARRAY_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["E"],
   SHARED_FUNCTION_INFO_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
-  ONE_BYTE_INTERNALIZED_STRING_TYPE ["E"],
 ]
 handlers: [
 ]
@@ -133,7 +133,7 @@ snippet: "
 "
 frame size: 8
 parameter count: 1
-bytecode array length: 100
+bytecode array length: 98
 bytecodes: [
   /*   30 E> */ B(StackCheck),
                 B(CreateBlockContext), U8(0),
@@ -153,23 +153,22 @@ bytecodes: [
                 B(Mov), R(5), R(0),
   /*   38 E> */ B(CreateBlockContext), U8(4),
                 B(PushContext), R(2),
-  /*   77 E> */ B(CreateClosure), U8(6), U8(2), U8(2),
+                B(LdaConstant), U8(6),
+                B(Star), R(4),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(StaCurrentContextSlot), U8(5),
+  /*   77 E> */ B(CreateClosure), U8(7), U8(2), U8(2),
                 B(Star), R(3),
                 B(LdaConstant), U8(5),
-                B(Star), R(4),
-                B(CreateClosure), U8(7), U8(3), U8(2),
-                B(StaCurrentContextSlot), U8(4),
-                B(Mov), R(3), R(5),
-                B(Mov), R(0), R(6),
-                B(CallRuntime), U16(Runtime::kDefineClass), R(4), U8(3),
-                B(Star), R(4),
-                B(LdaConstant), U8(8),
                 B(Star), R(5),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(5), U8(1),
-                B(StaCurrentContextSlot), U8(5),
-                B(LdaCurrentContextSlot), U8(4),
+                B(Mov), R(3), R(6),
+                B(Mov), R(0), R(7),
+                B(CallRuntime), U16(Runtime::kDefineClass), R(5), U8(3),
+                B(Star), R(5),
+                B(CreateClosure), U8(8), U8(3), U8(2),
+                B(StaCurrentContextSlot), U8(4),
                 B(Star), R(6),
-                B(Ldar), R(4),
+                B(Ldar), R(5),
                 B(StaNamedProperty), R(6), U8(9), U8(0),
                 B(PopContext), R(2),
                 B(Mov), R(3), R(1),
@@ -183,9 +182,9 @@ constant pool: [
   SHARED_FUNCTION_INFO_TYPE,
   SCOPE_INFO_TYPE,
   FIXED_ARRAY_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["C"],
   SHARED_FUNCTION_INFO_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
-  ONE_BYTE_INTERNALIZED_STRING_TYPE ["C"],
   SYMBOL_TYPE,
 ]
 handlers: [

test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodDeclaration.golden

@@ -27,11 +27,11 @@ bytecodes: [
                 B(Star), R(2),
                 B(LdaConstant), U8(1),
                 B(Star), R(3),
-                B(CreateClosure), U8(3), U8(1), U8(2),
-                B(StaCurrentContextSlot), U8(4),
                 B(Mov), R(2), R(4),
                 B(CallRuntime), U16(Runtime::kDefineClass), R(3), U8(3),
                 B(Star), R(3),
+                B(CreateClosure), U8(3), U8(1), U8(2),
+                B(StaCurrentContextSlot), U8(4),
                 B(PopContext), R(1),
                 B(Mov), R(4), R(0),
                 B(LdaUndefined),
@@ -189,42 +189,42 @@ snippet: "
     }
   }
 "
-frame size: 6
+frame size: 7
 parameter count: 1
 bytecode array length: 58
 bytecodes: [
   /*   30 E> */ B(StackCheck),
                 B(CreateBlockContext), U8(0),
                 B(PushContext), R(1),
+                B(LdaConstant), U8(2),
+                B(Star), R(3),
+                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(3), U8(1),
+                B(StaCurrentContextSlot), U8(6),
                 B(LdaTheHole),
-                B(Star), R(5),
-                B(CreateClosure), U8(2), U8(0), U8(2),
+                B(Star), R(6),
+                B(CreateClosure), U8(3), U8(0), U8(2),
                 B(Star), R(2),
                 B(LdaConstant), U8(1),
-                B(Star), R(3),
-                B(CreateClosure), U8(3), U8(1), U8(2),
+                B(Star), R(4),
+                B(Mov), R(2), R(5),
+                B(CallRuntime), U16(Runtime::kDefineClass), R(4), U8(3),
+                B(Star), R(4),
+                B(CreateClosure), U8(4), U8(1), U8(2),
                 B(StaCurrentContextSlot), U8(4),
-                B(CreateClosure), U8(4), U8(2), U8(2),
+                B(CreateClosure), U8(5), U8(2), U8(2),
                 B(StaCurrentContextSlot), U8(5),
-                B(Mov), R(2), R(4),
-                B(CallRuntime), U16(Runtime::kDefineClass), R(3), U8(3),
-                B(Star), R(3),
-                B(LdaConstant), U8(5),
-                B(Star), R(4),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
-                B(StaCurrentContextSlot), U8(6),
                 B(PopContext), R(1),
-                B(Mov), R(2), R(0),
+                B(Mov), R(5), R(0),
                 B(LdaUndefined),
   /*   87 S> */ B(Return),
 ]
 constant pool: [
   SCOPE_INFO_TYPE,
   FIXED_ARRAY_TYPE,
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["A"],
   SHARED_FUNCTION_INFO_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
   SHARED_FUNCTION_INFO_TYPE,
-  ONE_BYTE_INTERNALIZED_STRING_TYPE ["A"],
 ]
 handlers: [
 ]

test/cctest/test-parsing.cc

@@ -1506,8 +1506,11 @@ TEST(DiscardFunctionBody) {
         fun = exp->AsObjectLiteral()->properties()->at(0)->value()->
               AsFunctionLiteral();
       } else {
-        fun = exp->AsClassLiteral()->properties()->at(0)->value()->
-              AsFunctionLiteral();
+        fun = exp->AsClassLiteral()
+                  ->public_members()
+                  ->at(0)
+                  ->value()
+                  ->AsFunctionLiteral();
       }
     }
     CHECK(!fun->ShouldEagerCompile());

test/mjsunit/harmony/private-accessors.js

@@ -83,6 +83,30 @@
   assertEquals('d', new C().getA().getD());
 }
 
+{
+  assertThrows(() => {
+    class A {
+      [this.#a] = 1;
+      get #a() {}
+    }
+  }, TypeError);
+
+  assertThrows(() => {
+    class A {
+      [this.#a] = 1;
+      set #a(val) {}
+    }
+  }, TypeError);
+
+  assertThrows(() => {
+    class A {
+      [this.#a] = 1;
+      set #a(val) {}
+      get #a() {}
+    }
+  }, TypeError);
+}
+
 // Duplicate private accessors.
 // https://tc39.es/proposal-private-methods/#sec-static-semantics-early-errors
 {

test/mjsunit/harmony/private-fields.js

@@ -476,3 +476,12 @@
   let c = new C;
   assertThrows(() => c.getA(), SyntaxError);
 }
+
+{
+  assertThrows(() => {
+    class A {
+      [this.#a] = 1;
+      #a = 2;
+    }
+  }, TypeError);
+}

test/mjsunit/harmony/private-methods.js

@@ -295,3 +295,12 @@
 
   assertEquals(1, new C().fn());
 }
+
+{
+  assertThrows(() => {
+    class A {
+      [this.#a] = 1;
+      #a() { }
+    }
+  }, TypeError);
+}