Clean up some "GetProperty" methods/functions.

src/objects.cc

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 469 matching lines @@
 
   Handle<Object> args[] = { receiver, name };
   Handle<Object> result = CallTrap(
     "get", isolate->derived_get_trap(), ARRAY_SIZE(args), args);
   if (isolate->has_pending_exception()) return Failure::Exception();
 
   return *result;
 }
 
 
+Handle<Object> Object::GetPropertyOrElement(Handle<Object> object,
+                                            Handle<Name> name) {
+  uint32_t index;
+  Isolate* isolate = name->GetIsolate();
+  if (name->AsArrayIndex(&index)) return GetElement(isolate, object, index);
+  return GetProperty(object, name);
+}
+
+
 Handle<Object> Object::GetProperty(Handle<Object> object,
                                    Handle<Name> name) {
-  // TODO(rossberg): The index test should not be here but in the GetProperty
-  // method (or somewhere else entirely). Needs more global clean-up.
-  uint32_t index;
-  Isolate* isolate = name->GetIsolate();
-  if (name->AsArrayIndex(&index)) return GetElement(isolate, object, index);
-  CALL_HEAP_FUNCTION(isolate, object->GetProperty(*name), Object);
+  CALL_HEAP_FUNCTION(name->GetIsolate(), object->GetProperty(*name), Object);
 }
 
 
 MaybeObject* JSProxy::GetElementWithHandler(Object* receiver,
                                             uint32_t index) {
   String* name;
   MaybeObject* maybe = GetHeap()->Uint32ToString(index);
   if (!maybe->To<String>(&name)) return maybe;
   return GetPropertyWithHandler(receiver, name);
 }
@@ skipping 3084 matching lines @@
   Handle<Object> argv[] = { result };
   Handle<Object> desc = Execution::Call(
       isolate, isolate->to_complete_property_descriptor(), result,
       ARRAY_SIZE(argv), argv, &has_pending_exception);
   if (has_pending_exception) return Handle<Object>();
 
   // [[GetProperty]] requires to check that all properties are configurable.
   Handle<String> configurable_name =
       isolate->factory()->InternalizeOneByteString(
           STATIC_ASCII_VECTOR("configurable_"));
-  Handle<Object> configurable(
-      v8::internal::GetProperty(isolate, desc, configurable_name));
-  ASSERT(!isolate->has_pending_exception());
+  Handle<Object> configurable = Object::GetProperty(desc, configurable_name);

[Igor Sheludko, 2014/03/26 18:37:31]

Is it possible to get undefined or null as a desc here? If not is it worth
ASSERTing about that?

[Yang, 2014/03/28 09:56:58]

I think it's alright the way it is. This is purely mechanical after all.

+  ASSERT(!configurable.is_null());
   ASSERT(configurable->IsTrue() || configurable->IsFalse());
   if (configurable->IsFalse()) {
     Handle<String> trap =
         isolate->factory()->InternalizeOneByteString(
             STATIC_ASCII_VECTOR("getPropertyDescriptor"));
     Handle<Object> args[] = { handler, trap, name };
     Handle<Object> error = isolate->factory()->NewTypeError(
         "proxy_prop_not_configurable", HandleVector(args, ARRAY_SIZE(args)));
     isolate->Throw(*error);
     return Handle<Object>();
   }
   ASSERT(configurable->IsTrue());
 
   // Check for DataDescriptor.
   Handle<String> hasWritable_name =
       isolate->factory()->InternalizeOneByteString(
           STATIC_ASCII_VECTOR("hasWritable_"));
-  Handle<Object> hasWritable(
-      v8::internal::GetProperty(isolate, desc, hasWritable_name));
-  ASSERT(!isolate->has_pending_exception());
+  Handle<Object> hasWritable = Object::GetProperty(desc, hasWritable_name);
+  ASSERT(!hasWritable.is_null());
   ASSERT(hasWritable->IsTrue() || hasWritable->IsFalse());
   if (hasWritable->IsTrue()) {
     Handle<String> writable_name =
         isolate->factory()->InternalizeOneByteString(
             STATIC_ASCII_VECTOR("writable_"));
-    Handle<Object> writable(
-        v8::internal::GetProperty(isolate, desc, writable_name));
-    ASSERT(!isolate->has_pending_exception());
+    Handle<Object> writable = Object::GetProperty(desc, writable_name);
+    ASSERT(!writable.is_null());
     ASSERT(writable->IsTrue() || writable->IsFalse());
     *done = writable->IsFalse();
     if (!*done) return isolate->factory()->the_hole_value();
     if (strict_mode == SLOPPY) return value;
     Handle<Object> args[] = { name, receiver };
     Handle<Object> error = isolate->factory()->NewTypeError(
         "strict_read_only_property", HandleVector(args, ARRAY_SIZE(args)));
     isolate->Throw(*error);
     return Handle<Object>();
   }
 
   // We have an AccessorDescriptor.
   Handle<String> set_name = isolate->factory()->InternalizeOneByteString(
       STATIC_ASCII_VECTOR("set_"));
-  Handle<Object> setter(v8::internal::GetProperty(isolate, desc, set_name));
-  ASSERT(!isolate->has_pending_exception());
+  Handle<Object> setter = Object::GetProperty(desc, set_name);
+  ASSERT(!setter.is_null());
   if (!setter->IsUndefined()) {
     // TODO(rossberg): nicer would be to cast to some JSCallable here...
     return SetPropertyWithDefinedSetter(
         receiver, Handle<JSReceiver>::cast(setter), value);
   }
 
   if (strict_mode == SLOPPY) return value;
   Handle<Object> args2[] = { name, proxy };
   Handle<Object> error = isolate->factory()->NewTypeError(
       "no_setter_in_callback", HandleVector(args2, ARRAY_SIZE(args2)));
@@ skipping 57 matching lines @@
   bool has_pending_exception;
   Handle<Object> argv[] = { result };
   Handle<Object> desc = Execution::Call(
       isolate, isolate->to_complete_property_descriptor(), result,
       ARRAY_SIZE(argv), argv, &has_pending_exception);
   if (has_pending_exception) return NONE;
 
   // Convert result to PropertyAttributes.
   Handle<String> enum_n = isolate->factory()->InternalizeOneByteString(
       STATIC_ASCII_VECTOR("enumerable_"));
-  Handle<Object> enumerable(v8::internal::GetProperty(isolate, desc, enum_n));
-  if (isolate->has_pending_exception()) return NONE;
+  Handle<Object> enumerable = Object::GetProperty(desc, enum_n);
+  RETURN_IF_EMPTY_HANDLE_VALUE(isolate, enumerable, NONE);
   Handle<String> conf_n = isolate->factory()->InternalizeOneByteString(
       STATIC_ASCII_VECTOR("configurable_"));
-  Handle<Object> configurable(v8::internal::GetProperty(isolate, desc, conf_n));
-  if (isolate->has_pending_exception()) return NONE;
+  Handle<Object> configurable = Object::GetProperty(desc, conf_n);
+  RETURN_IF_EMPTY_HANDLE_VALUE(isolate, configurable, NONE);
   Handle<String> writ_n = isolate->factory()->InternalizeOneByteString(
       STATIC_ASCII_VECTOR("writable_"));
-  Handle<Object> writable(v8::internal::GetProperty(isolate, desc, writ_n));
-  if (isolate->has_pending_exception()) return NONE;
+  Handle<Object> writable = Object::GetProperty(desc, writ_n);
+  RETURN_IF_EMPTY_HANDLE_VALUE(isolate, writable, NONE);
   if (!writable->BooleanValue()) {
     Handle<String> set_n = isolate->factory()->InternalizeOneByteString(
         STATIC_ASCII_VECTOR("set_"));
-    Handle<Object> setter(v8::internal::GetProperty(isolate, desc, set_n));
-    if (isolate->has_pending_exception()) return NONE;
+    Handle<Object> setter = Object::GetProperty(desc, set_n);
+  RETURN_IF_EMPTY_HANDLE_VALUE(isolate, setter, NONE);

[Igor Sheludko, 2014/03/26 18:37:31]

Add two spaces in front of the line.

[Yang, 2014/03/28 09:56:58]

Done.

     writable = isolate->factory()->ToBoolean(!setter->IsUndefined());
   }
 
   if (configurable->IsFalse()) {
     Handle<Object> handler(proxy->handler(), isolate);
     Handle<String> trap = isolate->factory()->InternalizeOneByteString(
         STATIC_ASCII_VECTOR("getPropertyDescriptor"));
     Handle<Object> args[] = { handler, trap, name };
     Handle<Object> error = isolate->factory()->NewTypeError(
         "proxy_prop_not_configurable", HandleVector(args, ARRAY_SIZE(args)));
@@ skipping 42 matching lines @@
 
 
 MUST_USE_RESULT Handle<Object> JSProxy::CallTrap(const char* name,
                                                  Handle<Object> derived,
                                                  int argc,
                                                  Handle<Object> argv[]) {
   Isolate* isolate = GetIsolate();
   Handle<Object> handler(this->handler(), isolate);
 
   Handle<String> trap_name = isolate->factory()->InternalizeUtf8String(name);
-  Handle<Object> trap(v8::internal::GetProperty(isolate, handler, trap_name));
-  if (isolate->has_pending_exception()) return trap;
+  Handle<Object> trap = Object::GetPropertyOrElement(handler, trap_name);
+  RETURN_IF_EMPTY_HANDLE_VALUE(isolate, trap, Handle<Object>());
 
   if (trap->IsUndefined()) {
     if (derived.is_null()) {
       Handle<Object> args[] = { handler, trap_name };
       Handle<Object> error = isolate->factory()->NewTypeError(
         "handler_trap_missing", HandleVector(args, ARRAY_SIZE(args)));
       isolate->Throw(*error);
       return Handle<Object>();
     }
     trap = Handle<Object>(derived);
@@ skipping 245 matching lines @@
       return Handle<Object>();
     } else {
       return value;
     }
   }
 
   Handle<Object> old_value = isolate->factory()->the_hole_value();
   bool is_observed = object->map()->is_observed() &&
                      *name != isolate->heap()->hidden_string();
   if (is_observed && lookup->IsDataProperty()) {
-    old_value = Object::GetProperty(object, name);
+    old_value = Object::GetPropertyOrElement(object, name);
     CHECK_NOT_EMPTY_HANDLE(isolate, old_value);
   }
 
   // This is a real property that is not read-only, or it is a
   // transition or null descriptor and there are no setters in the prototypes.
   Handle<Object> result = value;
   switch (lookup->type()) {
     case NORMAL:
       SetNormalizedProperty(handle(lookup->holder()), lookup, value);
       break;
@@ skipping 25 matching lines @@
 
   RETURN_IF_EMPTY_HANDLE_VALUE(isolate, result, Handle<Object>());
 
   if (is_observed) {
     if (lookup->IsTransition()) {
       EnqueueChangeRecord(object, "add", name, old_value);
     } else {
       LookupResult new_lookup(isolate);
       object->LocalLookup(*name, &new_lookup, true);
       if (new_lookup.IsDataProperty()) {
-        Handle<Object> new_value = Object::GetProperty(object, name);
+        Handle<Object> new_value = Object::GetPropertyOrElement(object, name);
         CHECK_NOT_EMPTY_HANDLE(isolate, new_value);
         if (!new_value->SameValue(*old_value)) {
           EnqueueChangeRecord(object, "update", name, old_value);
         }
       }
     }
   }
 
   return result;
 }
@@ skipping 58 matching lines @@
     return AddProperty(object, name, value, attributes, SLOPPY,
         MAY_BE_STORE_FROM_KEYED, extensibility_check, value_type, mode, flag);
   }
 
   Handle<Object> old_value = isolate->factory()->the_hole_value();
   PropertyAttributes old_attributes = ABSENT;
   bool is_observed = object->map()->is_observed() &&
                      *name != isolate->heap()->hidden_string();
   if (is_observed && lookup.IsProperty()) {
     if (lookup.IsDataProperty()) {
-      old_value = Object::GetProperty(object, name);
+      old_value = Object::GetPropertyOrElement(object, name);
       CHECK_NOT_EMPTY_HANDLE(isolate, old_value);
     }
     old_attributes = lookup.GetAttributes();
   }
 
   // Check of IsReadOnly removed from here in clone.
   switch (lookup.type()) {
     case NORMAL:
       ReplaceSlowProperty(object, name, value, attributes);
       break;
@@ skipping 25 matching lines @@
   if (is_observed) {
     if (lookup.IsTransition()) {
       EnqueueChangeRecord(object, "add", name, old_value);
     } else if (old_value->IsTheHole()) {
       EnqueueChangeRecord(object, "reconfigure", name, old_value);
     } else {
       LookupResult new_lookup(isolate);
       object->LocalLookup(*name, &new_lookup, true);
       bool value_changed = false;
       if (new_lookup.IsDataProperty()) {
-        Handle<Object> new_value = Object::GetProperty(object, name);
+        Handle<Object> new_value = Object::GetPropertyOrElement(object, name);
         CHECK_NOT_EMPTY_HANDLE(isolate, new_value);
         value_changed = !old_value->SameValue(*new_value);
       }
       if (new_lookup.GetAttributes() != old_attributes) {
         if (!value_changed) old_value = isolate->factory()->the_hole_value();
         EnqueueChangeRecord(object, "reconfigure", name, old_value);
       } else if (value_changed) {
         EnqueueChangeRecord(object, "update", name, old_value);
       }
     }
@@ skipping 968 matching lines @@
       isolate->Throw(*error);
       return Handle<Object>();
     }
     return isolate->factory()->false_value();
   }
 
   Handle<Object> old_value = isolate->factory()->the_hole_value();
   bool is_observed = object->map()->is_observed() &&
                      *name != isolate->heap()->hidden_string();
   if (is_observed && lookup.IsDataProperty()) {
-    old_value = Object::GetProperty(object, name);
+    old_value = Object::GetPropertyOrElement(object, name);
     CHECK_NOT_EMPTY_HANDLE(isolate, old_value);
   }
   Handle<Object> result;
 
   // Check for interceptor.
   if (lookup.IsInterceptor()) {
     // Skip interceptor if forcing a deletion.
     if (mode == FORCE_DELETION) {
       result = DeletePropertyPostInterceptor(object, name, mode);
     } else {
@@ skipping 1085 matching lines @@
     if (is_element) {
       preexists = HasLocalElement(object, index);
       if (preexists && object->GetLocalElementAccessorPair(index) == NULL) {
         old_value = Object::GetElementNoExceptionThrown(isolate, object, index);
       }
     } else {
       LookupResult lookup(isolate);
       object->LocalLookup(*name, &lookup, true);
       preexists = lookup.IsProperty();
       if (preexists && lookup.IsDataProperty()) {
-        old_value = Object::GetProperty(object, name);
+        old_value = Object::GetPropertyOrElement(object, name);
         CHECK_NOT_EMPTY_HANDLE(isolate, old_value);
       }
     }
   }
 
   if (is_element) {
     DefineElementAccessor(
         object, index, getter, setter, attributes, access_control);
   } else {
     DefinePropertyAccessor(
@@ skipping 10072 matching lines @@
 #define ERROR_MESSAGES_TEXTS(C, T) T,
   static const char* error_messages_[] = {
       ERROR_MESSAGES_LIST(ERROR_MESSAGES_TEXTS)
   };
 #undef ERROR_MESSAGES_TEXTS
   return error_messages_[reason];
 }
 
 
 } }  // namespace v8::internal