[class] Fix lazy parsing of private fields

Declare the private field variable in the preparser as well, to be
consistent with the parser.

Bug: v8:5386
Change-Id: I961ddf14e47b99701e2463cab0f4d4de140e1e3a
Reviewed-on: https://chromium-review.googlesource.com/905843Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51154}

src/parsing/parser-base.h

@@ -1126,10 +1126,10 @@ class ParserBase {
                                 bool* ok);
   ExpressionT ParseObjectLiteral(bool* ok);
   ClassLiteralPropertyT ParseClassPropertyDefinition(
-      ClassLiteralChecker* checker, ClassInfo* class_info, bool has_extends,
-      bool* is_computed_name, bool* has_seen_constructor,
-      ClassLiteralProperty::Kind* property_kind, bool* is_static,
-      bool* has_name_static_property, bool* ok);
+      ClassLiteralChecker* checker, ClassInfo* class_info,
+      IdentifierT* property_name, bool has_extends, bool* is_computed_name,
+      bool* has_seen_constructor, ClassLiteralProperty::Kind* property_kind,
+      bool* is_static, bool* has_name_static_property, bool* ok);
   ExpressionT ParseClassFieldInitializer(ClassInfo* class_info, bool is_static,
                                          bool* ok);
   ObjectLiteralPropertyT ParseObjectPropertyDefinition(
@@ -2271,8 +2271,8 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParsePropertyName(
 template <typename Impl>
 typename ParserBase<Impl>::ClassLiteralPropertyT
 ParserBase<Impl>::ParseClassPropertyDefinition(
-    ClassLiteralChecker* checker, ClassInfo* class_info, bool has_extends,
-    bool* is_computed_name, bool* has_seen_constructor,
+    ClassLiteralChecker* checker, ClassInfo* class_info, IdentifierT* name,
+    bool has_extends, bool* is_computed_name, bool* has_seen_constructor,
     ClassLiteralProperty::Kind* property_kind, bool* is_static,
     bool* has_name_static_property, bool* ok) {
   DCHECK_NOT_NULL(has_seen_constructor);
@@ -2290,19 +2290,19 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
                  allow_harmony_private_fields());
 
   int name_token_position = scanner()->peek_location().beg_pos;
-  IdentifierT name = impl()->NullIdentifier();
+  *name = impl()->NullIdentifier();
   ExpressionT name_expression;
   if (name_token == Token::STATIC) {
     Consume(Token::STATIC);
     name_token_position = scanner()->peek_location().beg_pos;
     if (peek() == Token::LPAREN) {
       kind = PropertyKind::kMethodProperty;
-      name = impl()->GetSymbol();  // TODO(bakkot) specialize on 'static'
-      name_expression = factory()->NewStringLiteral(name, position());
+      *name = impl()->GetSymbol();  // TODO(bakkot) specialize on 'static'
+      name_expression = factory()->NewStringLiteral(*name, position());
     } else if (peek() == Token::ASSIGN || peek() == Token::SEMICOLON ||
                peek() == Token::RBRACE) {
-      name = impl()->GetSymbol();  // TODO(bakkot) specialize on 'static'
-      name_expression = factory()->NewStringLiteral(name, position());
+      *name = impl()->GetSymbol();  // TODO(bakkot) specialize on 'static'
+      name_expression = factory()->NewStringLiteral(*name, position());
     } else if (peek() == Token::PRIVATE_NAME) {
       DCHECK(allow_harmony_private_fields());
       // TODO(gsathya): Make a better error message for this.
@@ -2311,21 +2311,21 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
       return impl()->NullLiteralProperty();
     } else {
       *is_static = true;
-      name_expression = ParsePropertyName(&name, &kind, &is_generator, &is_get,
+      name_expression = ParsePropertyName(name, &kind, &is_generator, &is_get,
                                           &is_set, &is_async, is_computed_name,
                                           CHECK_OK_CUSTOM(NullLiteralProperty));
     }
   } else if (name_token == Token::PRIVATE_NAME) {
     Consume(Token::PRIVATE_NAME);
-    name = impl()->GetSymbol();
-    name_expression = factory()->NewStringLiteral(name, position());
+    *name = impl()->GetSymbol();
+    name_expression = factory()->NewStringLiteral(*name, position());
   } else {
-    name_expression = ParsePropertyName(&name, &kind, &is_generator, &is_get,
+    name_expression = ParsePropertyName(name, &kind, &is_generator, &is_get,
                                         &is_set, &is_async, is_computed_name,
                                         CHECK_OK_CUSTOM(NullLiteralProperty));
   }
 
-  if (!*has_name_static_property && *is_static && impl()->IsName(name)) {
+  if (!*has_name_static_property && *is_static && impl()->IsName(*name)) {
     *has_name_static_property = true;
   }
 
@@ -2359,7 +2359,7 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
         ClassLiteralPropertyT result = factory()->NewClassLiteralProperty(
             name_expression, initializer, *property_kind, *is_static,
             *is_computed_name);
-        impl()->SetFunctionNameFromPropertyName(result, name);
+        impl()->SetFunctionNameFromPropertyName(result, *name);
         return result;
 
       } else {
@@ -2387,14 +2387,14 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
 
       FunctionKind kind = MethodKindFor(is_generator, is_async);
 
-      if (!*is_static && impl()->IsConstructor(name)) {
+      if (!*is_static && impl()->IsConstructor(*name)) {
         *has_seen_constructor = true;
         kind = has_extends ? FunctionKind::kDerivedConstructor
                            : FunctionKind::kBaseConstructor;
       }
 
       ExpressionT value = impl()->ParseFunctionLiteral(
-          name, scanner()->location(), kSkipFunctionNameCheck, kind,
+          *name, scanner()->location(), kSkipFunctionNameCheck, kind,
           FLAG_harmony_function_tostring ? name_token_position
                                          : kNoSourcePosition,
           FunctionLiteral::kAccessorOrMethod, language_mode(), nullptr,
@@ -2404,7 +2404,7 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
       ClassLiteralPropertyT result = factory()->NewClassLiteralProperty(
           name_expression, value, *property_kind, *is_static,
           *is_computed_name);
-      impl()->SetFunctionNameFromPropertyName(result, name);
+      impl()->SetFunctionNameFromPropertyName(result, *name);
       return result;
     }
 
@@ -2419,14 +2419,14 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
         // Runtime_DefineAccessorPropertyUnchecked and since we can determine
         // this statically we can skip the extra runtime check.
         name_expression =
-            factory()->NewStringLiteral(name, name_expression->position());
+            factory()->NewStringLiteral(*name, name_expression->position());
       }
 
       FunctionKind kind = is_get ? FunctionKind::kGetterFunction
                                  : FunctionKind::kSetterFunction;
 
       FunctionLiteralT value = impl()->ParseFunctionLiteral(
-          name, scanner()->location(), kSkipFunctionNameCheck, kind,
+          *name, scanner()->location(), kSkipFunctionNameCheck, kind,
           FLAG_harmony_function_tostring ? name_token_position
                                          : kNoSourcePosition,
           FunctionLiteral::kAccessorOrMethod, language_mode(), nullptr,
@@ -2440,7 +2440,7 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
       const AstRawString* prefix =
           is_get ? ast_value_factory()->get_space_string()
                  : ast_value_factory()->set_space_string();
-      impl()->SetFunctionNameFromPropertyName(result, name, prefix);
+      impl()->SetFunctionNameFromPropertyName(result, *name, prefix);
       return result;
     }
     case PropertyKind::kSpreadProperty:
@@ -4548,11 +4548,12 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
     bool is_static;
     ClassLiteralProperty::Kind property_kind;
     ExpressionClassifier property_classifier(this);
+    IdentifierT property_name;
     // If we haven't seen the constructor yet, it potentially is the next
     // property.
     bool is_constructor = !class_info.has_seen_constructor;
     ClassLiteralPropertyT property = ParseClassPropertyDefinition(
-        &checker, &class_info, has_extends, &is_computed_name,
+        &checker, &class_info, &property_name, has_extends, &is_computed_name,
         &class_info.has_seen_constructor, &property_kind, &is_static,
         &class_info.has_name_static_property, CHECK_OK);
     if (!class_info.has_static_computed_names && is_static &&
@@ -4567,9 +4568,9 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
     ValidateExpression(CHECK_OK);
     AccumulateFormalParameterContainmentErrors();
 
-    impl()->DeclareClassProperty(name, property, property_kind, is_static,
-                                 is_constructor, is_computed_name, &class_info,
-                                 CHECK_OK);
+    impl()->DeclareClassProperty(name, property, property_name, property_kind,
+                                 is_static, is_constructor, is_computed_name,
+                                 &class_info, CHECK_OK);
     impl()->InferFunctionName();
   }
 

src/parsing/parser.cc

@@ -3316,6 +3316,7 @@ Variable* Parser::CreateSyntheticContextVariable(const AstRawString* name,
 //   - properties
 void Parser::DeclareClassProperty(const AstRawString* class_name,
                                   ClassLiteralProperty* property,
+                                  const AstRawString* property_name,
                                   ClassLiteralProperty::Kind kind,
                                   bool is_static, bool is_constructor,
                                   bool is_computed_name, ClassInfo* class_info,
@@ -3359,8 +3360,8 @@ void Parser::DeclareClassProperty(const AstRawString* class_name,
   }
 
   if (kind == ClassLiteralProperty::PRIVATE_FIELD) {
-    Variable* private_field_name_var = CreateSyntheticContextVariable(
-        property->key()->AsLiteral()->AsRawPropertyName(), CHECK_OK_VOID);
+    Variable* private_field_name_var =
+        CreateSyntheticContextVariable(property_name, CHECK_OK_VOID);
     property->set_private_field_name_var(private_field_name_var);
     class_info->properties->Add(property, zone());
   }

src/parsing/parser.h

@@ -377,6 +377,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
                                       int class_token_pos, bool* ok);
   V8_INLINE void DeclareClassProperty(const AstRawString* class_name,
                                       ClassLiteralProperty* property,
+                                      const AstRawString* property_name,
                                       ClassLiteralProperty::Kind kind,
                                       bool is_static, bool is_constructor,
                                       bool is_computed_name,

src/parsing/preparser.h

@@ -1151,18 +1151,23 @@ class PreParser : public ParserBase<PreParser> {
   }
   V8_INLINE void DeclareClassProperty(const PreParserIdentifier& class_name,
                                       const PreParserExpression& property,
+                                      const PreParserIdentifier& property_name,
                                       ClassLiteralProperty::Kind kind,
                                       bool is_static, bool is_constructor,
                                       bool is_computed_name,
                                       ClassInfo* class_info, bool* ok) {
-    // 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),
           CONST);
     }
+
+    if (kind == ClassLiteralProperty::PRIVATE_FIELD &&
+        property_name.string_ != nullptr) {
+      DCHECK(track_unresolved_variables_);
+      scope()->DeclareVariableName(property_name.string_, CONST);
+    }
   }
 
   V8_INLINE PreParserExpression

test/cctest/parsing/test-preparser.cc

@@ -682,6 +682,12 @@ TEST(PreParserScopeAnalysis) {
          i::FLAG_harmony_public_fields = false;
          i::FLAG_harmony_static_fields = false;
        }},
+      {"class X { #x = 1 }; new X;",
+       [] { i::FLAG_harmony_private_fields = true; },
+       [] { i::FLAG_harmony_private_fields = false; }},
+      {"function t() { return class { #x = 1 }; } new t();",
+       [] { i::FLAG_harmony_private_fields = true; },
+       [] { i::FLAG_harmony_private_fields = false; }},
   };
 
   for (unsigned outer_ix = 0; outer_ix < arraysize(outers); ++outer_ix) {

test/mjsunit/harmony/private-fields.js

@@ -2,7 +2,7 @@
 // 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
+// Flags: --harmony-private-fields --allow-natives-syntax
 
 
 "use strict";