[class] Implement private fields runtime semantics

Things that don't work yet:
(a) pre parsed scope data is broken
(b) private fields can be accessed outside classes
(c) no early or runtime error for accessing unknown fields

Things that do work:
everything else

Change-Id: I3d58be44e2be73ec50defb42403112a8a5e68c54
Bug: v8:5368
Reviewed-on: https://chromium-review.googlesource.com/865497
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50935}

src/ast/ast.cc

@@ -311,7 +311,7 @@ ClassLiteralProperty::ClassLiteralProperty(Expression* key, Expression* value,
     : LiteralProperty(key, value, is_computed_name),
       kind_(kind),
       is_static_(is_static),
-      computed_name_var_(nullptr) {}
+      private_or_computed_name_var_(nullptr) {}
 
 bool ObjectLiteral::Property::IsCompileTimeValue() const {
   return kind_ == CONSTANT ||

src/ast/ast.h

@@ -1579,7 +1579,6 @@ enum LhsKind {
   KEYED_SUPER_PROPERTY
 };
 
-
 class Property final : public Expression {
  public:
   bool IsValidReferenceExpression() const { return true; }
@@ -2354,14 +2353,29 @@ 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, FIELD };
+  enum Kind : uint8_t { METHOD, GETTER, SETTER, PUBLIC_FIELD, PRIVATE_FIELD };
 
   Kind kind() const { return kind_; }
 
   bool is_static() const { return is_static_; }
 
-  void set_computed_name_var(Variable* var) { computed_name_var_ = var; }
-  Variable* computed_name_var() const { return computed_name_var_; }
+  void set_computed_name_var(Variable* var) {
+    DCHECK_EQ(PUBLIC_FIELD, kind());
+    private_or_computed_name_var_ = var;
+  }
+  Variable* computed_name_var() const {
+    DCHECK_EQ(PUBLIC_FIELD, kind());
+    return private_or_computed_name_var_;
+  }
+
+  void set_private_field_name_var(Variable* var) {
+    DCHECK_EQ(PRIVATE_FIELD, kind());
+    private_or_computed_name_var_ = var;
+  }
+  Variable* private_field_name_var() const {
+    DCHECK_EQ(PRIVATE_FIELD, kind());
+    return private_or_computed_name_var_;
+  }
 
  private:
   friend class AstNodeFactory;
@@ -2371,7 +2385,7 @@ class ClassLiteralProperty final : public LiteralProperty {
 
   Kind kind_;
   bool is_static_;
-  Variable* computed_name_var_;
+  Variable* private_or_computed_name_var_;
 };
 
 class InitializeClassFieldsStatement final : public Statement {

src/ast/prettyprinter.cc

@@ -1043,8 +1043,11 @@ void AstPrinter::PrintClassProperties(
       case ClassLiteral::Property::SETTER:
         prop_kind = "SETTER";
         break;
-      case ClassLiteral::Property::FIELD:
-        prop_kind = "FIELD";
+      case ClassLiteral::Property::PUBLIC_FIELD:
+        prop_kind = "PUBLIC FIELD";
+        break;
+      case ClassLiteral::Property::PRIVATE_FIELD:
+        prop_kind = "PRIVATE FIELD";
         break;
     }
     EmbeddedVector<char, 128> buf;
@@ -1222,7 +1225,6 @@ void AstPrinter::VisitThrow(Throw* node) {
   Visit(node->exception());
 }
 
-
 void AstPrinter::VisitProperty(Property* node) {
   EmbeddedVector<char, 128> buf;
   SNPrintF(buf, "PROPERTY");

src/interpreter/bytecode-generator.cc

@@ -1829,6 +1829,7 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr) {
     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);
         Register key = register_allocator()->GrowRegisterList(&args);
 
         BuildLoadPropertyKey(property, key);
@@ -1846,7 +1847,7 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr) {
               .Bind(&done);
         }
 
-        if (property->kind() == ClassLiteral::Property::FIELD) {
+        if (property->kind() == ClassLiteral::Property::PUBLIC_FIELD) {
           // Initialize field's name variable with the computed name.
           DCHECK_NOT_NULL(property->computed_name_var());
           builder()->LoadAccumulatorWithRegister(key);
@@ -1854,11 +1855,19 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr) {
                                   HoleCheckMode::kElided);
         }
       }
-      if (property->kind() == ClassLiteral::Property::FIELD) {
+
+      if (property->kind() == ClassLiteral::Property::PUBLIC_FIELD) {
         // 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::kCreatePrivateSymbol);
+        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);
       VisitForRegisterValue(property->value(), value);
     }
@@ -1938,12 +1947,18 @@ void BytecodeGenerator::VisitInitializeClassFieldsStatement(
     ClassLiteral::Property* property = expr->fields()->at(i);
 
     if (property->is_computed_name()) {
+      DCHECK_EQ(property->kind(), ClassLiteral::Property::PUBLIC_FIELD);
       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) {
+      Variable* private_field_name_var = property->private_field_name_var();
+      DCHECK_NOT_NULL(private_field_name_var);
+      BuildVariableLoad(private_field_name_var, HoleCheckMode::kElided);
+      builder()->StoreAccumulatorInRegister(key);
     } else {
       BuildLoadPropertyKey(property, key);
     }

src/objects/literal-objects.cc

@@ -513,11 +513,14 @@ Handle<ClassBoilerplate> ClassBoilerplate::BuildClassBoilerplate(
       case ClassLiteral::Property::SETTER:
         value_kind = ClassBoilerplate::kSetter;
         break;
-      case ClassLiteral::Property::FIELD:
+      case ClassLiteral::Property::PUBLIC_FIELD:
         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

@@ -2337,7 +2337,9 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
     case PropertyKind::kShorthandProperty:
     case PropertyKind::kValueProperty:
       if (allow_harmony_public_fields() || allow_harmony_private_fields()) {
-        *property_kind = ClassLiteralProperty::FIELD;
+        *property_kind = name_token == Token::PRIVATE_NAME
+                             ? ClassLiteralProperty::PRIVATE_FIELD
+                             : ClassLiteralProperty::PUBLIC_FIELD;
         if (*is_static && !allow_harmony_static_fields()) {
           ReportUnexpectedToken(Next());
           *ok = false;
@@ -3712,15 +3714,18 @@ ParserBase<Impl>::ParseMemberExpressionContinuation(ExpressionT expression,
 
         Consume(Token::PERIOD);
         int pos = peek_position();
+        ExpressionT key;
         IdentifierT name;
         if (allow_harmony_private_fields() && peek() == Token::PRIVATE_NAME) {
+          // TODO(gsathya): Validate that we are in a class body.
           Consume(Token::PRIVATE_NAME);
           name = impl()->GetSymbol();
+          key = impl()->ExpressionFromIdentifier(name, pos, InferName::kNo);
         } else {
           name = ParseIdentifierName(CHECK_OK);
+          key = factory()->NewStringLiteral(name, pos);
         }
-        expression = factory()->NewProperty(
-            expression, factory()->NewStringLiteral(name, pos), pos);
+        expression = factory()->NewProperty(expression, key, pos);
         impl()->PushLiteralName(name);
         break;
       }
@@ -4548,7 +4553,8 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
         is_computed_name) {
       class_info.has_static_computed_names = true;
     }
-    if (is_computed_name && property_kind == ClassLiteralProperty::FIELD) {
+    if (is_computed_name &&
+        property_kind == ClassLiteralProperty::PUBLIC_FIELD) {
       class_info.computed_field_count++;
     }
     is_constructor &= class_info.has_seen_constructor;

src/parsing/parser.cc

@@ -3331,7 +3331,8 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
     return;
   }
 
-  if (kind != ClassLiteralProperty::FIELD) {
+  if (kind != ClassLiteralProperty::PUBLIC_FIELD &&
+      kind != ClassLiteralProperty::PRIVATE_FIELD) {
     class_info->properties->Add(property, zone());
     return;
   }
@@ -3340,12 +3341,14 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
 
   if (is_static) {
     DCHECK(allow_harmony_static_fields());
+    DCHECK_EQ(kind, ClassLiteralProperty::PUBLIC_FIELD);
     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);
     // We create a synthetic variable name here so that scope
     // analysis doesn't dedupe the vars.
     Variable* computed_name_var = CreateSyntheticContextVariable(
@@ -3355,6 +3358,13 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
     property->set_computed_name_var(computed_name_var);
     class_info->properties->Add(property, zone());
   }
+
+  if (kind == ClassLiteralProperty::PRIVATE_FIELD) {
+    Variable* private_field_name_var = CreateSyntheticContextVariable(
+        property->key()->AsLiteral()->AsRawPropertyName(), CHECK_OK_VOID);
+    property->set_private_field_name_var(private_field_name_var);
+    class_info->properties->Add(property, zone());
+  }
 }
 
 FunctionLiteral* Parser::CreateInitializerFunction(

src/parsing/preparser.h

@@ -1147,7 +1147,9 @@ class PreParser : public ParserBase<PreParser> {
                                       bool is_static, bool is_constructor,
                                       bool is_computed_name,
                                       ClassInfo* class_info, bool* ok) {
-    if (kind == ClassLiteralProperty::FIELD && is_computed_name) {
+    // TODO(gsathya): Fix preparsed scope data for PRIVATE_FIELDs by
+    // allocating context variable here.
+    if (kind == ClassLiteralProperty::PUBLIC_FIELD && is_computed_name) {
       scope()->DeclareVariableName(
           ClassFieldVariableName(ast_value_factory(),
                                  class_info->computed_field_count),

src/runtime/runtime-symbol.cc

@@ -25,11 +25,13 @@ RUNTIME_FUNCTION(Runtime_CreateSymbol) {
 
 RUNTIME_FUNCTION(Runtime_CreatePrivateSymbol) {
   HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(Object, name, 0);
-  CHECK(name->IsString() || name->IsUndefined(isolate));
+  DCHECK_GE(1, args.length());
   Handle<Symbol> symbol = isolate->factory()->NewPrivateSymbol();
-  if (name->IsString()) symbol->set_name(*name);
+  if (args.length() == 1) {
+    CONVERT_ARG_HANDLE_CHECKED(Object, name, 0);
+    CHECK(name->IsString() || name->IsUndefined(isolate));
+    if (name->IsString()) symbol->set_name(*name);
+  }
   return *symbol;
 }
 

src/runtime/runtime.h

@@ -547,11 +547,11 @@ namespace internal {
   F(StringCharFromCode, 1, 1)             \
   F(StringMaxLength, 0, 1)
 
-#define FOR_EACH_INTRINSIC_SYMBOL(F) \
-  F(CreateSymbol, 1, 1)              \
-  F(CreatePrivateSymbol, 1, 1)       \
-  F(SymbolDescription, 1, 1)         \
-  F(SymbolDescriptiveString, 1, 1)   \
+#define FOR_EACH_INTRINSIC_SYMBOL(F)       \
+  F(CreateSymbol, 1, 1)                    \
+  F(CreatePrivateSymbol, -1 /* <= 1 */, 1) \
+  F(SymbolDescription, 1, 1)               \
+  F(SymbolDescriptiveString, 1, 1)         \
   F(SymbolIsPrivate, 1, 1)
 
 #define FOR_EACH_INTRINSIC_TEST(F)            \

test/mjsunit/harmony/private-fields.js

+// Copyright 2018 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 --no-lazy --allow-natives-syntax
+
+
+"use strict";
+
+// TODO(gsathya): Missing tests:
+// (a) check private field access on proxies.
+// (b) throw reference error on missing private field access.
+// (c) throw when private fields are set without being declared.
+// (d) throw when private fields are accessed outside class bodies.
+// (e) check that private field isn't called 'constructor'.
+// (f) tests involving eval
+{
+  class C {
+    #a;
+    getA() { return this.#a; }
+  }
+
+  assertEquals(undefined, C.a);
+
+  let c = new C;
+  assertEquals(undefined, c.a);
+  assertEquals(undefined, c.getA());
+}
+
+{
+  class C {
+    #a = 1;
+    getA() { return this.#a; }
+  }
+
+  assertEquals(undefined, C.a);
+
+  let c = new C;
+  assertEquals(undefined, c.a);
+  assertEquals(1, c.getA());
+}
+
+{
+  class C {
+    #a = 1;
+    #b = this.#a;
+    getB() { return this.#b; }
+  }
+
+  let c = new C;
+  assertEquals(1, c.getB());
+}
+
+{
+  class C {
+    #b = this.#a;
+    #a = 1;
+    getB() { return this.#b; }
+  }
+
+  let c = new C;
+  assertEquals(undefined, c.getB());
+}
+
+{
+  class C {
+    #a = 1;
+    getA() { return this.#a; }
+    constructor() {
+      assertEquals(1, this.#a);
+      this.#a = 5;
+    }
+  }
+
+  let c = new C;
+  assertEquals(5, c.getA());
+}
+
+{
+  class C {
+    #a = this;
+    #b = () => this;
+    getA() { return this.#a; }
+    getB() { return this.#b; }
+  }
+
+  let c1 = new C;
+  assertSame(c1, c1.getA());
+  assertSame(c1, c1.getB()());
+  let c2 = new C;
+  assertSame(c1, c1.getB().call(c2));
+}
+
+{
+  class C {
+    #a = this;
+    #b = function() { return this; };
+    getA() { return this.#a; }
+    getB() { return this.#b; }
+  }
+
+  let c1 = new C;
+  assertSame(c1, c1.getA());
+  assertSame(c1, c1.getB().call(c1));
+  let c2 = new C;
+  assertSame(c2, c1.getB().call(c2));
+}
+
+
+{
+  class C {
+    #a = function() { return 1 };
+    getA() {return this.#a;}
+  }
+
+  let c = new C;
+  assertEquals('#a', c.getA().name);
+}
+
+{
+  let d = function() { return new.target; }
+  class C {
+    #c = d;
+    getC() { return this.#c; }
+  }
+
+  let c = new C;
+  assertEquals(undefined, c.getC()());
+  assertSame(new d, new (c.getC()));
+}
+
+{
+  class C {
+    #b = new.target;
+    #c = () => new.target;
+    getB() { return this.#b; }
+    getC() { return this.#c; }
+  }
+
+  let c = new C;
+  assertEquals(undefined, c.getB());
+  assertEquals(undefined, c.getC()());
+}
+
+{
+  class C {
+    #a = 1;
+    #b = () => this.#a;
+    getB() { return this.#b; }
+  }
+
+  let c1 = new C;
+  assertSame(1, c1.getB()());
+}
+
+{
+  class C {
+    #a = 1;
+    getA(instance) { return instance.#a; }
+  }
+
+  class B { }
+  let c = new C;
+  assertEquals(undefined, c.a);
+  assertEquals(1, c.getA(c));
+
+  //TODO(gsathya): This should be a TypeError.
+  //assertThrows(() => c.getA(new B), TypeError);
+}
+
+{
+  let prototypeLookup = false;
+  class A {
+    set a(val) {
+      prototypeLookup = true;
+    }
+
+    get a() { return undefined; }
+  }
+
+  class C extends A {
+    #a = 1;
+    getA() { return this.#a; }
+  }
+
+  let c = new C;
+  assertEquals(1, c.getA());
+  assertEquals(false, prototypeLookup);
+}
+
+{
+  class A {
+    constructor() { this.a = 1; }
+  }
+
+  class B extends A {
+    #b = this.a;
+    getB() { return this.#b; }
+  }
+
+  let b = new B;
+  assertEquals(1, b.getB());
+}
+
+{
+  class A {
+    #a = 1;
+    getA() { return this.#a; }
+  }
+
+  class B extends A {
+    #b = super.getA();
+    getB() { return this.#b; }
+  }
+
+  let b = new B;
+  assertEquals(1, b.getB());
+}
+
+{
+  class A {
+    #a = 1;
+    getA() { return this.#a;}
+  }
+
+  class B extends A {
+    #a = 2;
+    get_A() { return this.#a;}
+  }
+
+  let a = new A;
+  let b = new B;
+  assertEquals(1, a.getA());
+  assertEquals(1, b.getA());
+  assertEquals(2, b.get_A());
+}
+
+{
+  let foo = undefined;
+  class A {
+    #a = 1;
+    constructor() {
+      foo = this.#a;
+    }
+  }
+
+  let a = new A;
+  assertEquals(1, foo);
+}
+
+{
+  let foo = undefined;
+  class A extends class {} {
+    #a = 1;
+    constructor() {
+      super();
+      foo = this.#a;
+    }
+  }
+
+  let a = new A;
+  assertEquals(1, foo);
+}
+
+{
+  function makeClass() {
+    return class {
+      #a;
+      setA(val) { this.#a = val; }
+      getA() { return this.#a; }
+    }
+  }
+
+  let classA = makeClass();
+  let a = new classA;
+  let classB = makeClass();
+  let b = new classB;
+
+  assertEquals(undefined, a.getA());
+  assertEquals(undefined, b.getA());
+
+  a.setA(3);
+  assertEquals(3, a.getA());
+  assertEquals(undefined, b.getA());
+
+  b.setA(5);
+  assertEquals(3, a.getA());
+  assertEquals(5, b.getA());
+
+  // assertThrows(() => a.getA.call(b), ReferenceError);
+  // assertThrows(() => b.getA.call(a), ReferenceError);
+}
+
+{
+  let value = undefined;
+
+  new class {
+    #a = 1;
+    getA() { return this.#a; }
+
+    constructor() {
+      new class {
+        #a = 2;
+        constructor() {
+          value = this.#a;
+        }
+      }
+    }
+  }
+
+  assertEquals(2, value);
+}
+
+{
+  class A {
+    #a = 1;
+    b = class {
+      getA() { return this.#a; }
+      get_A(val) { return val.#a; }
+    }
+  }
+
+  let a = new A();
+  let b = new a.b;
+  assertEquals(1, b.getA.call(a));
+  assertEquals(1, b.get_A(a));
+}
+// {
+//   class C {
+//     b = this.#a;
+//     #a = 1;
+//   }
+
+//   assertThrows(() => new C, ReferenceError);
+// }
+
+{
+  let symbol = Symbol();
+
+  class C {
+    #a = 1;
+    [symbol] = 1;
+    getA() { return this.#a; }
+  }
+
+  var p = new Proxy(new C, {
+    get: function(target, name) {
+      if (typeof(arg) === 'symbol') {
+        assertFalse(%SymbolIsPrivate(name));
+      }
+      return target[name];
+    }
+  });
+
+  //assertThrows(() => p.getA(), ReferenceError);
+  assertEquals(1, p[symbol]);
+}