Implement proxy trap deleteProperty in Torque, apply to reflect

Reflect.deleteProperty now is a Torque builtins, also containing fast
path for proxy object.

Bug: v8:6664
Change-Id: I76d6fba2c9d05d991132957783d987a190585ec8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1704943
Commit-Queue: Z Nguyen-Huu <duongn@microsoft.com>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62813}

BUILD.gn

@@ -956,6 +956,7 @@ torque_files = [
   "src/builtins/object-fromentries.tq",
   "src/builtins/object.tq",
   "src/builtins/proxy-constructor.tq",
+  "src/builtins/proxy-delete-property.tq",
   "src/builtins/proxy-get-property.tq",
   "src/builtins/proxy-get-prototype-of.tq",
   "src/builtins/proxy-has-property.tq",

src/builtins/base.tq

@@ -1481,7 +1481,7 @@ extern transitioning builtin SetProperty(implicit context: Context)(
 extern transitioning builtin SetPropertyInLiteral(implicit context: Context)(
     Object, Object, Object);
 extern transitioning builtin DeleteProperty(implicit context: Context)(
-    Object, Object, LanguageMode);
+    Object, Object, LanguageMode): Object;
 extern transitioning builtin HasProperty(implicit context: Context)(
     Object, Object): Boolean;
 extern transitioning macro HasProperty_Inline(implicit context: Context)(
@@ -2142,9 +2142,9 @@ extern macro Float64Constant(constexpr float64): float64;
 extern macro SmiConstant(constexpr int31): Smi;
 extern macro SmiConstant(constexpr Smi): Smi;
 extern macro SmiConstant(constexpr MessageTemplate): Smi;
+extern macro SmiConstant(constexpr LanguageMode): Smi;
 extern macro BoolConstant(constexpr bool): bool;
 extern macro StringConstant(constexpr string): String;
-extern macro LanguageModeConstant(constexpr LanguageMode): LanguageMode;
 extern macro Int32Constant(constexpr ElementsKind): ElementsKind;
 extern macro IntPtrConstant(constexpr NativeContextSlot): NativeContextSlot;
 extern macro IntPtrConstant(constexpr ContextSlot): ContextSlot;

src/builtins/builtins-definitions.h

@@ -821,7 +821,6 @@ namespace internal {
   ASM(ReflectApply, Dummy)                                                     \
   ASM(ReflectConstruct, Dummy)                                                 \
   CPP(ReflectDefineProperty)                                                   \
-  CPP(ReflectDeleteProperty)                                                   \
   CPP(ReflectGetOwnPropertyDescriptor)                                         \
   TFJ(ReflectHas, 2, kReceiver, kTarget, kKey)                                 \
   CPP(ReflectOwnKeys)                                                          \

src/builtins/builtins-proxy-gen.cc

@@ -421,5 +421,63 @@ void ProxiesCodeStubAssembler::CheckHasTrapResult(TNode<Context> context,
   BIND(&check_passed);
 }
 
+void ProxiesCodeStubAssembler::CheckDeleteTrapResult(TNode<Context> context,
+                                                     TNode<JSReceiver> target,
+                                                     TNode<JSProxy> proxy,
+                                                     TNode<Name> name) {
+  TNode<Map> target_map = LoadMap(target);
+  TVARIABLE(Object, var_value);
+  TVARIABLE(Uint32T, var_details);
+  TVARIABLE(Object, var_raw_value);
+
+  Label if_found_value(this, Label::kDeferred),
+      throw_non_configurable(this, Label::kDeferred),
+      throw_non_extensible(this, Label::kDeferred), check_passed(this),
+      check_in_runtime(this, Label::kDeferred);
+
+  // 10. Let targetDesc be ? target.[[GetOwnProperty]](P).
+  GotoIfNot(IsUniqueNameNoIndex(name), &check_in_runtime);
+  TNode<Int32T> instance_type = LoadInstanceType(target);
+  TryGetOwnProperty(context, target, target, target_map, instance_type, name,
+                    &if_found_value, &var_value, &var_details, &var_raw_value,
+                    &check_passed, &check_in_runtime, kReturnAccessorPair);
+
+  // 11. If targetDesc is undefined, return true.
+  BIND(&if_found_value);
+  {
+    // 12. If targetDesc.[[Configurable]] is false, throw a TypeError exception.
+    TNode<BoolT> non_configurable = IsSetWord32(
+        var_details.value(), PropertyDetails::kAttributesDontDeleteMask);
+    GotoIf(non_configurable, &throw_non_configurable);
+
+    // 13. Let extensibleTarget be ? IsExtensible(target).
+    TNode<BoolT> target_extensible = IsExtensibleMap(target_map);
+
+    // 14. If extensibleTarget is false, throw a TypeError exception.
+    GotoIfNot(target_extensible, &throw_non_extensible);
+    Goto(&check_passed);
+  }
+
+  BIND(&throw_non_configurable);
+  {
+    ThrowTypeError(context,
+                   MessageTemplate::kProxyDeletePropertyNonConfigurable, name);
+  }
+
+  BIND(&throw_non_extensible);
+  {
+    ThrowTypeError(context, MessageTemplate::kProxyDeletePropertyNonExtensible,
+                   name);
+  }
+
+  BIND(&check_in_runtime);
+  {
+    CallRuntime(Runtime::kCheckProxyDeleteTrapResult, context, name, target);
+    Goto(&check_passed);
+  }
+
+  BIND(&check_passed);
+}
+
 }  // namespace internal
 }  // namespace v8

src/builtins/builtins-proxy-gen.h

@@ -30,6 +30,9 @@ class ProxiesCodeStubAssembler : public CodeStubAssembler {
   void CheckHasTrapResult(TNode<Context> context, TNode<JSReceiver> target,
                           TNode<JSProxy> proxy, TNode<Name> name);
 
+  void CheckDeleteTrapResult(TNode<Context> context, TNode<JSReceiver> target,
+                             TNode<JSProxy> proxy, TNode<Name> name);
+
  protected:
   enum ProxyRevokeFunctionContextSlot {
     kProxySlot = Context::MIN_CONTEXT_SLOTS,

src/builtins/builtins-reflect.cc

@@ -46,30 +46,6 @@ BUILTIN(ReflectDefineProperty) {
   return *isolate->factory()->ToBoolean(result.FromJust());
 }
 
-// ES6 section 26.1.4 Reflect.deleteProperty
-BUILTIN(ReflectDeleteProperty) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(3, args.length());
-  Handle<Object> target = args.at(1);
-  Handle<Object> key = args.at(2);
-
-  if (!target->IsJSReceiver()) {
-    THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate, NewTypeError(MessageTemplate::kCalledOnNonObject,
-                              isolate->factory()->NewStringFromAsciiChecked(
-                                  "Reflect.deleteProperty")));
-  }
-
-  Handle<Name> name;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, name,
-                                     Object::ToName(isolate, key));
-
-  Maybe<bool> result = JSReceiver::DeletePropertyOrElement(
-      Handle<JSReceiver>::cast(target), name, LanguageMode::kSloppy);
-  MAYBE_RETURN(result, ReadOnlyRoots(isolate).exception());
-  return *isolate->factory()->ToBoolean(result.FromJust());
-}
-
 // ES6 section 26.1.7 Reflect.getOwnPropertyDescriptor
 BUILTIN(ReflectGetOwnPropertyDescriptor) {
   HandleScope scope(isolate);

src/builtins/proxy-delete-property.tq

@@ -0,0 +1,67 @@
+// Copyright 2019 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/builtins/builtins-proxy-gen.h'
+
+namespace proxy {
+
+  // ES #sec-proxy-object-internal-methods-and-internal-slots-delete-p
+  // https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-delete-p
+  transitioning builtin
+  ProxyDeleteProperty(implicit context: Context)(
+      proxy: JSProxy, name: Name, languageMode: LanguageMode): Object {
+    const kTrapName: constexpr string = 'deleteProperty';
+    // 1. Assert: IsPropertyKey(P) is true.
+    assert(TaggedIsNotSmi(name));
+    assert(IsName(name));
+    assert(!IsPrivateSymbol(name));
+
+    try {
+      // 2. Let handler be O.[[ProxyHandler]].
+      // 3. If handler is null, throw a TypeError exception.
+      // 4. Assert: Type(handler) is Object.
+      assert(proxy.handler == Null || Is<JSReceiver>(proxy.handler));
+      const handler =
+          Cast<JSReceiver>(proxy.handler) otherwise ThrowProxyHandlerRevoked;
+
+      // 5. Let target be O.[[ProxyTarget]].
+      const target = UnsafeCast<JSReceiver>(proxy.target);
+
+      // 6. Let trap be ? GetMethod(handler, "deleteProperty").
+      // 7. If trap is undefined, then (see 7.a below).
+      const trap: Callable = GetMethod(handler, kTrapName)
+          otherwise goto TrapUndefined(target);
+
+      // 8. Let booleanTrapResult be ToBoolean(? Call(trap, handler,
+      // « target, P »)).
+      const trapResult = Call(context, trap, handler, target, name);
+
+      // 9. If booleanTrapResult is false, return false.
+      if (BranchIfToBooleanIsFalse(trapResult)) {
+        if (languageMode == SmiConstant(kStrict)) {
+          ThrowTypeError(kProxyTrapReturnedFalsishFor, kTrapName, name);
+        }
+        return False;
+      }
+
+      // 10. Let targetDesc be ? target.[[GetOwnProperty]](P).
+      // 11. If targetDesc is undefined, return true.
+      // 12. If targetDesc.[[Configurable]] is false, throw a TypeError
+      // exception.
+      // 13. Let extensibleTarget be ? IsExtensible(target).
+      // 14. If extensibleTarget is false, throw a TypeError exception.
+      CheckDeleteTrapResult(target, proxy, name);
+
+      // 15. Return true.
+      return True;
+    }
+    label TrapUndefined(target: Object) {
+      // 7.a. Return ? target.[[Delete]](P).
+      return DeleteProperty(target, name, languageMode);
+    }
+    label ThrowProxyHandlerRevoked deferred {
+      ThrowTypeError(kProxyRevoked, kTrapName);
+    }
+  }
+}

src/builtins/proxy.tq

@@ -19,6 +19,9 @@ namespace proxy {
       implicit context:
           Context)(JSReceiver, JSProxy, Name, Object, constexpr int31);
 
+  extern transitioning macro ProxiesCodeStubAssembler::CheckDeleteTrapResult(
+      implicit context: Context)(JSReceiver, JSProxy, Name);
+
   extern transitioning macro ProxiesCodeStubAssembler::CheckHasTrapResult(
       implicit context: Context)(JSReceiver, JSProxy, Name);
 
@@ -44,6 +47,8 @@ namespace proxy {
   generates 'MessageTemplate::kProxyGetPrototypeOfNonExtensible';
   const kProxySetPrototypeOfNonExtensible: constexpr MessageTemplate
   generates 'MessageTemplate::kProxySetPrototypeOfNonExtensible';
+  const kProxyDeletePropertyNonExtensible: constexpr MessageTemplate
+  generates 'MessageTemplate::kProxyDeletePropertyNonExtensible';
 
   const kProxyGet: constexpr int31
   generates 'JSProxy::AccessKind::kGet';

src/builtins/reflect.tq

@@ -64,4 +64,19 @@ namespace reflect {
     return GetPropertyWithReceiver(
         objectJSReceiver, name, receiver, SmiConstant(kReturnUndefined));
   }
+
+  // ES6 section 26.1.4 Reflect.deleteProperty
+  transitioning javascript builtin ReflectDeleteProperty(
+      js-implicit context:
+          Context)(_receiver: Object, object: Object, key: Object): Object {
+    const objectJSReceiver = Cast<JSReceiver>(object)
+        otherwise ThrowTypeError(kCalledOnNonObject, 'Reflect.deleteProperty');
+    const name: Name = ToName(key);
+    if (IsPrivateSymbol(name)) {
+      return DeleteProperty(objectJSReceiver, name, kSloppy);
+    }
+    const proxy = Cast<JSProxy>(objectJSReceiver)
+        otherwise return DeleteProperty(objectJSReceiver, name, kSloppy);
+    return proxy::ProxyDeleteProperty(proxy, name, kSloppy);
+  }
 }  // namespace reflect

src/objects/js-proxy.h

@@ -63,6 +63,10 @@ class JSProxy : public TorqueGeneratedJSProxy<JSProxy, JSReceiver> {
   V8_WARN_UNUSED_RESULT static Maybe<bool> CheckHasTrap(
       Isolate* isolate, Handle<Name> name, Handle<JSReceiver> target);
 
+  // ES6 9.5.10
+  V8_WARN_UNUSED_RESULT static Maybe<bool> CheckDeleteTrap(
+      Isolate* isolate, Handle<Name> name, Handle<JSReceiver> target);
+
   // ES6 9.5.8
   V8_WARN_UNUSED_RESULT static MaybeHandle<Object> GetProperty(
       Isolate* isolate, Handle<JSProxy> proxy, Handle<Name> name,

src/objects/objects.cc

@@ -3046,27 +3046,34 @@ Maybe<bool> JSProxy::DeletePropertyOrElement(Handle<JSProxy> proxy,
   }
 
   // Enforce the invariant.
+  return JSProxy::CheckDeleteTrap(isolate, name, target);
+}
+
+Maybe<bool> JSProxy::CheckDeleteTrap(Isolate* isolate, Handle<Name> name,
+                                     Handle<JSReceiver> target) {
+  // 10. Let targetDesc be ? target.[[GetOwnProperty]](P).
   PropertyDescriptor target_desc;
-  Maybe<bool> owned =
+  Maybe<bool> target_found =
       JSReceiver::GetOwnPropertyDescriptor(isolate, target, name, &target_desc);
-  MAYBE_RETURN(owned, Nothing<bool>());
-  if (owned.FromJust()) {
+  MAYBE_RETURN(target_found, Nothing<bool>());
+  // 11. If targetDesc is undefined, return true.
+  if (target_found.FromJust()) {
+    // 12. If targetDesc.[[Configurable]] is false, throw a TypeError exception.
     if (!target_desc.configurable()) {
-      isolate->Throw(*factory->NewTypeError(
+      isolate->Throw(*isolate->factory()->NewTypeError(
           MessageTemplate::kProxyDeletePropertyNonConfigurable, name));
       return Nothing<bool>();
     }
     // 13. Let extensibleTarget be ? IsExtensible(target).
+    Maybe<bool> extensible_target = JSReceiver::IsExtensible(target);
+    MAYBE_RETURN(extensible_target, Nothing<bool>());
     // 14. If extensibleTarget is false, throw a TypeError exception.
-    Maybe<bool> extensible = JSReceiver::IsExtensible(target);
-    MAYBE_RETURN(extensible, Nothing<bool>());
-    if (!extensible.FromJust()) {
-      isolate->Throw(*factory->NewTypeError(
+    if (!extensible_target.FromJust()) {
+      isolate->Throw(*isolate->factory()->NewTypeError(
           MessageTemplate::kProxyDeletePropertyNonExtensible, name));
       return Nothing<bool>();
     }
   }
-
   return Just(true);
 }
 

src/runtime/runtime-proxy.cc

@@ -86,5 +86,17 @@ RUNTIME_FUNCTION(Runtime_CheckProxyHasTrapResult) {
   return isolate->heap()->ToBoolean(result.FromJust());
 }
 
+RUNTIME_FUNCTION(Runtime_CheckProxyDeleteTrapResult) {
+  HandleScope scope(isolate);
+
+  DCHECK_EQ(2, args.length());
+  CONVERT_ARG_HANDLE_CHECKED(Name, name, 0);
+  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, target, 1);
+
+  Maybe<bool> result = JSProxy::CheckDeleteTrap(isolate, name, target);
+  if (!result.IsJust()) return ReadOnlyRoots(isolate).exception();
+  return isolate->heap()->ToBoolean(result.FromJust());
+}
+
 }  // namespace internal
 }  // namespace v8

src/runtime/runtime.h

@@ -362,6 +362,7 @@ namespace internal {
 #define FOR_EACH_INTRINSIC_PROXY(F, I) \
   F(CheckProxyGetSetTrapResult, 2, 1)  \
   F(CheckProxyHasTrapResult, 2, 1)     \
+  F(CheckProxyDeleteTrapResult, 2, 1)  \
   F(GetPropertyWithReceiver, 3, 1)     \
   F(SetPropertyWithReceiver, 4, 1)