Introduce LookupIterator::PropertyOrElement which converts name to index if possible.

BUG=v8:4137
LOG=n

Review URL: https://codereview.chromium.org/1178893002

Cr-Commit-Position: refs/heads/master@{#29004}

src/api-natives.cc

@@ -67,7 +67,7 @@ MaybeHandle<Object> DefineAccessorProperty(
 
 MaybeHandle<Object> DefineDataProperty(Isolate* isolate,
                                        Handle<JSObject> object,
-                                       Handle<Name> key,
+                                       Handle<Name> name,
                                        Handle<Object> prop_data,
                                        Smi* unchecked_attributes) {
   DCHECK((unchecked_attributes->value() &
@@ -78,20 +78,18 @@ MaybeHandle<Object> DefineDataProperty(Isolate* isolate,
 
   Handle<Object> value;
   ASSIGN_RETURN_ON_EXCEPTION(isolate, value,
-                             Instantiate(isolate, prop_data, key), Object);
+                             Instantiate(isolate, prop_data, name), Object);
 
-  uint32_t index = 0;
-  LookupIterator::Configuration c = LookupIterator::OWN_SKIP_INTERCEPTOR;
-  LookupIterator it = key->AsArrayIndex(&index)
-                          ? LookupIterator(isolate, object, index, c)
-                          : LookupIterator(object, key, c);
+  LookupIterator it = LookupIterator::PropertyOrElement(
+      isolate, object, name, LookupIterator::OWN_SKIP_INTERCEPTOR);
 
 #ifdef DEBUG
   Maybe<PropertyAttributes> maybe = JSReceiver::GetPropertyAttributes(&it);
   DCHECK(maybe.IsJust());
   if (it.IsFound()) {
     THROW_NEW_ERROR(
-        isolate, NewTypeError(MessageTemplate::kDuplicateTemplateProperty, key),
+        isolate,
+        NewTypeError(MessageTemplate::kDuplicateTemplateProperty, name),
         Object);
   }
 #endif

src/json-stringifier.h

@@ -580,12 +580,9 @@ BasicJsonStringifier::Result BasicJsonStringifier::SerializeJSObject(
       } else {
         DCHECK(key->IsNumber());
         key_handle = factory()->NumberToString(Handle<Object>(key, isolate_));
-        uint32_t index;
         if (key->IsSmi()) {
           maybe_property = Object::GetElement(
               isolate_, object, Smi::cast(key)->value());
-        } else if (key_handle->AsArrayIndex(&index)) {
-          maybe_property = Object::GetElement(isolate_, object, index);
         } else {
           maybe_property = Object::GetPropertyOrElement(object, key_handle);
         }

src/lookup.h

@@ -26,7 +26,8 @@ class LookupIterator final BASE_EMBEDDED {
     HIDDEN_SKIP_INTERCEPTOR = kHidden,
     HIDDEN = kHidden | kInterceptor,
     PROTOTYPE_CHAIN_SKIP_INTERCEPTOR = kHidden | kPrototypeChain,
-    PROTOTYPE_CHAIN = kHidden | kPrototypeChain | kInterceptor
+    PROTOTYPE_CHAIN = kHidden | kPrototypeChain | kInterceptor,
+    DEFAULT = PROTOTYPE_CHAIN
   };
 
   enum State {
@@ -44,7 +45,7 @@ class LookupIterator final BASE_EMBEDDED {
   };
 
   LookupIterator(Handle<Object> receiver, Handle<Name> name,
-                 Configuration configuration = PROTOTYPE_CHAIN)
+                 Configuration configuration = DEFAULT)
       : configuration_(ComputeConfiguration(configuration, name)),
         state_(NOT_FOUND),
         exotic_index_state_(ExoticIndexState::kUninitialized),
@@ -68,7 +69,7 @@ class LookupIterator final BASE_EMBEDDED {
 
   LookupIterator(Handle<Object> receiver, Handle<Name> name,
                  Handle<JSReceiver> holder,
-                 Configuration configuration = PROTOTYPE_CHAIN)
+                 Configuration configuration = DEFAULT)
       : configuration_(ComputeConfiguration(configuration, name)),
         state_(NOT_FOUND),
         exotic_index_state_(ExoticIndexState::kUninitialized),
@@ -91,7 +92,7 @@ class LookupIterator final BASE_EMBEDDED {
   }
 
   LookupIterator(Isolate* isolate, Handle<Object> receiver, uint32_t index,
-                 Configuration configuration = PROTOTYPE_CHAIN)
+                 Configuration configuration = DEFAULT)
       : configuration_(configuration),
         state_(NOT_FOUND),
         exotic_index_state_(ExoticIndexState::kUninitialized),
@@ -112,7 +113,7 @@ class LookupIterator final BASE_EMBEDDED {
 
   LookupIterator(Isolate* isolate, Handle<Object> receiver, uint32_t index,
                  Handle<JSReceiver> holder,
-                 Configuration configuration = PROTOTYPE_CHAIN)
+                 Configuration configuration = DEFAULT)
       : configuration_(configuration),
         state_(NOT_FOUND),
         exotic_index_state_(ExoticIndexState::kUninitialized),
@@ -131,6 +132,29 @@ class LookupIterator final BASE_EMBEDDED {
     Next();
   }
 
+  static LookupIterator PropertyOrElement(
+      Isolate* isolate, Handle<Object> receiver, Handle<Name> name,
+      Configuration configuration = DEFAULT) {
+    uint32_t index;
+    LookupIterator it =
+        name->AsArrayIndex(&index)
+            ? LookupIterator(isolate, receiver, index, configuration)
+            : LookupIterator(receiver, name, configuration);
+    it.name_ = name;
+    return it;
+  }
+
+  static LookupIterator PropertyOrElement(
+      Isolate* isolate, Handle<Object> receiver, Handle<Name> name,
+      Handle<JSReceiver> holder, Configuration configuration = DEFAULT) {
+    uint32_t index;
+    LookupIterator it =
+        name->AsArrayIndex(&index)
+            ? LookupIterator(isolate, receiver, index, holder, configuration)
+            : LookupIterator(receiver, name, holder, configuration);
+    it.name_ = name;
+    return it;
+  }
   Isolate* isolate() const { return isolate_; }
   State state() const { return state_; }
 

src/objects-inl.h

@@ -6583,10 +6583,8 @@ String* String::GetForwardedInternalizedString() {
 
 MaybeHandle<Object> Object::GetPropertyOrElement(Handle<Object> object,
                                                  Handle<Name> name) {
-  uint32_t index;
-  LookupIterator it = name->AsArrayIndex(&index)
-                          ? LookupIterator(name->GetIsolate(), object, index)
-                          : LookupIterator(object, name);
+  LookupIterator it =
+      LookupIterator::PropertyOrElement(name->GetIsolate(), object, name);
   return GetProperty(&it);
 }
 
@@ -6619,21 +6617,16 @@ Maybe<bool> JSReceiver::HasOwnProperty(Handle<JSReceiver> object,
 
 Maybe<PropertyAttributes> JSReceiver::GetPropertyAttributes(
     Handle<JSReceiver> object, Handle<Name> name) {
-  uint32_t index = 0;
-  LookupIterator it = name->AsArrayIndex(&index)
-                          ? LookupIterator(name->GetIsolate(), object, index)
-                          : LookupIterator(object, name);
+  LookupIterator it =
+      LookupIterator::PropertyOrElement(name->GetIsolate(), object, name);
   return GetPropertyAttributes(&it);
 }
 
 
 Maybe<PropertyAttributes> JSReceiver::GetOwnPropertyAttributes(
     Handle<JSReceiver> object, Handle<Name> name) {
-  uint32_t index = 0;
-  LookupIterator::Configuration c = LookupIterator::HIDDEN;
-  LookupIterator it = name->AsArrayIndex(&index)
-                          ? LookupIterator(name->GetIsolate(), object, index, c)
-                          : LookupIterator(object, name, c);
+  LookupIterator it = LookupIterator::PropertyOrElement(
+      name->GetIsolate(), object, name, LookupIterator::HIDDEN);
   return GetPropertyAttributes(&it);
 }
 

src/objects.cc

@@ -4290,11 +4290,8 @@ MaybeHandle<Object> JSObject::DefinePropertyOrElementIgnoreAttributes(
     Handle<JSObject> object, Handle<Name> name, Handle<Object> value,
     PropertyAttributes attributes, ExecutableAccessorInfoHandling handling) {
   Isolate* isolate = object->GetIsolate();
-  uint32_t index;
-  LookupIterator it =
-      name->AsArrayIndex(&index)
-          ? LookupIterator(isolate, object, index, LookupIterator::OWN)
-          : LookupIterator(object, name, LookupIterator::OWN);
+  LookupIterator it = LookupIterator::PropertyOrElement(isolate, object, name,
+                                                        LookupIterator::OWN);
   return DefineOwnPropertyIgnoreAttributes(&it, value, attributes, handling);
 }
 
@@ -5254,11 +5251,8 @@ MaybeHandle<Object> JSReceiver::DeleteElement(Handle<JSReceiver> object,
 MaybeHandle<Object> JSReceiver::DeleteProperty(Handle<JSReceiver> object,
                                                Handle<Name> name,
                                                LanguageMode language_mode) {
-  uint32_t index;
-  LookupIterator::Configuration c = LookupIterator::HIDDEN;
-  LookupIterator it = name->AsArrayIndex(&index)
-                          ? LookupIterator(name->GetIsolate(), object, index, c)
-                          : LookupIterator(object, name, c);
+  LookupIterator it = LookupIterator::PropertyOrElement(
+      name->GetIsolate(), object, name, LookupIterator::HIDDEN);
   return JSObject::DeleteProperty(&it, language_mode);
 }
 
@@ -6409,11 +6403,8 @@ MaybeHandle<Object> JSObject::DefineAccessor(Handle<JSObject> object,
   // Try to flatten before operating on the string.
   if (name->IsString()) name = String::Flatten(Handle<String>::cast(name));
 
-  uint32_t index = 0;
-  LookupIterator::Configuration c = LookupIterator::HIDDEN_SKIP_INTERCEPTOR;
-  LookupIterator it = name->AsArrayIndex(&index)
-                          ? LookupIterator(isolate, object, index, c)
-                          : LookupIterator(object, name, c);
+  LookupIterator it = LookupIterator::PropertyOrElement(
+      isolate, object, name, LookupIterator::HIDDEN_SKIP_INTERCEPTOR);
 
   if (it.state() == LookupIterator::ACCESS_CHECK) {
     if (!it.HasAccess()) {
@@ -6438,7 +6429,7 @@ MaybeHandle<Object> JSObject::DefineAccessor(Handle<JSObject> object,
   }
 
   if (it.IsElement()) {
-    DefineElementAccessor(it.GetStoreTarget(), index, getter, setter,
+    DefineElementAccessor(it.GetStoreTarget(), it.index(), getter, setter,
                           attributes);
   } else {
     DCHECK(getter->IsSpecFunction() || getter->IsUndefined() ||
@@ -6477,11 +6468,8 @@ MaybeHandle<Object> JSObject::SetAccessor(Handle<JSObject> object,
   Handle<Name> name(Name::cast(info->name()));
   if (name->IsString()) name = String::Flatten(Handle<String>::cast(name));
 
-  uint32_t index = 0;
-  LookupIterator::Configuration c = LookupIterator::HIDDEN_SKIP_INTERCEPTOR;
-  LookupIterator it = name->AsArrayIndex(&index)
-                          ? LookupIterator(isolate, object, index, c)
-                          : LookupIterator(object, name, c);
+  LookupIterator it = LookupIterator::PropertyOrElement(
+      isolate, object, name, LookupIterator::HIDDEN_SKIP_INTERCEPTOR);
 
   // Duplicate ACCESS_CHECK outside of GetPropertyAttributes for the case that
   // the FailedAccessCheckCallbackFunction doesn't throw an exception.
@@ -6512,7 +6500,7 @@ MaybeHandle<Object> JSObject::SetAccessor(Handle<JSObject> object,
   }
 
   if (it.IsElement()) {
-    SetElementCallback(object, index, info, info->property_attributes());
+    SetElementCallback(object, it.index(), info, info->property_attributes());
   } else {
     SetPropertyCallback(object, name, info, info->property_attributes());
   }
@@ -6530,13 +6518,8 @@ MaybeHandle<Object> JSObject::GetAccessor(Handle<JSObject> object,
   // interceptor calls.
   AssertNoContextChange ncc(isolate);
 
-  // Make the lookup and include prototypes.
-  uint32_t index = 0;
-  LookupIterator::Configuration c =
-      LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR;
-  LookupIterator it = name->AsArrayIndex(&index)
-                          ? LookupIterator(isolate, object, index, c)
-                          : LookupIterator(object, name, c);
+  LookupIterator it = LookupIterator::PropertyOrElement(
+      isolate, object, name, LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
 
   for (; it.IsFound(); it.Next()) {
     switch (it.state()) {

src/runtime/runtime-classes.cc

@@ -307,6 +307,7 @@ RUNTIME_FUNCTION(Runtime_LoadKeyedFromSuper) {
   Handle<Name> name;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, name,
                                      Runtime::ToName(isolate, key));
+  // TODO(verwaest): Unify using LookupIterator.
   if (name->AsArrayIndex(&index)) {
     return LoadElementFromSuper(isolate, receiver, home_object, index);
   }
@@ -397,6 +398,7 @@ static Object* StoreKeyedToSuper(Isolate* isolate, Handle<JSObject> home_object,
   Handle<Name> name;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, name,
                                      Runtime::ToName(isolate, key));
+  // TODO(verwaest): Unify using LookupIterator.
   if (name->AsArrayIndex(&index)) {
     return StoreElementToSuper(isolate, home_object, receiver, index, value,
                                language_mode);

src/runtime/runtime-debug.cc

@@ -306,6 +306,8 @@ RUNTIME_FUNCTION(Runtime_DebugGetPropertyDetails) {
   // Check if the name is trivially convertible to an index and get the element
   // if so.
   uint32_t index;
+  // TODO(verwaest): Make sure DebugGetProperty can handle arrays, and remove
+  // this special case.
   if (name->AsArrayIndex(&index)) {
     Handle<FixedArray> details = isolate->factory()->NewFixedArray(2);
     Handle<Object> element_or_char;

src/runtime/runtime-object.cc

@@ -72,6 +72,8 @@ MaybeHandle<Object> Runtime::GetObjectProperty(Isolate* isolate,
 
   // Check if the name is trivially convertible to an index and get
   // the element if so.
+  // TODO(verwaest): Make sure GetProperty(LookupIterator*) can handle this, and
+  // remove the special casing here.
   if (name->AsArrayIndex(&index)) {
     return GetElementOrCharAt(isolate, object, index);
   } else {
@@ -102,23 +104,13 @@ MaybeHandle<Object> Runtime::SetObjectProperty(Isolate* isolate,
   }
 
   Handle<Name> name;
-  if (key->IsName()) {
-    name = Handle<Name>::cast(key);
-  } else {
-    // Call-back into JavaScript to convert the key to a string.
-    Handle<Object> converted;
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, converted,
-                               Execution::ToString(isolate, key), Object);
-    name = Handle<String>::cast(converted);
-  }
+  ASSIGN_RETURN_ON_EXCEPTION(isolate, name, ToName(isolate, key), Object);
 
-  if (name->AsArrayIndex(&index)) {
-    // TODO(verwaest): Support other objects as well.
-    if (!object->IsJSReceiver()) return value;
-    return JSReceiver::SetElement(Handle<JSReceiver>::cast(object), index,
-                                  value, language_mode);
-  }
-  return Object::SetProperty(object, name, value, language_mode);
+  LookupIterator it = LookupIterator::PropertyOrElement(isolate, object, name);
+  // TODO(verwaest): Support other objects as well.
+  if (it.IsElement() && !object->IsJSReceiver()) return value;
+  return Object::SetProperty(&it, value, language_mode,
+                             Object::MAY_BE_STORE_FROM_KEYED);
 }
 
 
@@ -239,12 +231,9 @@ MUST_USE_RESULT static MaybeHandle<Object> GetOwnProperty(Isolate* isolate,
   Factory* factory = isolate->factory();
 
   PropertyAttributes attrs;
-  uint32_t index;
   // Get attributes.
-  LookupIterator it =
-      name->AsArrayIndex(&index)
-          ? LookupIterator(isolate, obj, index, LookupIterator::HIDDEN)
-          : LookupIterator(obj, name, LookupIterator::HIDDEN);
+  LookupIterator it = LookupIterator::PropertyOrElement(isolate, obj, name,
+                                                        LookupIterator::HIDDEN);
   Maybe<PropertyAttributes> maybe = JSObject::GetPropertyAttributes(&it);
 
   if (!maybe.IsJust()) return MaybeHandle<Object>();
@@ -1274,23 +1263,17 @@ RUNTIME_FUNCTION(Runtime_DefineDataPropertyUnchecked) {
   CONVERT_ARG_HANDLE_CHECKED(Object, value, 2);
   CONVERT_PROPERTY_ATTRIBUTES_CHECKED(attrs, 3);
 
-  uint32_t index = 0;
-  LookupIterator::Configuration c = LookupIterator::OWN_SKIP_INTERCEPTOR;
-  LookupIterator it = name->AsArrayIndex(&index)
-                          ? LookupIterator(isolate, object, index, c)
-                          : LookupIterator(object, name, c);
+  LookupIterator it = LookupIterator::PropertyOrElement(isolate, object, name,
+                                                        LookupIterator::OWN);
   if (it.state() == LookupIterator::ACCESS_CHECK && !it.HasAccess()) {
     return isolate->heap()->undefined_value();
   }
 
   Handle<Object> result;
-  MaybeHandle<Object> maybe_result =
-      it.IsElement()
-          ? JSObject::SetOwnElementIgnoreAttributes(object, index, value, attrs,
-                                                    JSObject::DONT_FORCE_FIELD)
-          : JSObject::SetOwnPropertyIgnoreAttributes(
-                object, name, value, attrs, JSObject::DONT_FORCE_FIELD);
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, result, maybe_result);
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result, JSObject::DefineOwnPropertyIgnoreAttributes(
+                           &it, value, attrs, JSObject::DONT_FORCE_FIELD));
+
   return *result;
 }