Reviewers: Rico,
Description:
Avoid calling inherited setters when creating object literals and their
boilerplates.
Please review this at http://codereview.chromium.org/6118001/
Affected files:
M src/handles.h
M src/handles.cc
M src/heap-inl.h
M src/objects.h
M src/objects.cc
M src/parser.cc
M src/runtime.cc
A + test/mjsunit/regress/regress-1015.js
M test/mjsunit/regress/regress-192.js
Index: src/handles.cc
diff --git a/src/handles.cc b/src/handles.cc
index
68c61b5cdb168aae904786d229da17e49046bf98..7189772f1a8026a4d6a736d1eae877a9f981a91d
100644
--- a/src/handles.cc
+++ b/src/handles.cc
@@ -422,6 +422,14 @@ Handle<Object> SetElement(Handle<JSObject> object,
}
+Handle<Object> SetOwnElement(Handle<JSObject> object,
+ uint32_t index,
+ Handle<Object> value) {
+ ASSERT(!(object->HasPixelElements() ||
object->HasExternalArrayElements()));
+ CALL_HEAP_FUNCTION(object->SetElement(index, *value, false), Object);
+}
+
+
Handle<JSObject> Copy(Handle<JSObject> obj) {
CALL_HEAP_FUNCTION(Heap::CopyJSObject(*obj), JSObject);
}
Index: src/handles.h
diff --git a/src/handles.h b/src/handles.h
index
8fd25dc9ef56f6d66517fd290cd7a9e5bfa0b24a..1816db7c2a0faa009b408be4e45269dca75bfec6
100644
--- a/src/handles.h
+++ b/src/handles.h
@@ -217,9 +217,10 @@ Handle<Object> SetNormalizedProperty(Handle<JSObject>
object,
Handle<Object> ForceDeleteProperty(Handle<JSObject> object,
Handle<Object> key);
-Handle<Object> IgnoreAttributesAndSetLocalProperty(Handle<JSObject> object,
- Handle<String> key,
- Handle<Object> value,
+Handle<Object> IgnoreAttributesAndSetLocalProperty(
+ Handle<JSObject> object,
+ Handle<String> key,
+ Handle<Object> value,
PropertyAttributes attributes);
Handle<Object> SetPropertyWithInterceptor(Handle<JSObject> object,
@@ -231,6 +232,10 @@ Handle<Object> SetElement(Handle<JSObject> object,
uint32_t index,
Handle<Object> value);
+Handle<Object> SetOwnElement(Handle<JSObject> object,
+ uint32_t index,
+ Handle<Object> value);
+
Handle<Object> GetProperty(Handle<JSObject> obj,
const char* name);
Index: src/heap-inl.h
diff --git a/src/heap-inl.h b/src/heap-inl.h
index
e7700e9cb5b05cdb42f44fcf2ffa889d3829c37c..7b91e8715a18d52294739664c8383825847e7f93
100644
--- a/src/heap-inl.h
+++ b/src/heap-inl.h
@@ -521,10 +521,6 @@ void Heap::SetLastScriptId(Object* last_script_id) {
CALL_AND_RETRY(FUNCTION_CALL, return, return)
-#define CALL_HEAP_FUNCTION_INLINE(FUNCTION_CALL) \
- CALL_AND_RETRY(FUNCTION_CALL, break, break)
-
-
#ifdef DEBUG
inline bool Heap::allow_allocation(bool new_state) {
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
927194f70daf2a14fb3d7617514f6326a293aacb..c46da4a0df8fc8cf0c1c8cd98064cfe8f5bb018b
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1823,8 +1823,9 @@ void
JSObject::LookupRealNamedPropertyInPrototypes(String* name,
// We only need to deal with CALLBACKS and INTERCEPTORS
MaybeObject* JSObject::SetPropertyWithFailedAccessCheck(LookupResult*
result,
String* name,
- Object* value) {
- if (!result->IsProperty()) {
+ Object* value,
+ bool
check_prototype) {
+ if (check_prototype && !result->IsProperty()) {
LookupCallbackSetterInPrototypes(name, result);
}
@@ -1850,7 +1851,8 @@ MaybeObject*
JSObject::SetPropertyWithFailedAccessCheck(LookupResult* result,
LookupResult r;
LookupRealNamedProperty(name, &r);
if (r.IsProperty()) {
- return SetPropertyWithFailedAccessCheck(&r, name, value);
+ return SetPropertyWithFailedAccessCheck(&r, name, value,
+ check_prototype);
}
break;
}
@@ -1891,7 +1893,7 @@ MaybeObject* JSObject::SetProperty(LookupResult*
result,
// Check access rights if needed.
if (IsAccessCheckNeeded()
&& !Top::MayNamedAccess(this, name, v8::ACCESS_SET)) {
- return SetPropertyWithFailedAccessCheck(result, name, value);
+ return SetPropertyWithFailedAccessCheck(result, name, value, true);
}
if (IsJSGlobalProxy()) {
@@ -1993,7 +1995,7 @@ MaybeObject*
JSObject::IgnoreAttributesAndSetLocalProperty(
// Check access rights if needed.
if (IsAccessCheckNeeded()
&& !Top::MayNamedAccess(this, name, v8::ACCESS_SET)) {
- return SetPropertyWithFailedAccessCheck(&result, name, value);
+ return SetPropertyWithFailedAccessCheck(&result, name, value, false);
}
if (IsJSGlobalProxy()) {
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
eac7f9208ed5676fec3c29bda43c87b87368c0ed..fd9ff46c3bb8da29fc6d9d035cad40c4fdc32fdf
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -1341,7 +1341,8 @@ class JSObject: public HeapObject {
MUST_USE_RESULT MaybeObject* SetPropertyWithFailedAccessCheck(
LookupResult* result,
String* name,
- Object* value);
+ Object* value,
+ bool check_prototype);
MUST_USE_RESULT MaybeObject* SetPropertyWithCallback(Object* structure,
String* name,
Object* value,
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
ddb13003af9561c5b5e9061f59cf1403693006c0..9faa7f73274165cd3d7b3c0b69ff20759d028bab
100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3660,11 +3660,9 @@ Handle<Object> JsonParser::ParseJsonObject() {
if (value.is_null()) return Handle<Object>::null();
uint32_t index;
if (key->AsArrayIndex(&index)) {
- CALL_HEAP_FUNCTION_INLINE(
- (*json_object)->SetElement(index, *value, true));
+ SetOwnElement(json_object, index, value);
} else {
- CALL_HEAP_FUNCTION_INLINE(
- (*json_object)->SetPropertyPostInterceptor(*key, *value,
NONE));
+ IgnoreAttributesAndSetLocalProperty(json_object, key, value, NONE);
}
} while (scanner_.Next() == Token::COMMA);
if (scanner_.current_token() != Token::RBRACE) {
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
724a4363483e0aa3a806f73cdb8a24fa8958441f..5dc6998cf8832bf7112eea4bea7ff07ef89018b7
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -330,13 +330,18 @@ static Handle<Object> CreateObjectLiteralBoilerplate(
Handle<Object> result;
uint32_t element_index = 0;
if (key->IsSymbol()) {
- // If key is a symbol it is not an array element.
- Handle<String> name(String::cast(*key));
- ASSERT(!name->AsArrayIndex(&element_index));
- result = SetProperty(boilerplate, name, value, NONE);
+ if (Handle<String>::cast(key)->AsArrayIndex(&element_index)) {
+ // Array index as string (uint32).
+ result = SetOwnElement(boilerplate, element_index, value);
+ } else {
+ Handle<String> name(String::cast(*key));
+ ASSERT(!name->AsArrayIndex(&element_index));
+ result = IgnoreAttributesAndSetLocalProperty(boilerplate, name,
+ value, NONE);
+ }
} else if (key->ToArrayIndex(&element_index)) {
// Array index (uint32).
- result = SetElement(boilerplate, element_index, value);
+ result = SetOwnElement(boilerplate, element_index, value);
} else {
// Non-uint32 number.
ASSERT(key->IsNumber());
@@ -345,7 +350,8 @@ static Handle<Object> CreateObjectLiteralBoilerplate(
Vector<char> buffer(arr, ARRAY_SIZE(arr));
const char* str = DoubleToCString(num, buffer);
Handle<String> name = Factory::NewStringFromAscii(CStrVector(str));
- result = SetProperty(boilerplate, name, value, NONE);
+ result = IgnoreAttributesAndSetLocalProperty(boilerplate, name,
+ value, NONE);
}
// If setting the property on the boilerplate throws an
// exception, the exception is converted to an empty handle in
Index: test/mjsunit/regress/regress-1015.js
diff --git a/test/mjsunit/bugs/bug-1015.js
b/test/mjsunit/regress/regress-1015.js
similarity index 100%
rename from test/mjsunit/bugs/bug-1015.js
rename to test/mjsunit/regress/regress-1015.js
Index: test/mjsunit/regress/regress-192.js
diff --git a/test/mjsunit/regress/regress-192.js
b/test/mjsunit/regress/regress-192.js
index
8f0978f1b6e798d9a69f1cde9c4455720dd4f4ef..a8e5e9da3305f19c0a73d319d77d388c0f0d31f1
100644
--- a/test/mjsunit/regress/regress-192.js
+++ b/test/mjsunit/regress/regress-192.js
@@ -30,9 +30,16 @@
// See http://code.google.com/p/v8/issues/detail?id=192
+// UPDATE: This bug report is no longer valid. In ES5, creating object
+// literals MUST NOT trigger inherited accessors, but act as if creating
+// the properties using DefineOwnProperty.
+
Object.prototype.__defineGetter__("x", function() {});
+Object.prototype.__defineSetter__("y",
+ function() {
assertUnreachable("setter"); });
-// Creating this object literal will throw an exception because we are
+// Creating this object literal will *not* throw an exception because we
are
// assigning to a property that has only a getter.
-assertThrows("({ x: 42 })");
-
+var x = ({ x: 42, y: 37 });
+assertEquals(42, x.x);
+assertEquals(37, x.y);
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
