Private fields

src/parsing/parser.cc

 // Copyright 2012 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.
 
 #include "src/parsing/parser.h"
 
 #include <memory>
 
 #include "src/api.h"
 #include "src/ast/ast-expression-rewriter.h"
@@ skipping 145 matching lines @@
       ast_value_factory()->GetOneByteString(".class-field-initializer");
   VariableProxy* init_fn_proxy = scope->NewUnresolved(factory(), init_fn_name);
   ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
   args->Add(init_fn_proxy, zone());
   args->Add(this_expr, zone());
   return factory()->NewCallRuntime(Runtime::kInlineCall, args,
                                    kNoSourcePosition);
 }
 
 Expression* Parser::RewriteSuperCall(Expression* super_call) {
-  if (!allow_harmony_class_fields()) {
+  if (!(allow_harmony_class_fields() || allow_harmony_private_class_fields())) {
     return super_call;
   }
   // TODO(bakkot) The whole charade with the do expression can be replaced by a
   // ternary conditional, except that the optimizer does not like it if you use
   // an AST node (at least, one containing a variable proxy) in two places.
   Variable* var_tmp =
       scope()->NewTemporary(ast_value_factory()->empty_string());
   Block* block = factory()->NewBlock(nullptr, 1, false, kNoSourcePosition);
   Assignment* assignment = factory()->NewAssignment(
       Token::ASSIGN, factory()->NewVariableProxy(var_tmp), super_call,
@@ skipping 400 matching lines @@
                       info->isolate()->is_tail_call_elimination_enabled());
   set_allow_harmony_do_expressions(FLAG_harmony_do_expressions);
   set_allow_harmony_for_in(FLAG_harmony_for_in);
   set_allow_harmony_function_sent(FLAG_harmony_function_sent);
   set_allow_harmony_restrictive_declarations(
       FLAG_harmony_restrictive_declarations);
   set_allow_harmony_async_await(FLAG_harmony_async_await);
   set_allow_harmony_restrictive_generators(FLAG_harmony_restrictive_generators);
   set_allow_harmony_trailing_commas(FLAG_harmony_trailing_commas);
   set_allow_harmony_class_fields(FLAG_harmony_class_fields);
+  set_allow_harmony_private_class_fields(FLAG_harmony_private_class_fields);
   for (int feature = 0; feature < v8::Isolate::kUseCounterFeatureCount;
        ++feature) {
     use_counts_[feature] = 0;
   }
   if (info->ast_value_factory() == NULL) {
     // info takes ownership of AstValueFactory.
     info->set_ast_value_factory(new AstValueFactory(zone(), info->hash_seed()));
     info->set_ast_value_factory_owned();
     ast_value_factory_ = info->ast_value_factory();
     ast_node_factory_.set_ast_value_factory(ast_value_factory_);
@@ skipping 2761 matching lines @@
   Variable* result = NewTemporary(ast_value_factory()->dot_result_string());
   Block* block = ParseBlock(nullptr, CHECK_OK);
   DoExpression* expr = factory()->NewDoExpression(block, result, pos);
   if (!Rewriter::Rewrite(this, GetClosureScope(), expr, ast_value_factory())) {
     *ok = false;
     return nullptr;
   }
   return expr;
 }
 
+const AstRawString* ClassPrivateFieldVariableName(
+    AstValueFactory* ast_value_factory, const AstRawString* raw_name) {
+  std::string name(reinterpret_cast<const char*>(raw_name->raw_data()),
+                   raw_name->length());  // TODO(bakkot) check complicated names

[Dan Ehrenberg, 2016/09/10 04:30:28]

What's the issue here? Non-latin1 names?

[bakkot, 2016/09/13 01:05:38]

Non-one-byte names, in particular, although it looks like this particular line
is ok.

+  name = "#" + name;

[Dan Ehrenberg, 2016/09/10 04:30:28]

We don't concatenate strings in the parser like this generally.

[bakkot, 2016/09/13 01:05:38]

I'm not sure it can be avoided; we need an AstRawString here.

+  return ast_value_factory->GetOneByteString(
+      name.c_str());  // TODO(bakkot) sometimes two-byte probably, though maybe
+                      // doesn't matter

[Dan Ehrenberg, 2016/09/10 04:30:28]

Yeah, you can in fact have identifier characters which don't fit in latin1 in JS

[bakkot, 2016/09/13 01:05:38]

The reason I think it might not matter is that the variables using these names
are totally synthetic. It's valuable to have the name match the field name only
if the name will be visible somewhere; currently it is, because of the
ReferenceError thrown at runtime when referring to an unbound name, but that can
change.

If I understand correctly, this will convert two-byte names to one-byte, which
will mangle the name in a deterministic way, and it'll function correctly. But
this may not be the case. If not, I'm not sure how to deal with this.

+}
+
+Property* Parser::ParsePrivateFieldReference(Expression* base, bool* ok) {
+  // PrimaryExpression ::
+  //     PrivateName
+  int pos = peek_position();
+
+  Expect(Token::HASH, CHECK_OK);
+  const AstRawString* property_name =
+      ParseIdentifierName(CHECK_OK);  // TODO(bakkot) more complex names
+  const AstRawString* symbol_var_name =
+      ClassPrivateFieldVariableName(ast_value_factory(), property_name);
+
+  ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
+  args->Add(base, zone());
+  args->Add(NewUnresolved(symbol_var_name), zone());
+  Expression* call = factory()->NewCallRuntime(
+      Runtime::kThrowIfMissingPrivateField, args, pos);
+
+  Expression* prop = NewUnresolved(symbol_var_name);
+  return factory()->NewProperty(call, prop, pos);

[Dan Ehrenberg, 2016/09/10 04:30:28]

Great logic here, ends up being very elegant, but long-term I think we want
ThrowIfMissingPrivateField to be either part of the load/store IC (a flag passed
into that), or, it looks like we could pretty easily make a code stub for this
runtime function (based on CodeStubAssembler::TryHasOwnProperty). Include a TODO
explaining the need for optimization here.

[bakkot, 2016/09/13 01:05:38]

Added a TODO.

+}
+
 void Parser::ParseArrowFunctionFormalParameterList(
     ParserFormalParameters* parameters, Expression* expr,
     const Scanner::Location& params_loc, Scanner::Location* duplicate_loc,
     const Scope::Snapshot& scope_snapshot, bool* ok) {
   if (expr->IsEmptyParentheses()) return;
 
   ParseArrowFunctionFormalParameters(parameters, expr, params_loc.end_pos,
                                      CHECK_OK_VOID);
 
   scope_snapshot.Reparent(parameters->scope);
@@ skipping 859 matching lines @@
     reusable_preparser_->set_allow_lazy(true);
 #define SET_ALLOW(name) reusable_preparser_->set_allow_##name(allow_##name());
     SET_ALLOW(natives);
     SET_ALLOW(harmony_do_expressions);
     SET_ALLOW(harmony_for_in);
     SET_ALLOW(harmony_function_sent);
     SET_ALLOW(harmony_restrictive_declarations);
     SET_ALLOW(harmony_async_await);
     SET_ALLOW(harmony_trailing_commas);
     SET_ALLOW(harmony_class_fields);
+    SET_ALLOW(harmony_private_class_fields);
 #undef SET_ALLOW
   }
   PreParser::PreParseResult result = reusable_preparser_->PreParseLazyFunction(
       language_mode(), function_state_->kind(),
       scope()->AsDeclarationScope()->has_simple_parameters(), parsing_module_,
       logger, may_abort, use_counts_);
   if (pre_parse_timer_ != NULL) {
     pre_parse_timer_->Stop();
   }
   return result;
@@ skipping 151 matching lines @@
     impl()->AccumulateFormalParameterContainmentErrors();
   } else {
     block_state.set_start_position(scanner()->location().end_pos);
   }
 
 
   ClassLiteralChecker checker(this);
   ZoneList<ClassLiteral::Property*>* properties = NewClassPropertyList(4);
   ZoneList<Expression*>* instance_field_initializers =
       new (zone()) ZoneList<Expression*>(0, zone());
+  ZoneList<Variable*>* private_field_symbol_vars =
+      new (zone()) ZoneList<Variable*>(0, zone());
+  ZoneList<Variable*>* private_field_name_vars =
+      new (zone()) ZoneList<Variable*>(0, zone());
   FunctionLiteral* constructor = nullptr;
   bool has_seen_constructor = false;
   Variable* static_initializer_var = nullptr;
 
   Block* do_block = factory()->NewBlock(nullptr, 1, false, pos);
   Variable* result_var = NewTemporary(ast_value_factory()->empty_string());
   DoExpression* do_expr = factory()->NewDoExpression(do_block, result_var, pos);
 
   Expect(Token::LBRACE, CHECK_OK);
 
   const bool has_extends = extends != nullptr;
   while (peek() != Token::RBRACE) {
     if (Check(Token::SEMICOLON)) continue;
     FuncNameInferrer::State fni_state(fni_);
+    const AstRawString* private_name = nullptr;
     bool is_computed_name = false;  // Classes do not care about computed
                                     // property names here.
     ExpressionClassifier property_classifier(this);
-    ClassLiteral::Property* property =
-        ParseClassPropertyDefinition(&checker, has_extends, &is_computed_name,
-                                     &has_seen_constructor, CHECK_OK);
+    ClassLiteral::Property* property = ParseClassPropertyDefinition(
+        &checker, has_extends, &private_name, &is_computed_name,
+        &has_seen_constructor, CHECK_OK);
     RewriteNonPattern(CHECK_OK);
     impl()->AccumulateFormalParameterContainmentErrors();
 
     if (has_seen_constructor && constructor == nullptr) {
       constructor = GetPropertyValue(property)->AsFunctionLiteral();
       DCHECK_NOT_NULL(constructor);
       constructor->set_raw_name(
           name != nullptr ? name : ast_value_factory()->empty_string());
     } else {
       if (property->kind() == ClassLiteralProperty::FIELD) {
-        DCHECK(allow_harmony_class_fields());
-        if (property->is_static()) {
+        if (private_name != nullptr) {
+          DCHECK(allow_harmony_private_class_fields());
+          DCHECK(property->key() == nullptr);
+          DCHECK(!property->is_static());
+
+          const AstRawString* symbol_var_name =
+              ClassPrivateFieldVariableName(ast_value_factory(), private_name);
+          Variable* symbol_var = scope()->DeclareLocal(
+              symbol_var_name, CONST, kCreatedInitialized,
+              Variable::NORMAL);  // TODO(bakkot) flags, Declare vs DeclareLocal

[Dan Ehrenberg, 2016/09/10 04:30:28]

I'm confused, where's the actual declaration AST node made? This creates a
rather strange AST, I think. I think Declare is what you want generally.

[bakkot, 2016/09/13 01:05:38]

Switched to that. This has the side effect of making duplicate property name
declaration an early SyntaxError: `class { #a; #a; }` produces "SyntaxError:
Identifier '#a' has already been declared". This isn't quite per spec, but
sounds like it probably should be:
https://github.com/tc39/proposal-private-fields/issues/48

+          private_field_symbol_vars->Add(symbol_var, zone());
+
+          const AstRawString* name_var_name = ClassFieldVariableName(
+              true, ast_value_factory(), instance_field_initializers->length());
+          Variable* name_var = scope()->DeclareLocal(
+              name_var_name, CONST, kCreatedInitialized,
+              Variable::NORMAL);  // TODO(bakkot) flags, Declare vs DeclareLocal
+          private_field_name_vars->Add(name_var, zone());
+
+          instance_field_initializers->Add(property->value(), zone());
+
+          // Does not add to properties because no action is required from the
+          // backend during class definition.
+        } else if (property->is_static()) {
+          DCHECK(allow_harmony_class_fields());
           if (static_initializer_var == nullptr) {
             static_initializer_var =
                 NewTemporary(ast_value_factory()->empty_string());
           }
           // TODO(bakkot) only do this conditionally
           Expression* function = InstallHomeObject(
               property->value(),
               factory()->NewVariableProxy(static_initializer_var));
           ZoneList<Expression*>* args =
               new (zone()) ZoneList<Expression*>(2, zone());
           args->Add(function, zone());
           args->Add(factory()->NewVariableProxy(static_initializer_var),
                     zone());
           Expression* call = factory()->NewCallRuntime(Runtime::kInlineCall,
                                                        args, kNoSourcePosition);
           property->set_value(call);
+          properties->Add(property, zone());
         } else {
+          DCHECK(allow_harmony_class_fields());
           // if (is_computed_name) { // TODO(bakkot) figure out why this is
           // necessary for non-computed names in full-codegen
           ZoneList<Expression*>* to_name_args =
               new (zone()) ZoneList<Expression*>(1, zone());
           to_name_args->Add(property->key(), zone());
           property->set_key(factory()->NewCallRuntime(
               Runtime::kToName, to_name_args, kNoSourcePosition));
           //}
           const AstRawString* name = ClassFieldVariableName(
               true, ast_value_factory(), instance_field_initializers->length());
           VariableProxy* name_proxy = NewUnresolved(name);
           Declaration* name_declaration =
               factory()
                   ->NewVariableDeclaration(  // TODO(bakkot) DeclareLocal? not
                                              // clear on the difference...
                       name_proxy, scope(), kNoSourcePosition);
           Variable* name_var =
               Declare(name_declaration, DeclarationDescriptor::NORMAL, CONST,
                       kNeedsInitialization, ok, scope());
           DCHECK(ok);
           if (!ok) return nullptr;
           instance_field_initializers->Add(property->value(), zone());
           property->set_value(factory()->NewVariableProxy(name_var));
+          properties->Add(property, zone());
         }
+      } else {
+        properties->Add(property, zone());
       }
-      properties->Add(property, zone());
     }
 
     DCHECK_NOT_NULL(fni_);
     fni_->Infer();
   }
 
   Expect(Token::RBRACE, CHECK_OK);
   int end_pos = scanner()->location().end_pos;
 
   bool has_instance_fields = instance_field_initializers->length() > 0;
-  DCHECK(!has_instance_fields || allow_harmony_class_fields());
+  DCHECK(!has_instance_fields || allow_harmony_class_fields() ||
+         allow_harmony_private_class_fields());
   bool has_default_constructor = constructor == nullptr;
   if (has_default_constructor) {
     constructor = DefaultConstructor(name, has_extends, has_instance_fields,
                                      pos, end_pos, block_state.language_mode());
   }
 
   if (has_instance_fields && extends == nullptr) {
     constructor = InsertClassFieldInitializer(constructor);
     constructor->set_requires_class_field_init(true);
   }  // The derived case is handled by rewriting super calls.
 
   block_state.set_end_position(end_pos);
 
   if (name != nullptr) {
     DCHECK_NOT_NULL(proxy);
     proxy->var()->set_initializer_position(end_pos);
   }
 
+  DCHECK(private_field_symbol_vars->length() ==
+         private_field_name_vars->length());
+  DCHECK(!allow_harmony_private_class_fields() ||
+         private_field_symbol_vars->length() == 0);
+  for (int i = 0; i < private_field_symbol_vars->length(); ++i) {

[Dan Ehrenberg, 2016/09/10 04:30:28]

A comment explaining the desugaring you're doing?

[bakkot, 2016/09/13 01:05:38]

Done.

+    VariableProxy* symbol_var_proxy =
+        factory()->NewVariableProxy(private_field_symbol_vars->at(i));
+    VariableProxy* name_var_proxy =
+        factory()->NewVariableProxy(private_field_name_vars->at(i));
+
+    ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(1, zone());
+    args->Add(factory()->NewUndefinedLiteral(kNoSourcePosition),
+              zone());  // TODO(bakkot) name
+    Expression* name =
+        factory()->NewCallRuntime(Runtime::kCreatePrivateSymbol, args, pos);
+
+    Assignment* inner_assignment = factory()->NewAssignment(
+        Token::INIT, name_var_proxy, name, kNoSourcePosition);
+    Assignment* outer_assignment = factory()->NewAssignment(
+        Token::INIT, symbol_var_proxy, inner_assignment, kNoSourcePosition);
+    do_block->statements()->Add(
+        factory()->NewExpressionStatement(outer_assignment, kNoSourcePosition),
+        zone());
+  }
+
   ClassLiteral* class_literal = factory()->NewClassLiteral(
       proxy, extends, constructor, properties, pos, end_pos);
 
   if (static_initializer_var != nullptr) {
     class_literal->set_static_initializer_proxy(
         factory()->NewVariableProxy(static_initializer_var));
   }
 
   do_block->statements()->Add(
       factory()->NewExpressionStatement(
           factory()->NewAssignment(Token::ASSIGN,
                                    factory()->NewVariableProxy(result_var),
                                    class_literal, kNoSourcePosition),
           pos),
       zone());
-  if (allow_harmony_class_fields() &&
+  if ((allow_harmony_class_fields() || allow_harmony_private_class_fields()) &&
       (has_instance_fields ||
        (extends != nullptr && !has_default_constructor))) {
     // Default constructors for derived classes without fields will not try to
     // read this variable, so there's no need to create it.
     const AstRawString* init_fn_name =
         ast_value_factory()->GetOneByteString(".class-field-initializer");
     Variable* init_fn_var =
         scope()->DeclareLocal(init_fn_name, CONST, kCreatedInitialized,
                               Variable::NORMAL);  // TODO(bakkot) flags
     Expression* initializer =
@@ skipping 1922 matching lines @@
   node->Print(Isolate::Current());
 }
 #endif  // DEBUG
 
 #undef CHECK_OK
 #undef CHECK_OK_VOID
 #undef CHECK_FAILED
 
 }  // namespace internal
 }  // namespace v8