Revision: 10956
Author: [email protected]
Date: Wed Mar 7 05:24:44 2012
Log: Make the runtime entry for setting/changing accessors "atomic".
Previously, there were 1 or 2 calls to the runtime when accessors were
changed
or set. This doesn't really work well with property attributes, leading to
some
hacks and complicates things even further when trying to share maps in
presence
of accessors. Therefore, the runtime entry now takes the full triple
(getter,
setter, attributes), where the getter and/or the setter can be null in case
they
shouldn't be changed.
For now, we do basically the same on the native side as we did before on the
JavaScript side, but this will change in future CLs, the current CL is
already
large enough.
Note that object literals with a getter and a setter for the same property
still
do 2 calls, but this is a little bit more tricky to fix and will be handled
in a
separate CL.
Review URL: https://chromiumcodereview.appspot.com/9616016
http://code.google.com/p/v8/source/detail?r=10956
Modified:
/branches/bleeding_edge/src/arm/full-codegen-arm.cc
/branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
/branches/bleeding_edge/src/messages.js
/branches/bleeding_edge/src/mips/full-codegen-mips.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/regexp.js
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/v8natives.js
/branches/bleeding_edge/src/x64/full-codegen-x64.cc
/branches/bleeding_edge/test/mjsunit/object-define-property.js
=======================================
--- /branches/bleeding_edge/src/arm/full-codegen-arm.cc Fri Mar 2 03:33:33
2012
+++ /branches/bleeding_edge/src/arm/full-codegen-arm.cc Wed Mar 7 05:24:44
2012
@@ -1498,11 +1498,15 @@
__ ldr(r0, MemOperand(sp));
__ push(r0);
VisitForStackValue(key);
- __ mov(r1, Operand(property->kind() ==
ObjectLiteral::Property::SETTER ?
- Smi::FromInt(1) :
- Smi::FromInt(0)));
- __ push(r1);
- VisitForStackValue(value);
+ if (property->kind() == ObjectLiteral::Property::GETTER) {
+ VisitForStackValue(value);
+ __ LoadRoot(r1, Heap::kNullValueRootIndex);
+ __ push(r1);
+ } else {
+ __ LoadRoot(r1, Heap::kNullValueRootIndex);
+ __ push(r1);
+ VisitForStackValue(value);
+ }
__ mov(r0, Operand(Smi::FromInt(NONE)));
__ push(r0);
__ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Fri Mar 2
03:33:33 2012
+++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Wed Mar 7
05:24:44 2012
@@ -1519,10 +1519,13 @@
case ObjectLiteral::Property::GETTER:
__ push(Operand(esp, 0)); // Duplicate receiver.
VisitForStackValue(key);
- __ push(Immediate(property->kind() ==
ObjectLiteral::Property::SETTER ?
- Smi::FromInt(1) :
- Smi::FromInt(0)));
- VisitForStackValue(value);
+ if (property->kind() == ObjectLiteral::Property::GETTER) {
+ VisitForStackValue(value);
+ __ push(Immediate(isolate()->factory()->null_value()));
+ } else {
+ __ push(Immediate(isolate()->factory()->null_value()));
+ VisitForStackValue(value);
+ }
__ push(Immediate(Smi::FromInt(NONE)));
__ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
break;
=======================================
--- /branches/bleeding_edge/src/messages.js Mon Mar 5 05:57:48 2012
+++ /branches/bleeding_edge/src/messages.js Wed Mar 7 05:24:44 2012
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// 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:
@@ -769,8 +769,7 @@
hasBeenSet = true;
value = v;
};
- %DefineOrRedefineAccessorProperty(obj, name, GETTER, getter, DONT_ENUM);
- %DefineOrRedefineAccessorProperty(obj, name, SETTER, setter, DONT_ENUM);
+ %DefineOrRedefineAccessorProperty(obj, name, getter, setter, DONT_ENUM);
}
function CallSite(receiver, fun, pos) {
=======================================
--- /branches/bleeding_edge/src/mips/full-codegen-mips.cc Wed Feb 29
04:05:58 2012
+++ /branches/bleeding_edge/src/mips/full-codegen-mips.cc Wed Mar 7
05:24:44 2012
@@ -1500,11 +1500,15 @@
__ lw(a0, MemOperand(sp));
__ push(a0);
VisitForStackValue(key);
- __ li(a1, Operand(property->kind() ==
ObjectLiteral::Property::SETTER ?
- Smi::FromInt(1) :
- Smi::FromInt(0)));
- __ push(a1);
- VisitForStackValue(value);
+ if (property->kind() == ObjectLiteral::Property::GETTER) {
+ VisitForStackValue(value);
+ __ LoadRoot(a1, Heap::kNullValueRootIndex);
+ __ push(a1);
+ } else {
+ __ LoadRoot(a1, Heap::kNullValueRootIndex);
+ __ push(a1);
+ VisitForStackValue(value);
+ }
__ li(a0, Operand(Smi::FromInt(NONE)));
__ push(a0);
__ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
=======================================
--- /branches/bleeding_edge/src/objects.h Wed Mar 7 02:03:32 2012
+++ /branches/bleeding_edge/src/objects.h Wed Mar 7 05:24:44 2012
@@ -854,6 +854,8 @@
class Object : public MaybeObject {
public:
// Type testing.
+ bool IsObject() { return true; }
+
#define IS_TYPE_FUNCTION_DECL(type_) inline bool Is##type_();
OBJECT_TYPE_LIST(IS_TYPE_FUNCTION_DECL)
HEAP_OBJECT_TYPE_LIST(IS_TYPE_FUNCTION_DECL)
=======================================
--- /branches/bleeding_edge/src/regexp.js Tue Feb 21 04:47:27 2012
+++ /branches/bleeding_edge/src/regexp.js Wed Mar 7 05:24:44 2012
@@ -1,4 +1,4 @@
-// Copyright 2006-2009 the V8 project authors. All rights reserved.
+// 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:
@@ -421,18 +421,12 @@
LAST_INPUT(lastMatchInfo) = ToString(string);
};
- %DefineOrRedefineAccessorProperty($RegExp, 'input', GETTER,
RegExpGetInput,
- DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, 'input', SETTER,
RegExpSetInput,
- DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$_', GETTER, RegExpGetInput,
- DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$_', SETTER, RegExpSetInput,
- DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$input', GETTER,
RegExpGetInput,
- DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$input', SETTER,
RegExpSetInput,
- DONT_ENUM | DONT_DELETE);
+ %DefineOrRedefineAccessorProperty($RegExp, 'input', RegExpGetInput,
+ RegExpSetInput, DONT_DELETE);
+ %DefineOrRedefineAccessorProperty($RegExp, '$_', RegExpGetInput,
+ RegExpSetInput, DONT_ENUM |
DONT_DELETE);
+ %DefineOrRedefineAccessorProperty($RegExp, '$input', RegExpGetInput,
+ RegExpSetInput, DONT_ENUM |
DONT_DELETE);
// The properties multiline and $* are aliases for each other. When this
// value is set in SpiderMonkey, the value it is set to is coerced to a
@@ -446,58 +440,39 @@
var RegExpGetMultiline = function() { return multiline; };
var RegExpSetMultiline = function(flag) { multiline = flag ? true :
false; };
- %DefineOrRedefineAccessorProperty($RegExp, 'multiline', GETTER,
- RegExpGetMultiline, DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, 'multiline', SETTER,
+ %DefineOrRedefineAccessorProperty($RegExp, 'multiline',
RegExpGetMultiline,
RegExpSetMultiline, DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$*', GETTER,
RegExpGetMultiline,
+ %DefineOrRedefineAccessorProperty($RegExp, '$*', RegExpGetMultiline,
+ RegExpSetMultiline,
DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$*', SETTER,
RegExpSetMultiline,
- DONT_ENUM | DONT_DELETE);
var NoOpSetter = function(ignored) {};
// Static properties set by a successful match.
- %DefineOrRedefineAccessorProperty($RegExp, 'lastMatch', GETTER,
- RegExpGetLastMatch, DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, 'lastMatch', SETTER,
NoOpSetter,
+ %DefineOrRedefineAccessorProperty($RegExp, 'lastMatch',
RegExpGetLastMatch,
+ NoOpSetter, DONT_DELETE);
+ %DefineOrRedefineAccessorProperty($RegExp, '$&', RegExpGetLastMatch,
+ NoOpSetter, DONT_ENUM | DONT_DELETE);
+ %DefineOrRedefineAccessorProperty($RegExp, 'lastParen',
RegExpGetLastParen,
+ NoOpSetter, DONT_DELETE);
+ %DefineOrRedefineAccessorProperty($RegExp, '$+', RegExpGetLastParen,
+ NoOpSetter, DONT_ENUM | DONT_DELETE);
+ %DefineOrRedefineAccessorProperty($RegExp, 'leftContext',
+ RegExpGetLeftContext, NoOpSetter,
DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$&', GETTER,
RegExpGetLastMatch,
- DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$&', SETTER, NoOpSetter,
- DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, 'lastParen', GETTER,
- RegExpGetLastParen, DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, 'lastParen', SETTER,
NoOpSetter,
+ %DefineOrRedefineAccessorProperty($RegExp, '$`', RegExpGetLeftContext,
+ NoOpSetter, DONT_ENUM | DONT_DELETE);
+ %DefineOrRedefineAccessorProperty($RegExp, 'rightContext',
+ RegExpGetRightContext, NoOpSetter,
DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$+', GETTER,
RegExpGetLastParen,
- DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$+', SETTER, NoOpSetter,
- DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, 'leftContext', GETTER,
- RegExpGetLeftContext, DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, 'leftContext', SETTER,
NoOpSetter,
- DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$`', GETTER,
RegExpGetLeftContext,
- DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$`', SETTER, NoOpSetter,
- DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, 'rightContext', GETTER,
- RegExpGetRightContext, DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, 'rightContext', SETTER,
NoOpSetter,
- DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, "$'", GETTER,
- RegExpGetRightContext,
- DONT_ENUM | DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, "$'", SETTER, NoOpSetter,
- DONT_ENUM | DONT_DELETE);
+ %DefineOrRedefineAccessorProperty($RegExp, "$'", RegExpGetRightContext,
+ NoOpSetter, DONT_ENUM | DONT_DELETE);
for (var i = 1; i < 10; ++i) {
- %DefineOrRedefineAccessorProperty($RegExp, '$' + i, GETTER,
- RegExpMakeCaptureGetter(i),
DONT_DELETE);
- %DefineOrRedefineAccessorProperty($RegExp, '$' + i, SETTER, NoOpSetter,
+ %DefineOrRedefineAccessorProperty($RegExp, '$' + i,
+ RegExpMakeCaptureGetter(i),
NoOpSetter,
DONT_DELETE);
}
}
=======================================
--- /branches/bleeding_edge/src/runtime.cc Wed Mar 7 03:48:36 2012
+++ /branches/bleeding_edge/src/runtime.cc Wed Mar 7 05:24:44 2012
@@ -995,23 +995,14 @@
DESCRIPTOR_SIZE
};
-// Returns an array with the property description:
-// if args[1] is not a property on args[0]
-// returns undefined
-// if args[1] is a data property on args[0]
-// [false, value, Writeable, Enumerable, Configurable]
-// if args[1] is an accessor on args[0]
-// [true, GetFunction, SetFunction, Enumerable, Configurable]
-RUNTIME_FUNCTION(MaybeObject*, Runtime_GetOwnProperty) {
- ASSERT(args.length() == 2);
+
+static MaybeObject* GetOwnProperty(Isolate* isolate,
+ Handle<JSObject> obj,
+ Handle<String> name) {
Heap* heap = isolate->heap();
- HandleScope scope(isolate);
Handle<FixedArray> elms =
isolate->factory()->NewFixedArray(DESCRIPTOR_SIZE);
Handle<JSArray> desc = isolate->factory()->NewJSArrayWithElements(elms);
LookupResult result(isolate);
- CONVERT_ARG_HANDLE_CHECKED(JSObject, obj, 0);
- CONVERT_ARG_HANDLE_CHECKED(String, name, 1);
-
// This could be an element.
uint32_t index;
if (name->AsArrayIndex(&index)) {
@@ -1143,6 +1134,22 @@
return *desc;
}
+
+
+// Returns an array with the property description:
+// if args[1] is not a property on args[0]
+// returns undefined
+// if args[1] is a data property on args[0]
+// [false, value, Writeable, Enumerable, Configurable]
+// if args[1] is an accessor on args[0]
+// [true, GetFunction, SetFunction, Enumerable, Configurable]
+RUNTIME_FUNCTION(MaybeObject*, Runtime_GetOwnProperty) {
+ ASSERT(args.length() == 2);
+ HandleScope scope(isolate);
+ CONVERT_ARG_HANDLE_CHECKED(JSObject, obj, 0);
+ CONVERT_ARG_HANDLE_CHECKED(String, name, 1);
+ return GetOwnProperty(isolate, obj, name);
+}
RUNTIME_FUNCTION(MaybeObject*, Runtime_PreventExtensions) {
@@ -4306,6 +4313,12 @@
args.at<Object>(0),
args.at<Object>(1));
}
+
+
+static bool IsValidAccessor(Handle<Object> obj) {
+ return obj->IsUndefined() || obj->IsSpecFunction() || obj->IsNull();
+}
+
// Implements part of 8.12.9 DefineOwnProperty.
// There are 3 cases that lead here:
@@ -4317,18 +4330,37 @@
ASSERT(args.length() == 5);
HandleScope scope(isolate);
CONVERT_ARG_HANDLE_CHECKED(JSObject, obj, 0);
- CONVERT_ARG_CHECKED(String, name, 1);
- CONVERT_SMI_ARG_CHECKED(flag, 2);
- Object* fun = args[3];
+ RUNTIME_ASSERT(!obj->IsNull());
+ CONVERT_ARG_HANDLE_CHECKED(String, name, 1);
+ CONVERT_ARG_HANDLE_CHECKED(Object, getter, 2);
+ RUNTIME_ASSERT(IsValidAccessor(getter));
+ CONVERT_ARG_HANDLE_CHECKED(Object, setter, 3);
+ RUNTIME_ASSERT(IsValidAccessor(setter));
CONVERT_SMI_ARG_CHECKED(unchecked, 4);
-
RUNTIME_ASSERT((unchecked & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) ==
0);
PropertyAttributes attr = static_cast<PropertyAttributes>(unchecked);
- RUNTIME_ASSERT(!obj->IsNull());
- RUNTIME_ASSERT(fun->IsSpecFunction() || fun->IsUndefined());
- AccessorComponent component = flag == 0 ? ACCESSOR_GETTER :
ACCESSOR_SETTER;
- return obj->DefineAccessor(name, component, fun, attr);
+ // TODO(svenpanne) Define getter/setter/attributes in a single step.
+ if (getter->IsNull() && setter->IsNull()) {
+ JSArray* array;
+ { MaybeObject* maybe_array = GetOwnProperty(isolate, obj, name);
+ if (!maybe_array->To(&array)) return maybe_array;
+ }
+ Object* current =
FixedArray::cast(array->elements())->get(GETTER_INDEX);
+ getter = Handle<Object>(current, isolate);
+ }
+ if (!getter->IsNull()) {
+ MaybeObject* ok =
+ obj->DefineAccessor(*name, ACCESSOR_GETTER, *getter, attr);
+ if (ok->IsFailure()) return ok;
+ }
+ if (!setter->IsNull()) {
+ MaybeObject* ok =
+ obj->DefineAccessor(*name, ACCESSOR_SETTER, *setter, attr);
+ if (ok->IsFailure()) return ok;
+ }
+
+ return isolate->heap()->undefined_value();
}
// Implements part of 8.12.9 DefineOwnProperty.
@@ -4342,9 +4374,8 @@
HandleScope scope(isolate);
CONVERT_ARG_HANDLE_CHECKED(JSObject, js_object, 0);
CONVERT_ARG_HANDLE_CHECKED(String, name, 1);
- Handle<Object> obj_value = args.at<Object>(2);
+ CONVERT_ARG_HANDLE_CHECKED(Object, obj_value, 2);
CONVERT_SMI_ARG_CHECKED(unchecked, 3);
-
RUNTIME_ASSERT((unchecked & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) ==
0);
PropertyAttributes attr = static_cast<PropertyAttributes>(unchecked);
=======================================
--- /branches/bleeding_edge/src/v8natives.js Mon Mar 5 05:57:48 2012
+++ /branches/bleeding_edge/src/v8natives.js Wed Mar 7 05:24:44 2012
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// 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:
@@ -834,10 +834,6 @@
}
%DefineOrRedefineDataProperty(obj, p, value, flag);
- } else if (IsGenericDescriptor(desc)) {
- // Step 12 - updating an existing accessor property with generic
- // descriptor. Changing flags only.
- %DefineOrRedefineAccessorProperty(obj, p, GETTER, current.getGet(),
flag);
} else {
// There are 3 cases that lead here:
// Step 4b - defining a new accessor property.
@@ -845,12 +841,9 @@
// property.
// Step 12 - updating an existing accessor property with an accessor
// descriptor.
- if (desc.hasGetter()) {
- %DefineOrRedefineAccessorProperty(obj, p, GETTER, desc.getGet(),
flag);
- }
- if (desc.hasSetter()) {
- %DefineOrRedefineAccessorProperty(obj, p, SETTER, desc.getSet(),
flag);
- }
+ var getter = desc.hasGetter() ? desc.getGet() : null;
+ var setter = desc.hasSetter() ? desc.getSet() : null;
+ %DefineOrRedefineAccessorProperty(obj, p, getter, setter, flag);
}
return true;
}
=======================================
--- /branches/bleeding_edge/src/x64/full-codegen-x64.cc Fri Mar 2 03:33:33
2012
+++ /branches/bleeding_edge/src/x64/full-codegen-x64.cc Wed Mar 7 05:24:44
2012
@@ -1459,10 +1459,13 @@
case ObjectLiteral::Property::GETTER:
__ push(Operand(rsp, 0)); // Duplicate receiver.
VisitForStackValue(key);
- __ Push(property->kind() == ObjectLiteral::Property::SETTER ?
- Smi::FromInt(1) :
- Smi::FromInt(0));
- VisitForStackValue(value);
+ if (property->kind() == ObjectLiteral::Property::GETTER) {
+ VisitForStackValue(value);
+ __ PushRoot(Heap::kNullValueRootIndex);
+ } else {
+ __ PushRoot(Heap::kNullValueRootIndex);
+ VisitForStackValue(value);
+ }
__ Push(Smi::FromInt(NONE));
__ CallRuntime(Runtime::kDefineOrRedefineAccessorProperty, 5);
break;
=======================================
--- /branches/bleeding_edge/test/mjsunit/object-define-property.js Wed Mar
7 02:03:32 2012
+++ /branches/bleeding_edge/test/mjsunit/object-define-property.js Wed Mar
7 05:24:44 2012
@@ -1,4 +1,4 @@
-// Copyright 2010 the V8 project authors. All rights reserved.
+// 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:
@@ -503,7 +503,7 @@
// Defining properties null should fail even when we have
// other allowed values
try {
- %DefineOrRedefineAccessorProperty(null, 'foo', 0, func, 0);
+ %DefineOrRedefineAccessorProperty(null, 'foo', func, null, 0);
} catch (e) {
assertTrue(/illegal access/.test(e));
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
