Reimplement SetProperty using the LookupIterator

BUG=
R=ishell@chromium.org

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22482 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/debug.cc

@@ -1547,7 +1547,7 @@ void Debug::PrepareStep(StepAction step_action,
     if (is_load_or_store) {
       // Remember source position and frame to handle step in getter/setter. If
       // there is a custom getter/setter it will be handled in
-      // Object::Get/SetPropertyWithCallback, otherwise the step action will be
+      // Object::Get/SetPropertyWithAccessor, otherwise the step action will be
       // propagated on the next Debug::Break.
       thread_local_.last_statement_position_ =
           debug_info->code()->SourceStatementPosition(frame->pc());

src/ic.cc

@@ -1281,6 +1281,8 @@ MaybeHandle<Object> StoreIC::Store(Handle<Object> object,
                                    Handle<String> name,
                                    Handle<Object> value,
                                    JSReceiver::StoreFromKeyed store_mode) {
+  // TODO(verwaest): Let SetProperty do the migration, since storing a property
+  // might deprecate the current map again, if value does not fit.
   if (MigrateDeprecated(object) || object->IsJSProxy()) {
     Handle<JSReceiver> receiver = Handle<JSReceiver>::cast(object);
     Handle<Object> result;
@@ -1757,6 +1759,8 @@ KeyedAccessStoreMode KeyedStoreIC::GetStoreMode(Handle<JSObject> receiver,
 MaybeHandle<Object> KeyedStoreIC::Store(Handle<Object> object,
                                         Handle<Object> key,
                                         Handle<Object> value) {
+  // TODO(verwaest): Let SetProperty do the migration, since storing a property
+  // might deprecate the current map again, if value does not fit.
   if (MigrateDeprecated(object)) {
     Handle<Object> result;
     ASSIGN_RETURN_ON_EXCEPTION(

src/lookup.cc

@@ -144,6 +144,78 @@ bool LookupIterator::HasProperty() {
 }
 
 
+void LookupIterator::PrepareForDataProperty(Handle<Object> value) {
+  ASSERT(has_property_);
+  ASSERT(HolderIsReceiver());
+  if (property_encoding_ == DICTIONARY) return;
+  holder_map_ = Map::PrepareForDataProperty(holder_map_, number_, value);
+  JSObject::MigrateToMap(GetHolder(), holder_map_);
+  // Reload property information.
+  if (holder_map_->is_dictionary_map()) {
+    property_encoding_ = DICTIONARY;
+  } else {
+    property_encoding_ = DESCRIPTOR;
+  }
+  CHECK(HasProperty());
+}
+
+
+void LookupIterator::TransitionToDataProperty(
+    Handle<Object> value, PropertyAttributes attributes,
+    Object::StoreFromKeyed store_mode) {
+  ASSERT(!has_property_ || !HolderIsReceiver());
+
+  // Can only be called when the receiver is a JSObject. JSProxy has to be
+  // handled via a trap. Adding properties to primitive values is not
+  // observable.
+  Handle<JSObject> receiver = Handle<JSObject>::cast(GetReceiver());
+
+  // Properties have to be added to context extension objects through
+  // SetOwnPropertyIgnoreAttributes.
+  ASSERT(!receiver->IsJSContextExtensionObject());
+
+  if (receiver->IsJSGlobalProxy()) {
+    PrototypeIterator iter(isolate(), receiver);
+    receiver =
+        Handle<JSGlobalObject>::cast(PrototypeIterator::GetCurrent(iter));
+  }
+
+  maybe_holder_ = receiver;
+  holder_map_ = Map::TransitionToDataProperty(handle(receiver->map()), name_,
+                                              value, attributes, store_mode);
+  JSObject::MigrateToMap(receiver, holder_map_);
+
+  // Reload the information.
+  state_ = NOT_FOUND;
+  configuration_ = CHECK_OWN_REAL;
+  state_ = LookupInHolder();
+  ASSERT(IsFound());
+  HasProperty();
+}
+
+
+bool LookupIterator::HolderIsReceiver() const {
+  ASSERT(has_property_ || state_ == INTERCEPTOR || state_ == JSPROXY);
+  DisallowHeapAllocation no_gc;
+  Handle<Object> receiver = GetReceiver();
+  if (!receiver->IsJSReceiver()) return false;
+  Object* current = *receiver;
+  JSReceiver* holder = *maybe_holder_.ToHandleChecked();
+  // JSProxy do not occur as hidden prototypes.
+  if (current->IsJSProxy()) {
+    return JSReceiver::cast(current) == holder;
+  }
+  PrototypeIterator iter(isolate(), current,
+                         PrototypeIterator::START_AT_RECEIVER);
+  do {
+    if (JSReceiver::cast(iter.GetCurrent()) == holder) return true;
+    ASSERT(!current->IsJSProxy());
+    iter.Advance();
+  } while (!iter.IsAtEnd(PrototypeIterator::END_AT_NON_HIDDEN));
+  return false;
+}
+
+
 Handle<Object> LookupIterator::FetchValue() const {
   Object* result = NULL;
   switch (property_encoding_) {
@@ -181,4 +253,29 @@ Handle<Object> LookupIterator::GetDataValue() const {
 }
 
 
+void LookupIterator::WriteDataValue(Handle<Object> value) {
+  ASSERT(is_guaranteed_to_have_holder());
+  ASSERT(has_property_);
+  if (property_encoding_ == DICTIONARY) {
+    Handle<JSObject> holder = GetHolder();
+    NameDictionary* property_dictionary = holder->property_dictionary();
+    if (holder->IsGlobalObject()) {
+      Handle<PropertyCell> cell(
+          PropertyCell::cast(property_dictionary->ValueAt(number_)));
+      PropertyCell::SetValueInferType(cell, value);
+    } else {
+      property_dictionary->ValueAtPut(number_, *value);
+    }
+  } else if (property_details_.type() == v8::internal::FIELD) {
+    GetHolder()->WriteToField(number_, *value);
+  } else {
+    ASSERT_EQ(v8::internal::CONSTANT, property_details_.type());
+  }
+}
+
+
+void LookupIterator::InternalizeName() {
+  if (name_->IsUniqueName()) return;
+  name_ = factory()->InternalizeString(Handle<String>::cast(name_));
+}
 } }  // namespace v8::internal

src/lookup.h

@@ -92,11 +92,13 @@ class LookupIterator V8_FINAL BASE_EMBEDDED {
   Handle<Object> GetReceiver() const {
     return Handle<Object>::cast(maybe_receiver_.ToHandleChecked());
   }
+  Handle<Map> holder_map() const { return holder_map_; }
   Handle<JSObject> GetHolder() const {
     ASSERT(IsFound() && state_ != JSPROXY);
     return Handle<JSObject>::cast(maybe_holder_.ToHandleChecked());
   }
   Handle<JSReceiver> GetRoot() const;
+  bool HolderIsReceiver() const;
 
   /* Dynamically reduce the trapped types. */
   void skip_interceptor() {
@@ -116,6 +118,10 @@ class LookupIterator V8_FINAL BASE_EMBEDDED {
   // below can be used. It ensures that we are able to provide a definite
   // answer, and loads extra information about the property.
   bool HasProperty();
+  void PrepareForDataProperty(Handle<Object> value);
+  void TransitionToDataProperty(Handle<Object> value,
+                                PropertyAttributes attributes,
+                                Object::StoreFromKeyed store_mode);
   PropertyKind property_kind() const {
     ASSERT(has_property_);
     return property_kind_;
@@ -124,11 +130,18 @@ class LookupIterator V8_FINAL BASE_EMBEDDED {
     ASSERT(has_property_);
     return property_details_;
   }
+  int descriptor_number() const {
+    ASSERT(has_property_);
+    ASSERT_EQ(DESCRIPTOR, property_encoding_);
+    return number_;
+  }
   Handle<Object> GetAccessors() const;
   Handle<Object> GetDataValue() const;
+  void WriteDataValue(Handle<Object> value);
 
-  /* JSPROXY */
+  void InternalizeName();
 
+  /* JSPROXY */
   Handle<JSProxy> GetJSProxy() const {
     return Handle<JSProxy>::cast(maybe_holder_.ToHandleChecked());
   }

src/objects-inl.h

@@ -2079,23 +2079,12 @@ bool JSObject::HasFastProperties() {
 }
 
 
-bool JSObject::TooManyFastProperties(StoreFromKeyed store_mode) {
-  // Allow extra fast properties if the object has more than
-  // kFastPropertiesSoftLimit in-object properties. When this is the case, it is
-  // very unlikely that the object is being used as a dictionary and there is a
-  // good chance that allowing more map transitions will be worth it.
-  Map* map = this->map();
-  if (map->unused_property_fields() != 0) return false;
-
-  int inobject = map->inobject_properties();
-
-  int limit;
-  if (store_mode == CERTAINLY_NOT_STORE_FROM_KEYED) {
-    limit = Max(inobject, kMaxFastProperties);
-  } else {
-    limit = Max(inobject, kFastPropertiesSoftLimit);
-  }
-  return properties()->length() > limit;
+bool Map::TooManyFastProperties(StoreFromKeyed store_mode) {
+  if (unused_property_fields() != 0) return false;
+  int minimum = store_mode == CERTAINLY_NOT_STORE_FROM_KEYED ? 128 : 12;
+  int limit = Max(minimum, inobject_properties());
+  int external = NumberOfFields() - inobject_properties();
+  return external > limit;
 }
 
 

src/objects.h

@@ -1375,6 +1375,13 @@ class Object {
   HEAP_OBJECT_TYPE_LIST(IS_TYPE_FUNCTION_DECL)
 #undef IS_TYPE_FUNCTION_DECL
 
+  // A non-keyed store is of the form a.x = foo or a["x"] = foo whereas
+  // a keyed store is of the form a[expression] = foo.
+  enum StoreFromKeyed {
+    MAY_BE_STORE_FROM_KEYED,
+    CERTAINLY_NOT_STORE_FROM_KEYED
+  };
+
   INLINE(bool IsFixedArrayBase() const);
   INLINE(bool IsExternal() const);
   INLINE(bool IsAccessorInfo() const);
@@ -1476,6 +1483,16 @@ class Object {
   void Lookup(Handle<Name> name, LookupResult* result);
 
   MUST_USE_RESULT static MaybeHandle<Object> GetProperty(LookupIterator* it);
+  MUST_USE_RESULT static MaybeHandle<Object> SetProperty(
+      LookupIterator* it, Handle<Object> value, StrictMode strict_mode,
+      StoreFromKeyed store_mode);
+  MUST_USE_RESULT static MaybeHandle<Object> WriteToReadOnlyProperty(
+      LookupIterator* it, Handle<Object> value, StrictMode strict_mode);
+  MUST_USE_RESULT static MaybeHandle<Object> SetDataProperty(
+      LookupIterator* it, Handle<Object> value);
+  MUST_USE_RESULT static MaybeHandle<Object> AddDataProperty(
+      LookupIterator* it, Handle<Object> value, PropertyAttributes attributes,
+      StrictMode strict_mode, StoreFromKeyed store_mode);
   MUST_USE_RESULT static inline MaybeHandle<Object> GetPropertyOrElement(
       Handle<Object> object,
       Handle<Name> key);
@@ -1492,12 +1509,9 @@ class Object {
       Handle<Name> name,
       Handle<JSObject> holder,
       Handle<Object> structure);
-  MUST_USE_RESULT static MaybeHandle<Object> SetPropertyWithCallback(
-      Handle<Object> receiver,
-      Handle<Name> name,
-      Handle<Object> value,
-      Handle<JSObject> holder,
-      Handle<Object> structure,
+  MUST_USE_RESULT static MaybeHandle<Object> SetPropertyWithAccessor(
+      Handle<Object> receiver, Handle<Name> name, Handle<Object> value,
+      Handle<JSObject> holder, Handle<Object> structure,
       StrictMode strict_mode);
 
   MUST_USE_RESULT static MaybeHandle<Object> GetPropertyWithDefinedGetter(
@@ -1918,13 +1932,6 @@ class JSReceiver: public HeapObject {
     FORCE_DELETION
   };
 
-  // A non-keyed store is of the form a.x = foo or a["x"] = foo whereas
-  // a keyed store is of the form a[expression] = foo.
-  enum StoreFromKeyed {
-    MAY_BE_STORE_FROM_KEYED,
-    CERTAINLY_NOT_STORE_FROM_KEYED
-  };
-
   // Internal properties (e.g. the hidden properties dictionary) might
   // be added even though the receiver is non-extensible.
   enum ExtensibilityCheck {
@@ -2136,18 +2143,7 @@ class JSObject: public JSReceiver {
                                                    uint32_t limit);
 
   MUST_USE_RESULT static MaybeHandle<Object> SetPropertyWithInterceptor(
-      Handle<JSObject> object,
-      Handle<Name> name,
-      Handle<Object> value,
-      StrictMode strict_mode);
-
-  MUST_USE_RESULT static MaybeHandle<Object> SetPropertyForResult(
-      Handle<JSObject> object,
-      LookupResult* result,
-      Handle<Name> name,
-      Handle<Object> value,
-      StrictMode strict_mode,
-      StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED);
+      LookupIterator* it, Handle<Object> value);
 
   // SetLocalPropertyIgnoreAttributes converts callbacks to fields. We need to
   // grant an exemption to ExecutableAccessor callbacks in some cases.
@@ -2573,12 +2569,6 @@ class JSObject: public JSReceiver {
 
   Object* SlowReverseLookup(Object* value);
 
-  // Maximal number of fast properties for the JSObject. Used to
-  // restrict the number of map transitions to avoid an explosion in
-  // the number of maps for objects used as dictionaries.
-  inline bool TooManyFastProperties(
-      StoreFromKeyed store_mode = MAY_BE_STORE_FROM_KEYED);
-
   // Maximal number of elements (numbered 0 .. kMaxElementCount - 1).
   // Also maximal value of JSArray's length property.
   static const uint32_t kMaxElementCount = 0xffffffffu;
@@ -2606,8 +2596,6 @@ class JSObject: public JSReceiver {
   // "global.Object" and not to arbitrary other JSObject maps.
   static const int kInitialGlobalObjectUnusedPropertiesCount = 4;
 
-  static const int kFastPropertiesSoftLimit = 12;
-  static const int kMaxFastProperties = 128;
   static const int kMaxInstanceSize = 255 * kPointerSize;
   // When extending the backing storage for property values, we increase
   // its size by more than the 1 entry necessary, so sequentially adding fields
@@ -2734,21 +2722,6 @@ class JSObject: public JSReceiver {
       StrictMode strict_mode,
       bool check_prototype = true);
 
-  // Searches the prototype chain for property 'name'. If it is found and
-  // has a setter, invoke it and set '*done' to true. If it is found and is
-  // read-only, reject and set '*done' to true. Otherwise, set '*done' to
-  // false. Can throw and return an empty handle with '*done==true'.
-  MUST_USE_RESULT static MaybeHandle<Object> SetPropertyViaPrototypes(
-      Handle<JSObject> object,
-      Handle<Name> name,
-      Handle<Object> value,
-      StrictMode strict_mode,
-      bool* done);
-  MUST_USE_RESULT static MaybeHandle<Object> SetPropertyPostInterceptor(
-      Handle<JSObject> object,
-      Handle<Name> name,
-      Handle<Object> value,
-      StrictMode strict_mode);
   MUST_USE_RESULT static MaybeHandle<Object> SetPropertyUsingTransition(
       Handle<JSObject> object,
       LookupResult* lookup,
@@ -2756,12 +2729,7 @@ class JSObject: public JSReceiver {
       Handle<Object> value,
       PropertyAttributes attributes);
   MUST_USE_RESULT static MaybeHandle<Object> SetPropertyWithFailedAccessCheck(
-      Handle<JSObject> object,
-      LookupResult* result,
-      Handle<Name> name,
-      Handle<Object> value,
-      bool check_prototype,
-      StrictMode strict_mode);
+      LookupIterator* it, Handle<Object> value, StrictMode strict_mode);
 
   // Add a property to an object.
   MUST_USE_RESULT static MaybeHandle<Object> AddPropertyInternal(
@@ -3452,6 +3420,8 @@ class DescriptorArray: public FixedArray {
                     FixedArray* new_cache,
                     Object* new_index_cache);
 
+  bool CanHoldValue(int descriptor, Object* value);
+
   // Accessors for fetching instance descriptor at descriptor number.
   inline Name* GetKey(int descriptor_number);
   inline Object** GetKeySlot(int descriptor_number);
@@ -6334,6 +6304,10 @@ class Map: public HeapObject {
       StoreMode store_mode,
       const char* reason);
 
+  static Handle<Map> PrepareForDataProperty(Handle<Map> old_map,
+                                            int descriptor_number,
+                                            Handle<Object> value);
+
   static Handle<Map> Normalize(Handle<Map> map, PropertyNormalizationMode mode);
 
   // Returns the constructor name (the name (possibly, inferred name) of the
@@ -6526,6 +6500,15 @@ class Map: public HeapObject {
   static Handle<Map> CopyForObserved(Handle<Map> map);
 
   static Handle<Map> CopyForFreeze(Handle<Map> map);
+  // Maximal number of fast properties. Used to restrict the number of map
+  // transitions to avoid an explosion in the number of maps for objects used as
+  // dictionaries.
+  inline bool TooManyFastProperties(StoreFromKeyed store_mode);
+  static Handle<Map> TransitionToDataProperty(Handle<Map> map,
+                                              Handle<Name> name,
+                                              Handle<Object> value,
+                                              PropertyAttributes attributes,
+                                              StoreFromKeyed store_mode);
 
   inline void AppendDescriptor(Descriptor* desc);
 
@@ -6834,6 +6817,9 @@ class Map: public HeapObject {
                                             Handle<Object> prototype,
                                             Handle<Map> target_map);
 
+  static const int kFastPropertiesSoftLimit = 12;
+  static const int kMaxFastProperties = 128;
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(Map);
 };
 
@@ -9968,12 +9954,8 @@ class JSProxy: public JSReceiver {
   // otherwise set it to false.
   MUST_USE_RESULT
   static MaybeHandle<Object> SetPropertyViaPrototypesWithHandler(
-      Handle<JSProxy> proxy,
-      Handle<JSReceiver> receiver,
-      Handle<Name> name,
-      Handle<Object> value,
-      StrictMode strict_mode,
-      bool* done);
+      Handle<JSProxy> proxy, Handle<Object> receiver, Handle<Name> name,
+      Handle<Object> value, StrictMode strict_mode, bool* done);
 
   static PropertyAttributes GetPropertyAttributesWithHandler(
       Handle<JSProxy> proxy,
@@ -9983,6 +9965,9 @@ class JSProxy: public JSReceiver {
       Handle<JSProxy> proxy,
       Handle<JSReceiver> receiver,
       uint32_t index);
+  MUST_USE_RESULT static MaybeHandle<Object> SetPropertyWithHandler(
+      Handle<JSProxy> proxy, Handle<Object> receiver, Handle<Name> name,
+      Handle<Object> value, StrictMode strict_mode);
 
   // Turn the proxy into an (empty) JSObject.
   static void Fix(Handle<JSProxy> proxy);
@@ -10022,12 +10007,6 @@ class JSProxy: public JSReceiver {
  private:
   friend class JSReceiver;
 
-  MUST_USE_RESULT static MaybeHandle<Object> SetPropertyWithHandler(
-      Handle<JSProxy> proxy,
-      Handle<JSReceiver> receiver,
-      Handle<Name> name,
-      Handle<Object> value,
-      StrictMode strict_mode);
   MUST_USE_RESULT static inline MaybeHandle<Object> SetElementWithHandler(
       Handle<JSProxy> proxy,
       Handle<JSReceiver> receiver,

src/stub-cache.cc

@@ -581,8 +581,8 @@ RUNTIME_FUNCTION(StoreInterceptorProperty) {
   ASSERT(receiver->HasNamedInterceptor());
   Handle<Object> result;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, result, JSObject::SetPropertyWithInterceptor(
-                           receiver, name, value, ic.strict_mode()));
+      isolate, result,
+      JSObject::SetProperty(receiver, name, value, ic.strict_mode()));
   return *result;
 }
 

src/v8natives.js

@@ -1322,7 +1322,9 @@ function ObjectIsSealed(obj) {
   for (var i = 0; i < names.length; i++) {
     var name = names[i];
     var desc = GetOwnPropertyJS(obj, name);
-    if (desc.isConfigurable()) return false;
+    if (desc.isConfigurable()) {
+      return false;
+    }
   }
   return true;
 }

test/cctest/test-decls.cc

@@ -423,11 +423,8 @@ TEST(ExistsInPrototype) {
   // Sanity check to make sure that the holder of the interceptor
   // really is the prototype object.
   { ExistsInPrototypeContext context;
-    context.Check("this.x = 87; this.x",
-                  0,
-                  0,
-                  0,
-                  EXPECT_RESULT, Number::New(CcTest::isolate(), 87));
+    context.Check("this.x = 87; this.x", 0, 0, 1, EXPECT_RESULT,
+                  Number::New(CcTest::isolate(), 87));
   }
 
   { ExistsInPrototypeContext context;