Revision: 11578
Author: [email protected]
Date: Wed May 16 04:07:54 2012
Log: Revert r11496.
CL being reverted: https://chromiumcodereview.appspot.com/10238005
BUG=128146
TEST=regress-128146
Review URL: https://chromiumcodereview.appspot.com/10386166
http://code.google.com/p/v8/source/detail?r=11578
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-128146.js
Deleted:
/branches/bleeding_edge/test/mjsunit/accessor-map-sharing.js
Modified:
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-128146.js Wed May
16 04:07:54 2012
@@ -0,0 +1,30 @@
+// Copyright 2012 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
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+Object.defineProperty({},"foo",{set:function(){},configurable:false});
+Object.defineProperty({},"foo",{get:function(){},configurable:false});
+Object.defineProperty({},"foo",{});
=======================================
--- /branches/bleeding_edge/test/mjsunit/accessor-map-sharing.js Thu May 3
05:41:40 2012
+++ /dev/null
@@ -1,176 +0,0 @@
-// Copyright 2012 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
-// with the distribution.
-// * Neither the name of Google Inc. nor the names of its
-// contributors may be used to endorse or promote products derived
-// from this software without specific prior written permission.
-//
-// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-// Flags: --allow-natives-syntax
-
-// Handy abbreviations.
-var dp = Object.defineProperty;
-var gop = Object.getOwnPropertyDescriptor;
-
-function getter() { return 111; }
-function setter(x) { print(222); }
-function anotherGetter() { return 333; }
-function anotherSetter(x) { print(444); }
-var obj1, obj2;
-
-// Two objects with the same getter.
-obj1 = {};
-dp(obj1, "alpha", { get: getter });
-obj2 = {};
-dp(obj2, "alpha", { get: getter });
-assertTrue(%HaveSameMap(obj1, obj2));
-
-// Two objects with the same getter, oldskool.
-obj1 = {};
-obj1.__defineGetter__("bravo", getter);
-assertEquals(getter, obj1.__lookupGetter__("bravo"));
-obj2 = {};
-obj2.__defineGetter__("bravo", getter);
-assertEquals(getter, obj2.__lookupGetter__("bravo"));
-assertTrue(%HaveSameMap(obj1, obj2));
-
-// Two objects with the same setter.
-obj1 = {};
-dp(obj1, "charlie", { set: setter });
-obj2 = {};
-dp(obj2, "charlie", { set: setter });
-assertTrue(%HaveSameMap(obj1, obj2));
-
-// Two objects with the same setter, oldskool.
-obj1 = {};
-obj1.__defineSetter__("delta", setter);
-assertEquals(setter, obj1.__lookupSetter__("delta"));
-obj2 = {};
-obj2.__defineSetter__("delta", setter);
-assertEquals(setter, obj2.__lookupSetter__("delta"));
-assertTrue(%HaveSameMap(obj1, obj2));
-
-// Two objects with the same getter and setter.
-obj1 = {};
-dp(obj1, "foxtrot", { get: getter, set: setter });
-obj2 = {};
-dp(obj2, "foxtrot", { get: getter, set: setter });
-assertTrue(%HaveSameMap(obj1, obj2));
-
-// Two objects with the same getter and setter, set separately.
-obj1 = {};
-dp(obj1, "golf", { get: getter, configurable: true });
-dp(obj1, "golf", { set: setter, configurable: true });
-obj2 = {};
-dp(obj2, "golf", { get: getter, configurable: true });
-dp(obj2, "golf", { set: setter, configurable: true });
-assertTrue(%HaveSameMap(obj1, obj2));
-
-// Two objects with the same getter and setter, set separately, oldskool.
-obj1 = {};
-obj1.__defineGetter__("hotel", getter);
-obj1.__defineSetter__("hotel", setter);
-obj2 = {};
-obj2.__defineGetter__("hotel", getter);
-obj2.__defineSetter__("hotel", setter);
-assertTrue(%HaveSameMap(obj1, obj2));
-
-// Attribute-only change, shouldn't affect previous descriptor properties.
-obj1 = {};
-dp(obj1, "india", { get: getter, configurable: true, enumerable: true });
-assertEquals(getter, gop(obj1, "india").get);
-assertTrue(gop(obj1, "india").configurable);
-assertTrue(gop(obj1, "india").enumerable);
-dp(obj1, "india", { enumerable: false });
-assertEquals(getter, gop(obj1, "india").get);
-assertTrue(gop(obj1, "india").configurable);
-assertFalse(gop(obj1, "india").enumerable);
-
-// Attribute-only change, shouldn't affect objects with previously shared
maps.
-obj1 = {};
-dp(obj1, "juliet", { set: setter, configurable: true, enumerable: false });
-assertEquals(setter, gop(obj1, "juliet").set);
-assertTrue(gop(obj1, "juliet").configurable);
-assertFalse(gop(obj1, "juliet").enumerable);
-obj2 = {};
-dp(obj2, "juliet", { set: setter, configurable: true, enumerable: false });
-assertEquals(setter, gop(obj2, "juliet").set);
-assertTrue(gop(obj2, "juliet").configurable);
-assertFalse(gop(obj2, "juliet").enumerable);
-dp(obj1, "juliet", { set: setter, configurable: false, enumerable: true });
-assertEquals(setter, gop(obj1, "juliet").set);
-assertFalse(gop(obj1, "juliet").configurable);
-assertTrue(gop(obj1, "juliet").enumerable);
-assertEquals(setter, gop(obj2, "juliet").set);
-assertTrue(gop(obj2, "juliet").configurable);
-assertFalse(gop(obj2, "juliet").enumerable);
-
-// Two objects with the different getters.
-obj1 = {};
-dp(obj1, "kilo", { get: getter });
-obj2 = {};
-dp(obj2, "kilo", { get: anotherGetter });
-assertEquals(getter, gop(obj1, "kilo").get);
-assertEquals(anotherGetter, gop(obj2, "kilo").get);
-assertFalse(%HaveSameMap(obj1, obj2));
-
-// Two objects with the same getters and different setters.
-obj1 = {};
-dp(obj1, "lima", { get: getter, set: setter });
-obj2 = {};
-dp(obj2, "lima", { get: getter, set: anotherSetter });
-assertEquals(setter, gop(obj1, "lima").set);
-assertEquals(anotherSetter, gop(obj2, "lima").set);
-assertFalse(%HaveSameMap(obj1, obj2));
-
-// Even 'undefined' is a kind of getter.
-obj1 = {};
-dp(obj1, "mike", { get: undefined });
-assertTrue("mike" in obj1);
-assertEquals(undefined, gop(obj1, "mike").get);
-assertEquals(undefined, obj1.__lookupGetter__("mike"));
-assertEquals(undefined, gop(obj1, "mike").set);
-assertEquals(undefined, obj1.__lookupSetter__("mike"));
-
-// Even 'undefined' is a kind of setter.
-obj1 = {};
-dp(obj1, "november", { set: undefined });
-assertTrue("november" in obj1);
-assertEquals(undefined, gop(obj1, "november").get);
-assertEquals(undefined, obj1.__lookupGetter__("november"));
-assertEquals(undefined, gop(obj1, "november").set);
-assertEquals(undefined, obj1.__lookupSetter__("november"));
-
-// Redefining a data property.
-obj1 = {};
-obj1.oscar = 12345;
-dp(obj1, "oscar", { set: setter });
-assertEquals(setter, gop(obj1, "oscar").set);
-
-// Re-adding the same getter/attributes pair.
-obj1 = {};
-dp(obj1, "papa", { get: getter, configurable: true });
-dp(obj1, "papa", { get: getter, set: setter, configurable: true });
-assertEquals(getter, gop(obj1, "papa").get);
-assertEquals(setter, gop(obj1, "papa").set);
-assertTrue(gop(obj1, "papa").configurable);
-assertFalse(gop(obj1, "papa").enumerable);
=======================================
--- /branches/bleeding_edge/src/objects.cc Wed May 9 08:18:50 2012
+++ /branches/bleeding_edge/src/objects.cc Wed May 16 04:07:54 2012
@@ -4417,12 +4417,7 @@
LookupResult result(GetHeap()->isolate());
LocalLookupRealNamedProperty(name, &result);
if (result.IsProperty() && result.type() == CALLBACKS) {
- // Note that the result can actually have IsDontDelete() == true when
we
- // e.g. have to fall back to the slow case while adding a setter after
- // successfully reusing a map transition for a getter. Nevertheless,
this is
- // OK, because the assertion only holds for the whole addition of both
- // accessors, not for the addition of each part. See first comment in
- // DefinePropertyAccessor below.
+ ASSERT(!result.IsDontDelete());
Object* obj = result.GetCallbackObject();
if (obj->IsAccessorPair()) {
return AccessorPair::cast(obj)->CopyWithoutTransitions();
@@ -4436,28 +4431,6 @@
Object* getter,
Object* setter,
PropertyAttributes
attributes) {
- // We could assert that the property is configurable here, but we would
need
- // to do a lookup, which seems to be a bit of overkill.
- Heap* heap = GetHeap();
- bool only_attribute_changes = getter->IsNull() && setter->IsNull();
- if (HasFastProperties() && !only_attribute_changes) {
- MaybeObject* getterOk = heap->undefined_value();
- if (!getter->IsNull()) {
- getterOk = DefineFastAccessor(name, ACCESSOR_GETTER, getter,
attributes);
- if (getterOk->IsFailure()) return getterOk;
- }
-
- MaybeObject* setterOk = heap->undefined_value();
- if (getterOk != heap->null_value() && !setter->IsNull()) {
- setterOk = DefineFastAccessor(name, ACCESSOR_SETTER, setter,
attributes);
- if (setterOk->IsFailure()) return setterOk;
- }
-
- if (getterOk != heap->null_value() && setterOk != heap->null_value()) {
- return heap->undefined_value();
- }
- }
-
AccessorPair* accessors;
{ MaybeObject* maybe_accessors = CreateAccessorPairFor(name);
if (!maybe_accessors->To(&accessors)) return maybe_accessors;
@@ -4605,159 +4578,6 @@
DefineElementAccessor(index, getter, setter, attributes) :
DefinePropertyAccessor(name, getter, setter, attributes);
}
-
-
-static MaybeObject* CreateFreshAccessor(JSObject* obj,
- String* name,
- AccessorComponent component,
- Object* accessor,
- PropertyAttributes attributes) {
- // step 1: create a new getter/setter pair with only the accessor in it
- Heap* heap = obj->GetHeap();
- AccessorPair* accessors2;
- { MaybeObject* maybe_accessors2 = heap->AllocateAccessorPair();
- if (!maybe_accessors2->To(&accessors2)) return maybe_accessors2;
- }
- accessors2->set(component, accessor);
-
- // step 2: create a copy of the descriptors, incl. the new getter/setter
pair
- Map* map1 = obj->map();
- CallbacksDescriptor callbacks_descr2(name, accessors2, attributes);
- DescriptorArray* descriptors2;
- { MaybeObject* maybe_descriptors2 =
- map1->instance_descriptors()->CopyInsert(&callbacks_descr2,
- REMOVE_TRANSITIONS);
- if (!maybe_descriptors2->To(&descriptors2)) return maybe_descriptors2;
- }
-
- // step 3: create a new map with the new descriptors
- Map* map2;
- { MaybeObject* maybe_map2 = map1->CopyDropDescriptors();
- if (!maybe_map2->To(&map2)) return maybe_map2;
- }
- map2->set_instance_descriptors(descriptors2);
-
- // step 4: create a new getter/setter pair with a transition to the new
map
- AccessorPair* accessors1;
- { MaybeObject* maybe_accessors1 = heap->AllocateAccessorPair();
- if (!maybe_accessors1->To(&accessors1)) return maybe_accessors1;
- }
- accessors1->set(component, map2);
-
- // step 5: create a copy of the descriptors, incl. the new getter/setter
pair
- // with the transition
- CallbacksDescriptor callbacks_descr1(name, accessors1, attributes);
- DescriptorArray* descriptors1;
- { MaybeObject* maybe_descriptors1 =
- map1->instance_descriptors()->CopyInsert(&callbacks_descr1,
- KEEP_TRANSITIONS);
- if (!maybe_descriptors1->To(&descriptors1)) return maybe_descriptors1;
- }
-
- // step 6: everything went well so far, so we make our changes visible
- obj->set_map(map2);
- map1->set_instance_descriptors(descriptors1);
- map2->SetBackPointer(map1);
- return obj;
-}
-
-
-static bool TransitionToSameAccessor(Object* map,
- String* name,
- AccessorComponent component,
- Object* accessor,
- PropertyAttributes attributes ) {
- DescriptorArray* descs = Map::cast(map)->instance_descriptors();
- int number = descs->SearchWithCache(name);
- ASSERT(number != DescriptorArray::kNotFound);
- Object* target_accessor =
-
AccessorPair::cast(descs->GetCallbacksObject(number))->get(component);
- PropertyAttributes target_attributes =
descs->GetDetails(number).attributes();
- return target_accessor == accessor && target_attributes == attributes;
-}
-
-
-static MaybeObject* NewCallbackTransition(JSObject* obj,
- String* name,
- AccessorComponent component,
- Object* accessor,
- PropertyAttributes attributes,
- AccessorPair* accessors2) {
- // step 1: copy the old getter/setter pair and set the new accessor
- AccessorPair* accessors3;
- { MaybeObject* maybe_accessors3 = accessors2->CopyWithoutTransitions();
- if (!maybe_accessors3->To(&accessors3)) return maybe_accessors3;
- }
- accessors3->set(component, accessor);
-
- // step 2: create a copy of the descriptors, incl. the new getter/setter
pair
- Map* map2 = obj->map();
- CallbacksDescriptor callbacks_descr3(name, accessors3, attributes);
- DescriptorArray* descriptors3;
- { MaybeObject* maybe_descriptors3 =
- map2->instance_descriptors()->CopyInsert(&callbacks_descr3,
- REMOVE_TRANSITIONS);
- if (!maybe_descriptors3->To(&descriptors3)) return maybe_descriptors3;
- }
-
- // step 3: create a new map with the new descriptors
- Map* map3;
- { MaybeObject* maybe_map3 = map2->CopyDropDescriptors();
- if (!maybe_map3->To(&map3)) return maybe_map3;
- }
- map3->set_instance_descriptors(descriptors3);
-
- // step 4: everything went well so far, so we make our changes visible
- obj->set_map(map3);
- accessors2->set(component, map3);
- map3->SetBackPointer(map2);
- return obj;
-}
-
-
-MaybeObject* JSObject::DefineFastAccessor(String* name,
- AccessorComponent component,
- Object* accessor,
- PropertyAttributes attributes) {
- ASSERT(accessor->IsSpecFunction() || accessor->IsUndefined());
- LookupResult result(GetIsolate());
- LocalLookup(name, &result);
-
- // If we have a new property, create a fresh accessor plus a transition
to it.
- if (!result.IsFound()) {
- return CreateFreshAccessor(this, name, component, accessor,
attributes);
- }
-
- // If the property is not a JavaScript accessor, fall back to the slow
case.
- if (result.type() != CALLBACKS) return GetHeap()->null_value();
- Object* callback_value = result.GetValue();
- if (!callback_value->IsAccessorPair()) return GetHeap()->null_value();
- AccessorPair* accessors = AccessorPair::cast(callback_value);
-
- // Follow a callback transition, if there is a fitting one.
- Object* entry = accessors->get(component);
- if (entry->IsMap() &&
- TransitionToSameAccessor(entry, name, component, accessor,
attributes)) {
- set_map(Map::cast(entry));
- return this;
- }
-
- // When we re-add the same accessor again, there is nothing to do.
- if (entry == accessor && result.GetAttributes() == attributes) return
this;
-
- // Only the other accessor has been set so far, create a new transition.
- if (entry->IsTheHole()) {
- return NewCallbackTransition(this,
- name,
- component,
- accessor,
- attributes,
- accessors);
- }
-
- // Nothing from the above worked, so we have to fall back to the slow
case.
- return GetHeap()->null_value();
-}
MaybeObject* JSObject::DefineAccessor(AccessorInfo* info) {
@@ -6127,8 +5947,8 @@
Object* AccessorPair::GetComponent(AccessorComponent component) {
- Object* accessor = get(component);
- return accessor->IsTheHole() ? GetHeap()->undefined_value() : accessor;
+ Object* accessor = (component == ACCESSOR_GETTER) ? getter() :
setter();
+ return accessor->IsTheHole() ? GetHeap()->undefined_value() : accessor;
}
=======================================
--- /branches/bleeding_edge/src/objects.h Tue May 15 08:45:38 2012
+++ /branches/bleeding_edge/src/objects.h Wed May 16 04:07:54 2012
@@ -704,13 +704,12 @@
WriteBarrierMode mode = UPDATE_WRITE_BARRIER); \
-class AccessorPair;
class DictionaryElementsAccessor;
class ElementsAccessor;
-class Failure;
class FixedArrayBase;
class ObjectVisitor;
class StringStream;
+class Failure;
struct ValueInfo : public Malloced {
ValueInfo() : type(FIRST_TYPE), ptr(NULL), str(NULL), number(0) { }
@@ -1643,14 +1642,6 @@
Object* getter,
Object* setter,
PropertyAttributes
attributes);
- // Try to define a single accessor paying attention to map transitions.
- // Returns a JavaScript null if this was not possible and we have to use
the
- // slow case. Note that we can fail due to allocations, too.
- MUST_USE_RESULT MaybeObject* DefineFastAccessor(
- String* name,
- AccessorComponent component,
- Object* accessor,
- PropertyAttributes attributes);
Object* LookupAccessor(String* name, AccessorComponent component);
MUST_USE_RESULT MaybeObject* DefineAccessor(AccessorInfo* info);
@@ -8128,18 +8119,6 @@
MUST_USE_RESULT MaybeObject* CopyWithoutTransitions();
- Object* get(AccessorComponent component) {
- return component == ACCESSOR_GETTER ? getter() : setter();
- }
-
- void set(AccessorComponent component, Object* value) {
- if (component == ACCESSOR_GETTER) {
- set_getter(value);
- } else {
- set_setter(value);
- }
- }
-
// Note: Returns undefined instead in case of a hole.
Object* GetComponent(AccessorComponent component);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
