Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vm: remove CopyProperties()- code review #13265

Closed
wants to merge 1 commit into from
Closed

vm: remove CopyProperties()- code review #13265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions deps/v8/include/v8.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3033,6 +3033,14 @@ enum class IndexFilter { kIncludeIndices, kSkipIndices };
*/
enum class IntegrityLevel { kFrozen, kSealed };

/**
* DONT_SKIP_INTERCEPTORS is the default behaviour and triggers the definer
* interceptor when setting an own property on an object using
* DefineProperty, while SKIP_INTERCEPTORS bypasses the interceptors.
*/
enum CallInterceptors { DONT_SKIP_INTERCEPTORS, SKIP_INTERCEPTORS };


/**
* A JavaScript object (ECMA-262, 4.3.3)
*/
Expand Down Expand Up @@ -3070,7 +3078,8 @@ class V8_EXPORT Object : public Value {
// Returns true on success.
V8_WARN_UNUSED_RESULT Maybe<bool> DefineOwnProperty(
Local<Context> context, Local<Name> key, Local<Value> value,
PropertyAttribute attributes = None);
PropertyAttribute attributes = None,
CallInterceptors call_interceptors = DONT_SKIP_INTERCEPTORS);

// Implements Object.DefineProperty(O, P, Attributes), see Ecma-262 19.1.2.4.
//
Expand All @@ -3084,9 +3093,13 @@ class V8_EXPORT Object : public Value {
//
// The PropertyDescriptor can change when redefining a property.
//
// CallInterceptors call_interceptors = SKIP_INTERCEPTORS
// bypasses interceptors when setting property on an object.
//
// Returns true on success.
V8_WARN_UNUSED_RESULT Maybe<bool> DefineProperty(
Local<Context> context, Local<Name> key, PropertyDescriptor& descriptor);
Local<Context> context, Local<Name> key, PropertyDescriptor& descriptor,
CallInterceptors call_interceptors = DONT_SKIP_INTERCEPTORS);

// Sets an own property on this object bypassing interceptors and
// overriding accessors or read-only properties.
Expand Down
10 changes: 6 additions & 4 deletions deps/v8/src/api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4436,7 +4436,8 @@ bool v8::PropertyDescriptor::has_configurable() const {
Maybe<bool> v8::Object::DefineOwnProperty(v8::Local<v8::Context> context,
v8::Local<Name> key,
v8::Local<Value> value,
v8::PropertyAttribute attributes) {
v8::PropertyAttribute attributes,
CallInterceptors call_interceptors) {
auto isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
ENTER_V8(isolate, context, Object, DefineOwnProperty, Nothing<bool>(),
i::HandleScope);
Expand All @@ -4450,15 +4451,16 @@ Maybe<bool> v8::Object::DefineOwnProperty(v8::Local<v8::Context> context,
desc.set_configurable(!(attributes & v8::DontDelete));
desc.set_value(value_obj);
Maybe<bool> success = i::JSReceiver::DefineOwnProperty(
isolate, self, key_obj, &desc, i::Object::DONT_THROW);
isolate, self, key_obj, &desc, i::Object::DONT_THROW, call_interceptors);
// Even though we said DONT_THROW, there might be accessors that do throw.
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
return success;
}

Maybe<bool> v8::Object::DefineProperty(v8::Local<v8::Context> context,
v8::Local<Name> key,
PropertyDescriptor& descriptor) {
PropertyDescriptor& descriptor,
CallInterceptors call_interceptors) {
auto isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
ENTER_V8(isolate, context, Object, DefineOwnProperty, Nothing<bool>(),
i::HandleScope);
Expand All @@ -4467,7 +4469,7 @@ Maybe<bool> v8::Object::DefineProperty(v8::Local<v8::Context> context,

Maybe<bool> success = i::JSReceiver::DefineOwnProperty(
isolate, self, key_obj, &descriptor.get_private()->desc,
i::Object::DONT_THROW);
i::Object::DONT_THROW, call_interceptors);
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
return success;
}
Expand Down
5 changes: 2 additions & 3 deletions deps/v8/src/builtins/builtins-object.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,8 @@ Object* ObjectDefineAccessor(Isolate* isolate, Handle<Object> object,
// To preserve legacy behavior, we ignore errors silently rather than
// throwing an exception.
Maybe<bool> success = JSReceiver::DefineOwnProperty(
isolate, receiver, name, &desc,
FLAG_harmony_strict_legacy_accessor_builtins ? Object::THROW_ON_ERROR
: Object::DONT_THROW);
isolate, receiver, name, &desc, Object::DONT_THROW,
DONT_SKIP_INTERCEPTORS);
MAYBE_RETURN(success, isolate->heap()->exception());
if (!success.FromJust()) {
isolate->CountUsage(v8::Isolate::kDefineGetterOrSetterWouldThrow);
Expand Down
76 changes: 48 additions & 28 deletions deps/v8/src/objects.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6445,7 +6445,8 @@ Maybe<bool> JSReceiver::DeletePropertyOrElement(Handle<JSReceiver> object,
// static
Object* JSReceiver::DefineProperty(Isolate* isolate, Handle<Object> object,
Handle<Object> key,
Handle<Object> attributes) {
Handle<Object> attributes,
CallInterceptors call_interceptors) {
// 1. If Type(O) is not Object, throw a TypeError exception.
if (!object->IsJSReceiver()) {
Handle<String> fun_name =
Expand All @@ -6464,7 +6465,8 @@ Object* JSReceiver::DefineProperty(Isolate* isolate, Handle<Object> object,
}
// 6. Let success be DefinePropertyOrThrow(O,key, desc).
Maybe<bool> success = DefineOwnProperty(
isolate, Handle<JSReceiver>::cast(object), key, &desc, THROW_ON_ERROR);
isolate, Handle<JSReceiver>::cast(object), key, &desc, THROW_ON_ERROR,
call_interceptors);
// 7. ReturnIfAbrupt(success).
MAYBE_RETURN(success, isolate->heap()->exception());
CHECK(success.FromJust());
Expand Down Expand Up @@ -6556,39 +6558,49 @@ Maybe<bool> JSReceiver::DefineOwnProperty(Isolate* isolate,
Handle<JSReceiver> object,
Handle<Object> key,
PropertyDescriptor* desc,
ShouldThrow should_throw) {
ShouldThrow should_throw,
CallInterceptors call_interceptors) {
if (object->IsJSArray()) {
return JSArray::DefineOwnProperty(isolate, Handle<JSArray>::cast(object),
key, desc, should_throw);
key, desc, should_throw, call_interceptors);
}
if (object->IsJSProxy()) {
return JSProxy::DefineOwnProperty(isolate, Handle<JSProxy>::cast(object),
key, desc, should_throw);
key, desc, should_throw, call_interceptors);
}
if (object->IsJSTypedArray()) {
return JSTypedArray::DefineOwnProperty(
isolate, Handle<JSTypedArray>::cast(object), key, desc, should_throw);
isolate, Handle<JSTypedArray>::cast(object), key, desc, should_throw,
call_interceptors);
}
// TODO(neis): Special case for JSModuleNamespace?

// OrdinaryDefineOwnProperty, by virtue of calling
// DefineOwnPropertyIgnoreAttributes, can handle arguments
// (ES#sec-arguments-exotic-objects-defineownproperty-p-desc).
return OrdinaryDefineOwnProperty(isolate, Handle<JSObject>::cast(object), key,
desc, should_throw);
desc, should_throw, call_interceptors);
}


// static
Maybe<bool> JSReceiver::OrdinaryDefineOwnProperty(Isolate* isolate,
Handle<JSObject> object,
Handle<Object> key,
PropertyDescriptor* desc,
ShouldThrow should_throw) {
Handle<JSObject> object,
Handle<Object> key,
PropertyDescriptor* desc,
ShouldThrow should_throw,
CallInterceptors call_interceptors) {
bool success = false;
DCHECK(key->IsName() || key->IsNumber()); // |key| is a PropertyKey...

LookupIterator::Configuration iterator_config = LookupIterator::OWN;
if (call_interceptors == CallInterceptors::SKIP_INTERCEPTORS) {
iterator_config = LookupIterator::OWN_SKIP_INTERCEPTOR;
}

LookupIterator it = LookupIterator::PropertyOrElement(
isolate, object, key, &success, LookupIterator::OWN);
isolate, object, key, &success, iterator_config);

DCHECK(success); // ...so creating a LookupIterator can't fail.

// Deal with access checks first.
Expand Down Expand Up @@ -6985,13 +6997,14 @@ bool PropertyKeyToArrayIndex(Handle<Object> index_obj, uint32_t* output) {
Maybe<bool> JSArray::DefineOwnProperty(Isolate* isolate, Handle<JSArray> o,
Handle<Object> name,
PropertyDescriptor* desc,
ShouldThrow should_throw) {
ShouldThrow should_throw,
CallInterceptors call_interceptors) {
// 1. Assert: IsPropertyKey(P) is true. ("P" is |name|.)
// 2. If P is "length", then:
// TODO(jkummerow): Check if we need slow string comparison.
if (*name == isolate->heap()->length_string()) {
// 2a. Return ArraySetLength(A, Desc).
return ArraySetLength(isolate, o, desc, should_throw);
return ArraySetLength(isolate, o, desc, should_throw, call_interceptors);
}
// 3. Else if P is an array index, then:
uint32_t index = 0;
Expand All @@ -7018,7 +7031,8 @@ Maybe<bool> JSArray::DefineOwnProperty(Isolate* isolate, Handle<JSArray> o,
}
// 3g. Let succeeded be OrdinaryDefineOwnProperty(A, P, Desc).
Maybe<bool> succeeded =
OrdinaryDefineOwnProperty(isolate, o, name, desc, should_throw);
OrdinaryDefineOwnProperty(isolate, o, name, desc, should_throw,
call_interceptors);
// 3h. Assert: succeeded is not an abrupt completion.
// In our case, if should_throw == THROW_ON_ERROR, it can be!
// 3i. If succeeded is false, return false.
Expand All @@ -7031,7 +7045,7 @@ Maybe<bool> JSArray::DefineOwnProperty(Isolate* isolate, Handle<JSArray> o,
// OrdinaryDefineOwnProperty(A, "length", oldLenDesc).
succeeded = OrdinaryDefineOwnProperty(isolate, o,
isolate->factory()->length_string(),
&old_len_desc, should_throw);
&old_len_desc, should_throw, call_interceptors);
// 3j iii. Assert: succeeded is true.
DCHECK(succeeded.FromJust());
USE(succeeded);
Expand All @@ -7041,7 +7055,8 @@ Maybe<bool> JSArray::DefineOwnProperty(Isolate* isolate, Handle<JSArray> o,
}

// 4. Return OrdinaryDefineOwnProperty(A, P, Desc).
return OrdinaryDefineOwnProperty(isolate, o, name, desc, should_throw);
return OrdinaryDefineOwnProperty(isolate, o, name, desc, should_throw,
call_interceptors);
}


Expand Down Expand Up @@ -7086,12 +7101,13 @@ bool JSArray::AnythingToArrayLength(Isolate* isolate,
// static
Maybe<bool> JSArray::ArraySetLength(Isolate* isolate, Handle<JSArray> a,
PropertyDescriptor* desc,
ShouldThrow should_throw) {
ShouldThrow should_throw,
CallInterceptors call_interceptors) {
// 1. If the [[Value]] field of Desc is absent, then
if (!desc->has_value()) {
// 1a. Return OrdinaryDefineOwnProperty(A, "length", Desc).
return OrdinaryDefineOwnProperty(
isolate, a, isolate->factory()->length_string(), desc, should_throw);
isolate, a, isolate->factory()->length_string(), desc, should_throw, call_interceptors);
}
// 2. Let newLenDesc be a copy of Desc.
// (Actual copying is not necessary.)
Expand Down Expand Up @@ -7121,7 +7137,8 @@ Maybe<bool> JSArray::ArraySetLength(Isolate* isolate, Handle<JSArray> a,
new_len_desc->set_value(isolate->factory()->NewNumberFromUint(new_len));
return OrdinaryDefineOwnProperty(isolate, a,
isolate->factory()->length_string(),
new_len_desc, should_throw);
new_len_desc, should_throw,
call_interceptors);
}
// 13. If oldLenDesc.[[Writable]] is false, return false.
if (!old_len_desc.writable()) {
Expand Down Expand Up @@ -7150,7 +7167,7 @@ Maybe<bool> JSArray::ArraySetLength(Isolate* isolate, Handle<JSArray> a,
readonly.set_writable(false);
Maybe<bool> success = OrdinaryDefineOwnProperty(
isolate, a, isolate->factory()->length_string(), &readonly,
should_throw);
should_throw, call_interceptors);
DCHECK(success.FromJust());
USE(success);
}
Expand All @@ -7174,7 +7191,8 @@ Maybe<bool> JSArray::ArraySetLength(Isolate* isolate, Handle<JSArray> a,
Maybe<bool> JSProxy::DefineOwnProperty(Isolate* isolate, Handle<JSProxy> proxy,
Handle<Object> key,
PropertyDescriptor* desc,
ShouldThrow should_throw) {
ShouldThrow should_throw,
CallInterceptors call_interceptors) {
STACK_CHECK(isolate, Nothing<bool>());
if (key->IsSymbol() && Handle<Symbol>::cast(key)->IsPrivate()) {
return SetPrivateProperty(isolate, proxy, Handle<Symbol>::cast(key), desc,
Expand Down Expand Up @@ -7204,7 +7222,7 @@ Maybe<bool> JSProxy::DefineOwnProperty(Isolate* isolate, Handle<JSProxy> proxy,
if (trap->IsUndefined(isolate)) {
// 7a. Return target.[[DefineOwnProperty]](P, Desc).
return JSReceiver::DefineOwnProperty(isolate, target, key, desc,
should_throw);
should_throw, call_interceptors);
}
// 8. Let descObj be FromPropertyDescriptor(Desc).
Handle<Object> desc_obj = desc->ToObject(isolate);
Expand Down Expand Up @@ -16728,10 +16746,11 @@ bool CanonicalNumericIndexString(Isolate* isolate, Handle<Object> s,
// ES#sec-integer-indexed-exotic-objects-defineownproperty-p-desc
// static
Maybe<bool> JSTypedArray::DefineOwnProperty(Isolate* isolate,
Handle<JSTypedArray> o,
Handle<Object> key,
PropertyDescriptor* desc,
ShouldThrow should_throw) {
Handle<JSTypedArray> o,
Handle<Object> key,
PropertyDescriptor* desc,
ShouldThrow should_throw,
CallInterceptors call_interceptors) {
// 1. Assert: IsPropertyKey(P) is true.
DCHECK(key->IsName() || key->IsNumber());
// 2. Assert: O is an Object that has a [[ViewedArrayBuffer]] internal slot.
Expand Down Expand Up @@ -16792,7 +16811,8 @@ Maybe<bool> JSTypedArray::DefineOwnProperty(Isolate* isolate,
}
}
// 4. Return ! OrdinaryDefineOwnProperty(O, P, Desc).
return OrdinaryDefineOwnProperty(isolate, o, key, desc, should_throw);
return OrdinaryDefineOwnProperty(isolate, o, key, desc, should_throw,
call_interceptors);
}

ExternalArrayType JSTypedArray::type() {
Expand Down
28 changes: 16 additions & 12 deletions deps/v8/src/objects.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2038,17 +2038,18 @@ class JSReceiver: public HeapObject {
Handle<JSReceiver> object, uint32_t index,
LanguageMode language_mode = SLOPPY);

MUST_USE_RESULT static Object* DefineProperty(Isolate* isolate,
Handle<Object> object,
Handle<Object> name,
Handle<Object> attributes);
MUST_USE_RESULT static Object* DefineProperty(
Isolate* isolate, Handle<Object> object, Handle<Object> name,
Handle<Object> attributes,
CallInterceptors call_interceptors = DONT_SKIP_INTERCEPTORS);
MUST_USE_RESULT static MaybeHandle<Object> DefineProperties(
Isolate* isolate, Handle<Object> object, Handle<Object> properties);

// "virtual" dispatcher to the correct [[DefineOwnProperty]] implementation.
MUST_USE_RESULT static Maybe<bool> DefineOwnProperty(
Isolate* isolate, Handle<JSReceiver> object, Handle<Object> key,
PropertyDescriptor* desc, ShouldThrow should_throw);
PropertyDescriptor* desc, ShouldThrow should_throw,
CallInterceptors call_interceptors = DONT_SKIP_INTERCEPTORS);

// ES6 7.3.4 (when passed DONT_THROW)
MUST_USE_RESULT static Maybe<bool> CreateDataProperty(
Expand All @@ -2057,7 +2058,8 @@ class JSReceiver: public HeapObject {
// ES6 9.1.6.1
MUST_USE_RESULT static Maybe<bool> OrdinaryDefineOwnProperty(
Isolate* isolate, Handle<JSObject> object, Handle<Object> key,
PropertyDescriptor* desc, ShouldThrow should_throw);
PropertyDescriptor* desc, ShouldThrow should_throw,
CallInterceptors call_interceptors = DONT_SKIP_INTERCEPTORS);
MUST_USE_RESULT static Maybe<bool> OrdinaryDefineOwnProperty(
LookupIterator* it, PropertyDescriptor* desc, ShouldThrow should_throw);
// ES6 9.1.6.2
Expand Down Expand Up @@ -6272,7 +6274,8 @@ class JSProxy: public JSReceiver {
// ES6 9.5.6
MUST_USE_RESULT static Maybe<bool> DefineOwnProperty(
Isolate* isolate, Handle<JSProxy> object, Handle<Object> key,
PropertyDescriptor* desc, ShouldThrow should_throw);
PropertyDescriptor* desc, ShouldThrow should_throw,
CallInterceptors call_interceptors = DONT_SKIP_INTERCEPTORS);

// ES6 9.5.7
MUST_USE_RESULT static Maybe<bool> HasProperty(Isolate* isolate,
Expand Down Expand Up @@ -6686,7 +6689,8 @@ class JSTypedArray: public JSArrayBufferView {
// ES6 9.4.5.3
MUST_USE_RESULT static Maybe<bool> DefineOwnProperty(
Isolate* isolate, Handle<JSTypedArray> o, Handle<Object> key,
PropertyDescriptor* desc, ShouldThrow should_throw);
PropertyDescriptor* desc, ShouldThrow should_throw,
CallInterceptors call_interceptors = DONT_SKIP_INTERCEPTORS);

DECL_CAST(JSTypedArray)

Expand Down Expand Up @@ -6816,15 +6820,15 @@ class JSArray: public JSObject {
// ES6 9.4.2.1
MUST_USE_RESULT static Maybe<bool> DefineOwnProperty(
Isolate* isolate, Handle<JSArray> o, Handle<Object> name,
PropertyDescriptor* desc, ShouldThrow should_throw);
PropertyDescriptor* desc, ShouldThrow should_throw,
CallInterceptors call_interceptors = DONT_SKIP_INTERCEPTORS);

static bool AnythingToArrayLength(Isolate* isolate,
Handle<Object> length_object,
uint32_t* output);
MUST_USE_RESULT static Maybe<bool> ArraySetLength(Isolate* isolate,
Handle<JSArray> a,
PropertyDescriptor* desc,
ShouldThrow should_throw);
Handle<JSArray> a, PropertyDescriptor* desc, ShouldThrow should_throw,
CallInterceptors call_interceptors = DONT_SKIP_INTERCEPTORS);

// Checks whether the Array has the current realm's Array.prototype as its
// prototype. This function is best-effort and only gives a conservative
Expand Down
Loading