[proxies] Teach ToPropertyDescriptor to deal with Proxies

src/property-descriptor.cc

 // Copyright 2011 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/property-descriptor.h"
 
 #include "src/bootstrapper.h"
 #include "src/factory.h"
 #include "src/isolate-inl.h"
 #include "src/lookup.h"
 #include "src/objects-inl.h"
 
 namespace v8 {
 namespace internal {
 
 // Helper function for ToPropertyDescriptor. Comments describe steps for
 // "enumerable", other properties are handled the same way.
 // Returns false if an exception was thrown.
 bool GetPropertyIfPresent(Handle<Object> obj, Handle<String> name,

[Camillo Bruni, 2015/11/12 13:52:19]

Probably this will (as so many other methods) need a Maybe<bool> result.

[Jakob Kummerow, 2015/11/12 14:01:45]

Maybe. Let's introduce them as needed.

                           Handle<Object>* value) {
   LookupIterator it(obj, name);
   // 4. Let hasEnumerable be HasProperty(Obj, "enumerable").
-  Maybe<PropertyAttributes> maybe_attr = JSReceiver::GetPropertyAttributes(&it);
+  Maybe<bool> has_property = JSReceiver::HasProperty(&it);
   // 5. ReturnIfAbrupt(hasEnumerable).
-  if (!maybe_attr.IsJust()) return false;
+  if (has_property.IsNothing()) return false;
   // 6. If hasEnumerable is true, then
-  if (maybe_attr.FromJust() != ABSENT) {
+  if (has_property.FromJust() == true) {
     // 6a. Let enum be ToBoolean(Get(Obj, "enumerable")).
     // 6b. ReturnIfAbrupt(enum).
     if (!JSObject::GetProperty(&it).ToHandle(value)) return false;
   }
   return true;
 }
 
 
 // Helper function for ToPropertyDescriptor. Handles the case of "simple"
 // objects: nothing on the prototype chain, just own fast data properties.
@@ skipping 115 matching lines @@
         MessageTemplate::kPropertyDescObject, obj));
     return false;
   }
   // 3. Let desc be a new Property Descriptor that initially has no fields.
   DCHECK(desc->is_empty());
 
   if (ToPropertyDescriptorFastPath(isolate, obj, desc)) {
     return true;
   }
 
-  // TODO(jkummerow): Implement JSProxy support.
-  // Specifically, instead of taking the attributes != ABSENT shortcut, we
-  // have to implement proper HasProperty for proxies.
-  if (!obj->IsJSProxy()) {
-    {  // enumerable?
-      Handle<Object> enumerable;
-      // 4 through 6b.
-      if (!GetPropertyIfPresent(obj, isolate->factory()->enumerable_string(),
-                                &enumerable)) {
-        return false;
-      }
-      // 6c. Set the [[Enumerable]] field of desc to enum.
-      if (!enumerable.is_null()) {
-        desc->set_enumerable(enumerable->BooleanValue());
-      }
-    }
-    {  // configurable?
-      Handle<Object> configurable;
-      // 7 through 9b.
-      if (!GetPropertyIfPresent(obj, isolate->factory()->configurable_string(),
-                                &configurable)) {
-        return false;
-      }
-      // 9c. Set the [[Configurable]] field of desc to conf.
-      if (!configurable.is_null()) {
-        desc->set_configurable(configurable->BooleanValue());
-      }
-    }
-    {  // value?
-      Handle<Object> value;
-      // 10 through 12b.
-      if (!GetPropertyIfPresent(obj, isolate->factory()->value_string(),
-                                &value))
-        return false;
-      // 12c. Set the [[Value]] field of desc to value.
-      if (!value.is_null()) desc->set_value(value);
-    }
-    {  // writable?
-      Handle<Object> writable;
-      // 13 through 15b.
-      if (!GetPropertyIfPresent(obj, isolate->factory()->writable_string(),
-                                &writable)) {
-        return false;
-      }
-      // 15c. Set the [[Writable]] field of desc to writable.
-      if (!writable.is_null()) desc->set_writable(writable->BooleanValue());
-    }
-    {  // getter?
-      Handle<Object> getter;
-      // 16 through 18b.
-      if (!GetPropertyIfPresent(obj, isolate->factory()->get_string(), &getter))
-        return false;
-      if (!getter.is_null()) {
-        // 18c. If IsCallable(getter) is false and getter is not undefined,
-        // throw a TypeError exception.
-        if (!getter->IsCallable() && !getter->IsUndefined()) {
-          isolate->Throw(*isolate->factory()->NewTypeError(
-              MessageTemplate::kObjectGetterCallable, getter));
-          return false;
-        }
-        // 18d. Set the [[Get]] field of desc to getter.
-        desc->set_get(getter);
-      }
-      {  // setter?
-        Handle<Object> setter;
-        // 19 through 21b.
-        if (!GetPropertyIfPresent(obj, isolate->factory()->set_string(),
-                                  &setter))
-          return false;
-        if (!setter.is_null()) {
-          // 21c. If IsCallable(setter) is false and setter is not undefined,
-          // throw a TypeError exception.
-          if (!setter->IsCallable() && !setter->IsUndefined()) {
-            isolate->Throw(*isolate->factory()->NewTypeError(
-                MessageTemplate::kObjectSetterCallable, setter));
-            return false;
-          }
-          // 21d. Set the [[Set]] field of desc to setter.
-          desc->set_set(setter);
-        }
-      }
-      // 22. If either desc.[[Get]] or desc.[[Set]] is present, then
-      // 22a. If either desc.[[Value]] or desc.[[Writable]] is present,
-      // throw a TypeError exception.
-      if ((desc->has_get() || desc->has_set()) &&
-          (desc->has_value() || desc->has_writable())) {
-        isolate->Throw(*isolate->factory()->NewTypeError(
-            MessageTemplate::kValueAndAccessor, obj));
-        return false;
-      }
-    }
-  } else {
-    DCHECK(obj->IsJSProxy());
-    // Having an UNIMPLEMENTED() here would upset ClusterFuzz, because
-    // --harmony-proxies makes it possible to reach this branch.
-    isolate->Throw(
-        *isolate->factory()->NewTypeError(MessageTemplate::kUnsupported));
+  // enumerable?

[Jakob Kummerow, 2015/11/12 13:44:45]

This is the same as old lines 167 through 253 with different indentation.

+  Handle<Object> enumerable;
+  // 4 through 6b.
+  if (!GetPropertyIfPresent(obj, isolate->factory()->enumerable_string(),
+                            &enumerable)) {
     return false;
   }
+  // 6c. Set the [[Enumerable]] field of desc to enum.
+  if (!enumerable.is_null()) {
+    desc->set_enumerable(enumerable->BooleanValue());
+  }
+
+  // configurable?
+  Handle<Object> configurable;
+  // 7 through 9b.
+  if (!GetPropertyIfPresent(obj, isolate->factory()->configurable_string(),
+                            &configurable)) {
+    return false;
+  }
+  // 9c. Set the [[Configurable]] field of desc to conf.
+  if (!configurable.is_null()) {
+    desc->set_configurable(configurable->BooleanValue());
+  }
+
+  // value?
+  Handle<Object> value;
+  // 10 through 12b.
+  if (!GetPropertyIfPresent(obj, isolate->factory()->value_string(), &value)) {
+    return false;
+  }
+  // 12c. Set the [[Value]] field of desc to value.
+  if (!value.is_null()) desc->set_value(value);
+
+  // writable?
+  Handle<Object> writable;
+  // 13 through 15b.
+  if (!GetPropertyIfPresent(obj, isolate->factory()->writable_string(),
+                            &writable)) {
+    return false;
+  }
+  // 15c. Set the [[Writable]] field of desc to writable.
+  if (!writable.is_null()) desc->set_writable(writable->BooleanValue());
+
+  // getter?
+  Handle<Object> getter;
+  // 16 through 18b.
+  if (!GetPropertyIfPresent(obj, isolate->factory()->get_string(), &getter)) {
+    return false;
+  }
+  if (!getter.is_null()) {
+    // 18c. If IsCallable(getter) is false and getter is not undefined,
+    // throw a TypeError exception.
+    if (!getter->IsCallable() && !getter->IsUndefined()) {
+      isolate->Throw(*isolate->factory()->NewTypeError(
+          MessageTemplate::kObjectGetterCallable, getter));
+      return false;
+    }
+    // 18d. Set the [[Get]] field of desc to getter.
+    desc->set_get(getter);
+  }
+  // setter?
+  Handle<Object> setter;
+  // 19 through 21b.
+  if (!GetPropertyIfPresent(obj, isolate->factory()->set_string(), &setter)) {
+    return false;
+  }
+  if (!setter.is_null()) {
+    // 21c. If IsCallable(setter) is false and setter is not undefined,
+    // throw a TypeError exception.
+    if (!setter->IsCallable() && !setter->IsUndefined()) {
+      isolate->Throw(*isolate->factory()->NewTypeError(
+          MessageTemplate::kObjectSetterCallable, setter));
+      return false;
+    }
+    // 21d. Set the [[Set]] field of desc to setter.
+    desc->set_set(setter);
+  }
+
+  // 22. If either desc.[[Get]] or desc.[[Set]] is present, then
+  // 22a. If either desc.[[Value]] or desc.[[Writable]] is present,
+  // throw a TypeError exception.
+  if ((desc->has_get() || desc->has_set()) &&
+      (desc->has_value() || desc->has_writable())) {
+    isolate->Throw(*isolate->factory()->NewTypeError(
+        MessageTemplate::kValueAndAccessor, obj));
+    return false;
+  }
+
   // 23. Return desc.
   return true;
 }
 
 
+// ES6 6.2.4.6
+// static
+void PropertyDescriptor::CompletePropertyDescriptor(Isolate* isolate,

[Jakob Kummerow, 2015/11/12 13:44:45]

I'm not using this yet, but my next CL is going to need it.

+                                                    PropertyDescriptor* desc) {
+  // 1. ReturnIfAbrupt(Desc).
+  // 2. Assert: Desc is a Property Descriptor.
+  // 3. Let like be Record{
+  //        [[Value]]: undefined, [[Writable]]: false,
+  //        [[Get]]: undefined, [[Set]]: undefined,
+  //        [[Enumerable]]: false, [[Configurable]]: false}.
+  // 4. If either IsGenericDescriptor(Desc) or IsDataDescriptor(Desc) is true,
+  // then:
+  if (!IsAccessorDescriptor(desc)) {
+    // 4a. If Desc does not have a [[Value]] field, set Desc.[[Value]] to
+    //     like.[[Value]].
+    if (!desc->has_value()) {
+      desc->set_value(isolate->factory()->undefined_value());
+    }
+    // 4b. If Desc does not have a [[Writable]] field, set Desc.[[Writable]]
+    //     to like.[[Writable]].
+    if (!desc->has_writable()) desc->set_writable(false);
+  } else {
+    // 5. Else,
+    // 5a. If Desc does not have a [[Get]] field, set Desc.[[Get]] to
+    //     like.[[Get]].
+    if (!desc->has_get()) {
+      desc->set_get(isolate->factory()->undefined_value());
+    }
+    // 5b. If Desc does not have a [[Set]] field, set Desc.[[Set]] to
+    //     like.[[Set]].
+    if (!desc->has_set()) {
+      desc->set_set(isolate->factory()->undefined_value());
+    }
+  }
+  // 6. If Desc does not have an [[Enumerable]] field, set
+  //    Desc.[[Enumerable]] to like.[[Enumerable]].
+  if (!desc->has_enumerable()) desc->set_enumerable(false);
+  // 7. If Desc does not have a [[Configurable]] field, set
+  //    Desc.[[Configurable]] to like.[[Configurable]].
+  if (!desc->has_configurable()) desc->set_configurable(false);
+  // 8. Return Desc.
+}
+
 }  // namespace internal
 }  // namespace v8