[ast] add ClassLiteralProperty::is_private() and merge Kind

This patch merges ClassLiteralProperty::PUBLIC_FIELD and
ClassLiteralProperty::PRIVATE_FIELD into ClassLiteralProperty::FIELD,
and moves the visibility part into ClassLiteralProperty::is_private()
for the ease of adding new combinations in the future.

Bug: v8:8330

R=gsathya@chromium.org

Change-Id: I54f64d05bccb1867d9111e4c80158a6075406d80
Reviewed-on: https://chromium-review.googlesource.com/c/1291052Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Cr-Commit-Position: refs/heads/master@{#56910}

src/ast/ast.cc

@@ -333,10 +333,12 @@ bool LiteralProperty::NeedsSetFunctionName() const {
 
 ClassLiteralProperty::ClassLiteralProperty(Expression* key, Expression* value,
                                            Kind kind, bool is_static,
-                                           bool is_computed_name)
+                                           bool is_computed_name,
+                                           bool is_private)
     : LiteralProperty(key, value, is_computed_name),
       kind_(kind),
       is_static_(is_static),
+      is_private_(is_private),
       private_or_computed_name_var_(nullptr) {}
 
 bool ObjectLiteral::Property::IsCompileTimeValue() const {

src/ast/ast.h

@@ -2433,27 +2433,34 @@ class FunctionLiteral final : public Expression {
 // about a class literal's properties from the parser to the code generator.
 class ClassLiteralProperty final : public LiteralProperty {
  public:
-  enum Kind : uint8_t { METHOD, GETTER, SETTER, PUBLIC_FIELD, PRIVATE_FIELD };
+  enum Kind : uint8_t { METHOD, GETTER, SETTER, FIELD };
 
   Kind kind() const { return kind_; }
 
   bool is_static() const { return is_static_; }
 
+  bool is_private() const { return is_private_; }
+
   void set_computed_name_var(Variable* var) {
-    DCHECK_EQ(PUBLIC_FIELD, kind());
+    DCHECK_EQ(FIELD, kind());
+    DCHECK(!is_private());
     private_or_computed_name_var_ = var;
   }
+
   Variable* computed_name_var() const {
-    DCHECK_EQ(PUBLIC_FIELD, kind());
+    DCHECK_EQ(FIELD, kind());
+    DCHECK(!is_private());
     return private_or_computed_name_var_;
   }
 
   void set_private_field_name_var(Variable* var) {
-    DCHECK_EQ(PRIVATE_FIELD, kind());
+    DCHECK_EQ(FIELD, kind());
+    DCHECK(is_private());
     private_or_computed_name_var_ = var;
   }
   Variable* private_field_name_var() const {
-    DCHECK_EQ(PRIVATE_FIELD, kind());
+    DCHECK_EQ(FIELD, kind());
+    DCHECK(is_private());
     return private_or_computed_name_var_;
   }
 
@@ -2461,10 +2468,11 @@ class ClassLiteralProperty final : public LiteralProperty {
   friend class AstNodeFactory;
 
   ClassLiteralProperty(Expression* key, Expression* value, Kind kind,
-                       bool is_static, bool is_computed_name);
+                       bool is_static, bool is_computed_name, bool is_private);
 
   Kind kind_;
   bool is_static_;
+  bool is_private_;
   Variable* private_or_computed_name_var_;
 };
 
@@ -3258,9 +3266,9 @@ class AstNodeFactory final {
 
   ClassLiteral::Property* NewClassLiteralProperty(
       Expression* key, Expression* value, ClassLiteralProperty::Kind kind,
-      bool is_static, bool is_computed_name) {
-    return new (zone_)
-        ClassLiteral::Property(key, value, kind, is_static, is_computed_name);
+      bool is_static, bool is_computed_name, bool is_private) {
+    return new (zone_) ClassLiteral::Property(key, value, kind, is_static,
+                                              is_computed_name, is_private);
   }
 
   ClassLiteral* NewClassLiteral(

src/ast/prettyprinter.cc

@@ -1064,16 +1064,13 @@ void AstPrinter::PrintClassProperties(
       case ClassLiteral::Property::SETTER:
         prop_kind = "SETTER";
         break;
-      case ClassLiteral::Property::PUBLIC_FIELD:
-        prop_kind = "PUBLIC FIELD";
-        break;
-      case ClassLiteral::Property::PRIVATE_FIELD:
-        prop_kind = "PRIVATE FIELD";
+      case ClassLiteral::Property::FIELD:
+        prop_kind = "FIELD";
         break;
     }
     EmbeddedVector<char, 128> buf;
-    SNPrintF(buf, "PROPERTY%s - %s", property->is_static() ? " - STATIC" : "",
-             prop_kind);
+    SNPrintF(buf, "PROPERTY%s%s - %s", property->is_static() ? " - STATIC" : "",
+             property->is_private() ? "- PRIVATE" : "- PUBLIC", prop_kind);
     IndentedScope prop(this, buf.start());
     PrintIndentedVisit("KEY", properties->at(i)->key());
     PrintIndentedVisit("VALUE", properties->at(i)->value());

src/interpreter/bytecode-generator.cc

@@ -1860,7 +1860,8 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) {
     for (int i = 0; i < expr->properties()->length(); i++) {
       ClassLiteral::Property* property = expr->properties()->at(i);
       if (property->is_computed_name()) {
-        DCHECK_NE(property->kind(), ClassLiteral::Property::PRIVATE_FIELD);
+        DCHECK_IMPLIES(property->kind() == ClassLiteral::Property::FIELD,
+                       !property->is_private());
         Register key = register_allocator()->GrowRegisterList(&args);
 
         builder()->SetExpressionAsStatementPosition(property->key());
@@ -1882,7 +1883,8 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) {
               .Bind(&done);
         }
 
-        if (property->kind() == ClassLiteral::Property::PUBLIC_FIELD) {
+        if (property->kind() == ClassLiteral::Property::FIELD &&
+            !property->is_private()) {
           // Initialize field's name variable with the computed name.
           DCHECK_NOT_NULL(property->computed_name_var());
           builder()->LoadAccumulatorWithRegister(key);
@@ -1891,16 +1893,16 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) {
         }
       }
 
-      if (property->kind() == ClassLiteral::Property::PUBLIC_FIELD) {
+      if (property->kind() == ClassLiteral::Property::FIELD) {
+        if (property->is_private()) {
+          builder()->CallRuntime(Runtime::kCreatePrivateFieldSymbol);
+          DCHECK_NOT_NULL(property->private_field_name_var());
+          BuildVariableAssignment(property->private_field_name_var(),
+                                  Token::INIT, HoleCheckMode::kElided);
+        }
         // We don't compute field's value here, but instead do it in the
         // initializer function.
         continue;
-      } else if (property->kind() == ClassLiteral::Property::PRIVATE_FIELD) {
-        builder()->CallRuntime(Runtime::kCreatePrivateFieldSymbol);
-        DCHECK_NOT_NULL(property->private_field_name_var());
-        BuildVariableAssignment(property->private_field_name_var(), Token::INIT,
-                                HoleCheckMode::kElided);
-        continue;
       }
 
       Register value = register_allocator()->GrowRegisterList(&args);
@@ -2004,14 +2006,16 @@ void BytecodeGenerator::VisitInitializeClassFieldsStatement(
     ClassLiteral::Property* property = stmt->fields()->at(i);
 
     if (property->is_computed_name()) {
-      DCHECK_EQ(property->kind(), ClassLiteral::Property::PUBLIC_FIELD);
+      DCHECK_EQ(property->kind(), ClassLiteral::Property::FIELD);
+      DCHECK(!property->is_private());
       Variable* var = property->computed_name_var();
       DCHECK_NOT_NULL(var);
       // The computed name is already evaluated and stored in a
       // variable at class definition time.
       BuildVariableLoad(var, HoleCheckMode::kElided);
       builder()->StoreAccumulatorInRegister(key);
-    } else if (property->kind() == ClassLiteral::Property::PRIVATE_FIELD) {
+    } else if (property->kind() == ClassLiteral::Property::FIELD &&
+               property->is_private()) {
       Variable* private_field_name_var = property->private_field_name_var();
       DCHECK_NOT_NULL(private_field_name_var);
       BuildVariableLoad(private_field_name_var, HoleCheckMode::kElided);
@@ -2025,7 +2029,8 @@ void BytecodeGenerator::VisitInitializeClassFieldsStatement(
     VisitSetHomeObject(value, constructor, property);
 
     Runtime::FunctionId function_id =
-        property->kind() == ClassLiteral::Property::PUBLIC_FIELD
+        property->kind() == ClassLiteral::Property::FIELD &&
+                !property->is_private()
             ? Runtime::kCreateDataProperty
             : Runtime::kAddPrivateField;
     builder()->CallRuntime(function_id, args);

src/objects/literal-objects.cc

@@ -525,14 +525,12 @@ Handle<ClassBoilerplate> ClassBoilerplate::BuildClassBoilerplate(
       case ClassLiteral::Property::SETTER:
         value_kind = ClassBoilerplate::kSetter;
         break;
-      case ClassLiteral::Property::PUBLIC_FIELD:
+      case ClassLiteral::Property::FIELD:
+        DCHECK_IMPLIES(property->is_computed_name(), !property->is_private());
         if (property->is_computed_name()) {
           ++dynamic_argument_index;
         }
         continue;
-      case ClassLiteral::Property::PRIVATE_FIELD:
-        DCHECK(!property->is_computed_name());
-        continue;
     }
 
     ObjectDescriptor& desc =

src/parsing/parser-base.h

@@ -1043,12 +1043,14 @@ class ParserBase {
 
   ExpressionT ParsePropertyName(IdentifierT* name, ParsePropertyKind* kind,
                                 ParseFunctionFlags* flags,
-                                bool* is_computed_name, bool* ok);
+                                bool* is_computed_name, bool* is_private,
+                                bool* ok);
   ExpressionT ParseObjectLiteral(bool* ok);
   ClassLiteralPropertyT ParseClassPropertyDefinition(
       ClassLiteralChecker* checker, ClassInfo* class_info,
       IdentifierT* property_name, bool has_extends, bool* is_computed_name,
-      ClassLiteralProperty::Kind* property_kind, bool* is_static, bool* ok);
+      ClassLiteralProperty::Kind* property_kind, bool* is_static,
+      bool* is_private, bool* ok);
   ExpressionT ParseClassFieldInitializer(ClassInfo* class_info, int beg_pos,
                                          bool is_static, bool* ok);
   ObjectLiteralPropertyT ParseObjectPropertyDefinition(
@@ -2057,7 +2059,7 @@ inline bool ParseAsAccessor(Token::Value token, Token::Value contextual_token,
 template <class Impl>
 typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParsePropertyName(
     IdentifierT* name, ParsePropertyKind* kind, ParseFunctionFlags* flags,
-    bool* is_computed_name, bool* ok) {
+    bool* is_computed_name, bool* is_private, bool* ok) {
   DCHECK_EQ(ParsePropertyKind::kNotSet, *kind);
   DCHECK_EQ(*flags, ParseFunctionFlag::kIsNormal);
   DCHECK(!*is_computed_name);
@@ -2181,7 +2183,8 @@ typename ParserBase<Impl>::ClassLiteralPropertyT
 ParserBase<Impl>::ParseClassPropertyDefinition(
     ClassLiteralChecker* checker, ClassInfo* class_info, IdentifierT* name,
     bool has_extends, bool* is_computed_name,
-    ClassLiteralProperty::Kind* property_kind, bool* is_static, bool* ok) {
+    ClassLiteralProperty::Kind* property_kind, bool* is_static,
+    bool* is_private, bool* ok) {
   DCHECK_NOT_NULL(class_info);
   ParseFunctionFlags function_flags = ParseFunctionFlag::kIsNormal;
   *is_static = false;
@@ -2217,16 +2220,17 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
       *is_static = true;
       name_expression =
           ParsePropertyName(name, &kind, &function_flags, is_computed_name,
-                            CHECK_OK_CUSTOM(NullLiteralProperty));
+                            is_private, CHECK_OK_CUSTOM(NullLiteralProperty));
     }
   } else if (name_token == Token::PRIVATE_NAME) {
     Consume(Token::PRIVATE_NAME);
+    *is_private = true;
     *name = impl()->GetSymbol();
     name_expression = factory()->NewStringLiteral(*name, position());
   } else {
     name_expression =
         ParsePropertyName(name, &kind, &function_flags, is_computed_name,
-                          CHECK_OK_CUSTOM(NullLiteralProperty));
+                          is_private, CHECK_OK_CUSTOM(NullLiteralProperty));
   }
 
   if (!class_info->has_name_static_property && *is_static &&
@@ -2247,9 +2251,8 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
     case ParsePropertyKind::kShorthand:
     case ParsePropertyKind::kValue:
       if (allow_harmony_public_fields() || allow_harmony_private_fields()) {
-        *property_kind = name_token == Token::PRIVATE_NAME
-                             ? ClassLiteralProperty::PRIVATE_FIELD
-                             : ClassLiteralProperty::PUBLIC_FIELD;
+        *property_kind = ClassLiteralProperty::FIELD;
+        *is_private = name_token == Token::PRIVATE_NAME;
         if (*is_static && !allow_harmony_static_fields()) {
           ReportUnexpectedToken(Next());
           *ok = false;
@@ -2265,7 +2268,7 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
         ExpectSemicolon(CHECK_OK_CUSTOM(NullLiteralProperty));
         ClassLiteralPropertyT result = factory()->NewClassLiteralProperty(
             name_expression, initializer, *property_kind, *is_static,
-            *is_computed_name);
+            *is_computed_name, *is_private);
         impl()->SetFunctionNameFromPropertyName(result, *name);
         return result;
 
@@ -2305,8 +2308,8 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
 
       *property_kind = ClassLiteralProperty::METHOD;
       ClassLiteralPropertyT result = factory()->NewClassLiteralProperty(
-          name_expression, value, *property_kind, *is_static,
-          *is_computed_name);
+          name_expression, value, *property_kind, *is_static, *is_computed_name,
+          *is_private);
       impl()->SetFunctionNameFromPropertyName(result, *name);
       return result;
     }
@@ -2338,8 +2341,8 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
       *property_kind =
           is_get ? ClassLiteralProperty::GETTER : ClassLiteralProperty::SETTER;
       ClassLiteralPropertyT result = factory()->NewClassLiteralProperty(
-          name_expression, value, *property_kind, *is_static,
-          *is_computed_name);
+          name_expression, value, *property_kind, *is_static, *is_computed_name,
+          *is_private);
       const AstRawString* prefix =
           is_get ? ast_value_factory()->get_space_string()
                  : ast_value_factory()->set_space_string();
@@ -2411,10 +2414,15 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
   int next_beg_pos = peek_position();
   int next_end_pos = peek_end_position();
 
+  bool is_private = false;
   ExpressionT name_expression =
       ParsePropertyName(&name, &kind, &function_flags, is_computed_name,
-                        CHECK_OK_CUSTOM(NullLiteralProperty));
+                        &is_private, CHECK_OK_CUSTOM(NullLiteralProperty));
 
+  if (is_private) {
+    // TODO(joyee): private names in object literals should be Syntax Errors
+    // https://tc39.github.io/proposal-private-methods/#prod-PropertyDefinition
+  }
   switch (kind) {
     case ParsePropertyKind::kSpread:
       DCHECK_EQ(function_flags, ParseFunctionFlag::kIsNormal);
@@ -4374,6 +4382,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
     bool is_computed_name = false;  // Classes do not care about computed
                                     // property names here.
     bool is_static;
+    bool is_private = false;
     ClassLiteralProperty::Kind property_kind;
     ExpressionClassifier property_classifier(this);
     IdentifierT property_name;
@@ -4382,13 +4391,13 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
     bool is_constructor = !class_info.has_seen_constructor;
     ClassLiteralPropertyT property = ParseClassPropertyDefinition(
         &checker, &class_info, &property_name, has_extends, &is_computed_name,
-        &property_kind, &is_static, CHECK_OK);
+        &property_kind, &is_static, &is_private, CHECK_OK);
     if (!class_info.has_static_computed_names && is_static &&
         is_computed_name) {
       class_info.has_static_computed_names = true;
     }
-    if (is_computed_name &&
-        property_kind == ClassLiteralProperty::PUBLIC_FIELD) {
+    if (is_computed_name && !is_private &&
+        property_kind == ClassLiteralProperty::FIELD) {
       class_info.computed_field_count++;
     }
     is_constructor &= class_info.has_seen_constructor;
@@ -4397,7 +4406,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
 
     impl()->DeclareClassProperty(name, property, property_name, property_kind,
                                  is_static, is_constructor, is_computed_name,
-                                 &class_info, CHECK_OK);
+                                 is_private, &class_info, CHECK_OK);
     impl()->InferFunctionName();
   }
 

src/parsing/parser.cc

@@ -3140,8 +3140,8 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
                                   const AstRawString* property_name,
                                   ClassLiteralProperty::Kind kind,
                                   bool is_static, bool is_constructor,
-                                  bool is_computed_name, ClassInfo* class_info,
-                                  bool* ok) {
+                                  bool is_computed_name, bool is_private,
+                                  ClassInfo* class_info, bool* ok) {
   if (is_constructor) {
     DCHECK(!class_info->constructor);
     class_info->constructor = property->value()->AsFunctionLiteral();
@@ -3152,8 +3152,7 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
     return;
   }
 
-  if (kind != ClassLiteralProperty::PUBLIC_FIELD &&
-      kind != ClassLiteralProperty::PRIVATE_FIELD) {
+  if (kind != ClassLiteralProperty::FIELD) {
     class_info->properties->Add(property, zone());
     return;
   }
@@ -3162,14 +3161,16 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
 
   if (is_static) {
     DCHECK(allow_harmony_static_fields());
-    DCHECK_EQ(kind, ClassLiteralProperty::PUBLIC_FIELD);
+    DCHECK_EQ(kind, ClassLiteralProperty::FIELD);
+    DCHECK(!is_private);
     class_info->static_fields->Add(property, zone());
   } else {
     class_info->instance_fields->Add(property, zone());
   }
 
   if (is_computed_name) {
-    DCHECK_EQ(kind, ClassLiteralProperty::PUBLIC_FIELD);
+    DCHECK_EQ(kind, ClassLiteralProperty::FIELD);
+    DCHECK(!is_private);
     // We create a synthetic variable name here so that scope
     // analysis doesn't dedupe the vars.
     Variable* computed_name_var = CreateSyntheticContextVariable(
@@ -3180,7 +3181,7 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
     class_info->properties->Add(property, zone());
   }
 
-  if (kind == ClassLiteralProperty::PRIVATE_FIELD) {
+  if (kind == ClassLiteralProperty::FIELD && is_private) {
     Variable* private_field_name_var =
         CreateSyntheticContextVariable(property_name, CHECK_OK_VOID);
     property->set_private_field_name_var(private_field_name_var);

src/parsing/parser.h

@@ -345,7 +345,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
                                       const AstRawString* property_name,
                                       ClassLiteralProperty::Kind kind,
                                       bool is_static, bool is_constructor,
-                                      bool is_computed_name,
+                                      bool is_computed_name, bool is_private,
                                       ClassInfo* class_info, bool* ok);
   V8_INLINE Expression* RewriteClassLiteral(Scope* block_scope,
                                             const AstRawString* name,

src/parsing/preparser.h

@@ -597,7 +597,8 @@ class PreParserFactory {
                                               const PreParserExpression& value,
                                               ClassLiteralProperty::Kind kind,
                                               bool is_static,
-                                              bool is_computed_name) {
+                                              bool is_computed_name,
+                                              bool is_private) {
     return PreParserExpression::Default();
   }
   PreParserExpression NewObjectLiteralProperty(const PreParserExpression& key,
@@ -1232,16 +1233,17 @@ class PreParser : public ParserBase<PreParser> {
                                       const PreParserIdentifier& property_name,
                                       ClassLiteralProperty::Kind kind,
                                       bool is_static, bool is_constructor,
-                                      bool is_computed_name,
+                                      bool is_computed_name, bool is_private,
                                       ClassInfo* class_info, bool* ok) {
-    if (kind == ClassLiteralProperty::PUBLIC_FIELD && is_computed_name) {
+    if (kind == ClassLiteralProperty::FIELD && !is_private &&
+        is_computed_name) {
       scope()->DeclareVariableName(
           ClassFieldVariableName(ast_value_factory(),
                                  class_info->computed_field_count),
           VariableMode::kConst);
     }
 
-    if (kind == ClassLiteralProperty::PRIVATE_FIELD &&
+    if (kind == ClassLiteralProperty::FIELD && is_private &&
         property_name.string_ != nullptr) {
       scope()->DeclareVariableName(property_name.string_, VariableMode::kConst);
     }