[class] Ban private field deletion

Bug: v8:5368
Change-Id: I7c4f9101837a0bf4917bbb0c2f09587118168a02
Reviewed-on: https://chromium-review.googlesource.com/923362
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51449}

src/messages.h

@@ -259,6 +259,7 @@ class ErrorUtils : public AllStatic {
   T(Debugger, "Debugger: %")                                                   \
   T(DebuggerLoading, "Error loading debugger")                                 \
   T(DefaultOptionsMissing, "Internal % error. Default options are missing.")   \
+  T(DeletePrivateField, "Private fields can not be deleted")                   \
   T(UncaughtException, "Uncaught %")                                           \
   T(Unsupported, "Not supported")                                              \
   T(WrongServiceType, "Internal error, wrong service type: %")                 \

src/parsing/parser-base.h

@@ -3234,13 +3234,19 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseUnaryExpression(
     ExpressionT expression = ParseUnaryExpression(CHECK_OK);
     ValidateExpression(CHECK_OK);
 
-    if (op == Token::DELETE && is_strict(language_mode())) {
-      if (impl()->IsIdentifier(expression)) {
+    if (op == Token::DELETE) {
+      if (impl()->IsIdentifier(expression) && is_strict(language_mode())) {
         // "delete identifier" is a syntax error in strict mode.
         ReportMessage(MessageTemplate::kStrictDelete);
         *ok = false;
         return impl()->NullExpression();
       }
+
+      if (impl()->IsPropertyWithPrivateFieldKey(expression)) {
+        ReportMessage(MessageTemplate::kDeletePrivateField);
+        *ok = false;
+        return impl()->NullExpression();
+      }
     }
 
     if (peek() == Token::EXP) {

src/parsing/parser.cc

@@ -3316,6 +3316,15 @@ void Parser::CheckConflictingVarDeclarations(Scope* scope, bool* ok) {
   }
 }
 
+bool Parser::IsPropertyWithPrivateFieldKey(Expression* expression) {
+  if (!expression->IsProperty()) return false;
+  Property* property = expression->AsProperty();
+
+  if (!property->key()->IsVariableProxy()) return false;
+  VariableProxy* key = property->key()->AsVariableProxy();
+
+  return key->is_private_field();
+}
 
 void Parser::InsertShadowingVarBindingInitializers(Block* inner_block) {
   // For each var-binding that shadows a parameter, insert an assignment

src/parsing/parser.h

@@ -401,6 +401,8 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   // hoisted over such a scope.
   void CheckConflictingVarDeclarations(Scope* scope, bool* ok);
 
+  bool IsPropertyWithPrivateFieldKey(Expression* property);
+
   // Insert initializer statements for var-bindings shadowing parameter bindings
   // from a non-simple parameter list.
   void InsertShadowingVarBindingInitializers(Block* block);

src/parsing/preparser.cc

@@ -49,6 +49,8 @@ PreParserIdentifier GetSymbolHelper(Scanner* scanner) {
       return PreParserIdentifier::Await();
     case Token::ASYNC:
       return PreParserIdentifier::Async();
+    case Token::PRIVATE_NAME:
+      return PreParserIdentifier::PrivateName();
     default:
       break;
   }

src/parsing/preparser.h

@@ -51,6 +51,9 @@ class PreParserIdentifier {
   static PreParserIdentifier Name() {
     return PreParserIdentifier(kNameIdentifier);
   }
+  static PreParserIdentifier PrivateName() {
+    return PreParserIdentifier(kPrivateNameIdentifier);
+  }
   bool IsNull() const { return type_ == kNullIdentifier; }
   bool IsEval() const { return type_ == kEvalIdentifier; }
   bool IsArguments() const { return type_ == kArgumentsIdentifier; }
@@ -58,6 +61,7 @@ class PreParserIdentifier {
   bool IsConstructor() const { return type_ == kConstructorIdentifier; }
   bool IsAwait() const { return type_ == kAwaitIdentifier; }
   bool IsName() const { return type_ == kNameIdentifier; }
+  bool IsPrivateName() const { return type_ == kPrivateNameIdentifier; }
 
  private:
   enum Type {
@@ -68,7 +72,8 @@ class PreParserIdentifier {
     kConstructorIdentifier,
     kAwaitIdentifier,
     kAsyncIdentifier,
-    kNameIdentifier
+    kNameIdentifier,
+    kPrivateNameIdentifier
   };
 
   explicit PreParserIdentifier(Type type) : type_(type), string_(nullptr) {}
@@ -169,6 +174,12 @@ class PreParserExpression {
                                variables);
   }
 
+  static PreParserExpression ThisPropertyWithPrivateFieldKey() {
+    return PreParserExpression(TypeField::encode(kExpression) |
+                               ExpressionTypeField::encode(
+                                   kThisPropertyExpressionWithPrivateFieldKey));
+  }
+
   static PreParserExpression ThisProperty() {
     return PreParserExpression(
         TypeField::encode(kExpression) |
@@ -181,6 +192,12 @@ class PreParserExpression {
         ExpressionTypeField::encode(kPropertyExpression));
   }
 
+  static PreParserExpression PropertyWithPrivateFieldKey() {
+    return PreParserExpression(
+        TypeField::encode(kExpression) |
+        ExpressionTypeField::encode(kPropertyExpressionWithPrivateFieldKey));
+  }
+
   static PreParserExpression Call() {
     return PreParserExpression(TypeField::encode(kExpression) |
                                ExpressionTypeField::encode(kCallExpression));
@@ -254,13 +271,27 @@ class PreParserExpression {
 
   bool IsThisProperty() const {
     return TypeField::decode(code_) == kExpression &&
-           ExpressionTypeField::decode(code_) == kThisPropertyExpression;
+           (ExpressionTypeField::decode(code_) == kThisPropertyExpression ||
+            ExpressionTypeField::decode(code_) ==
+                kThisPropertyExpressionWithPrivateFieldKey);
   }
 
   bool IsProperty() const {
     return TypeField::decode(code_) == kExpression &&
            (ExpressionTypeField::decode(code_) == kPropertyExpression ||
-            ExpressionTypeField::decode(code_) == kThisPropertyExpression);
+            ExpressionTypeField::decode(code_) == kThisPropertyExpression ||
+            ExpressionTypeField::decode(code_) ==
+                kPropertyExpressionWithPrivateFieldKey ||
+            ExpressionTypeField::decode(code_) ==
+                kThisPropertyExpressionWithPrivateFieldKey);
+  }
+
+  bool IsPropertyWithPrivateFieldKey() const {
+    return TypeField::decode(code_) == kExpression &&
+           (ExpressionTypeField::decode(code_) ==
+                kPropertyExpressionWithPrivateFieldKey ||
+            ExpressionTypeField::decode(code_) ==
+                kThisPropertyExpressionWithPrivateFieldKey);
   }
 
   bool IsCall() const {
@@ -301,6 +332,7 @@ class PreParserExpression {
   void set_is_private_field() {
     if (variables_ != nullptr) {
       DCHECK(IsIdentifier());
+      DCHECK(AsIdentifier().IsPrivateName());
       DCHECK_EQ(1, variables_->length());
       variables_->first()->set_is_private_field();
     }
@@ -328,7 +360,9 @@ class PreParserExpression {
   enum ExpressionType {
     kThisExpression,
     kThisPropertyExpression,
+    kThisPropertyExpressionWithPrivateFieldKey,
     kPropertyExpression,
+    kPropertyExpressionWithPrivateFieldKey,
     kCallExpression,
     kCallEvalExpression,
     kCallTaggedTemplateExpression,
@@ -589,8 +623,16 @@ class PreParserFactory {
   PreParserExpression NewVariableProxy(void* variable) {
     return PreParserExpression::Default();
   }
+
   PreParserExpression NewProperty(const PreParserExpression& obj,
                                   const PreParserExpression& key, int pos) {
+    if (key.IsIdentifier() && key.AsIdentifier().IsPrivateName()) {
+      if (obj.IsThis()) {
+        return PreParserExpression::ThisPropertyWithPrivateFieldKey();
+      }
+      return PreParserExpression::PropertyWithPrivateFieldKey();
+    }
+
     if (obj.IsThis()) {
       return PreParserExpression::ThisProperty();
     }
@@ -993,6 +1035,10 @@ class PreParser : public ParserBase<PreParser> {
       TemplateLiteralState* state, int start, const PreParserExpression& tag) {
     return PreParserExpression::Default();
   }
+  V8_INLINE bool IsPropertyWithPrivateFieldKey(
+      const PreParserExpression& expression) {
+    return expression.IsPropertyWithPrivateFieldKey();
+  }
   V8_INLINE void CheckConflictingVarDeclarations(Scope* scope, bool* ok) {}
 
   V8_INLINE void SetLanguageMode(Scope* scope, LanguageMode mode) {

test/cctest/interpreter/bytecode_expectations/AsyncGenerators.golden

@@ -367,7 +367,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(18),
                 B(LdaConstant), U8(15),
                 B(Star), R(19),

test/cctest/interpreter/bytecode_expectations/ForAwaitOf.golden

@@ -124,7 +124,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(19),
                 B(LdaConstant), U8(12),
                 B(Star), R(20),
@@ -378,7 +378,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(19),
                 B(LdaConstant), U8(12),
                 B(Star), R(20),
@@ -654,7 +654,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(19),
                 B(LdaConstant), U8(12),
                 B(Star), R(20),
@@ -886,7 +886,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(17),
                 B(LdaConstant), U8(10),
                 B(Star), R(18),

test/cctest/interpreter/bytecode_expectations/ForOf.golden

@@ -86,7 +86,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(12),
                 B(LdaConstant), U8(8),
                 B(Star), R(13),
@@ -220,7 +220,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(13),
                 B(LdaConstant), U8(8),
                 B(Star), R(14),
@@ -366,7 +366,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(12),
                 B(LdaConstant), U8(8),
                 B(Star), R(13),
@@ -502,7 +502,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(11),
                 B(LdaConstant), U8(10),
                 B(Star), R(12),

test/cctest/interpreter/bytecode_expectations/ForOfLoop.golden

@@ -90,7 +90,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(14),
                 B(LdaConstant), U8(7),
                 B(Star), R(15),
@@ -261,7 +261,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(14),
                 B(LdaConstant), U8(11),
                 B(Star), R(15),
@@ -408,7 +408,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(12),
                 B(LdaConstant), U8(9),
                 B(Star), R(13),
@@ -503,7 +503,7 @@ bytecodes: [
                 B(JumpIfUndefined), U8(6),
                 B(Ldar), R(6),
                 B(JumpIfNotNull), U8(16),
-                B(LdaSmi), I8(75),
+                B(LdaSmi), I8(76),
                 B(Star), R(18),
                 B(LdaConstant), U8(4),
                 B(Star), R(19),
@@ -559,7 +559,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(17),
                 B(LdaConstant), U8(9),
                 B(Star), R(18),
@@ -713,7 +713,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(16),
                 B(LdaConstant), U8(10),
                 B(Star), R(17),
@@ -882,7 +882,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(15),
                 B(LdaConstant), U8(13),
                 B(Star), R(16),
@@ -1037,7 +1037,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(21),
                 B(LdaConstant), U8(7),
                 B(Star), R(22),
@@ -1253,7 +1253,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(20),
                 B(LdaConstant), U8(8),
                 B(Star), R(21),

test/cctest/interpreter/bytecode_expectations/Generators.golden

@@ -204,7 +204,7 @@ bytecodes: [
                 B(TestTypeOf), U8(6),
                 B(JumpIfFalse), U8(4),
                 B(Jump), U8(18),
-                B(Wide), B(LdaSmi), I16(145),
+                B(Wide), B(LdaSmi), I16(146),
                 B(Star), R(14),
                 B(LdaConstant), U8(14),
                 B(Star), R(15),

test/cctest/interpreter/bytecode_expectations/StandardForLoop.golden

@@ -231,7 +231,7 @@ bytecodes: [
                 B(JumpIfUndefined), U8(6),
                 B(Ldar), R(3),
                 B(JumpIfNotNull), U8(16),
-                B(LdaSmi), I8(75),
+                B(LdaSmi), I8(76),
                 B(Star), R(4),
                 B(LdaConstant), U8(1),
                 B(Star), R(5),

test/cctest/test-parsing.cc

@@ -4940,6 +4940,14 @@ TEST(PrivateClassFieldsErrors) {
     "#a = f(arguments)",
     "#a = () => () => arguments",
 
+    "foo() { delete this.#a }",
+    "foo() { delete this.x.#a }",
+    "foo() { delete this.x().#a }",
+
+    "foo() { delete f.#a }",
+    "foo() { delete f.x.#a }",
+    "foo() { delete f.x().#a }",
+
     // ASI requires a linebreak
     "#a b",
     "#a = 0 b",