Skip to content

Commit d5335cb

Browse files
verwaestCommit bot
verwaest
authored and
Commit bot
committed
Avoid converting key to string for deleting of elements
Additionally rips out (now) unnecessary duplicate code in DefineArrayProperty. BUG= Review URL: https://codereview.chromium.org/1224523002 Cr-Commit-Position: refs/heads/master@{#29450}
1 parent dbda22f commit d5335cb

11 files changed

Lines changed: 80 additions & 94 deletions

File tree

‎src/api.cc‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3607,25 +3607,6 @@ bool v8::Object::ForceSet(v8::Handle<Value> key, v8::Handle<Value> value,
36073607
}
36083608

36093609

3610-
MUST_USE_RESULT
3611-
static i::MaybeHandle<i::Object> DeleteObjectProperty(
3612-
i::Isolate* isolate, i::Handle<i::JSReceiver> receiver,
3613-
i::Handle<i::Object> key, i::LanguageMode language_mode) {
3614-
// Check if the given key is an array index.
3615-
uint32_t index = 0;
3616-
if (key->ToArrayIndex(&index)) {
3617-
return i::JSReceiver::DeleteElement(receiver, index, language_mode);
3618-
}
3619-
3620-
i::Handle<i::Name> name;
3621-
ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, name,
3622-
i::Runtime::ToName(isolate, key),
3623-
i::MaybeHandle<i::Object>());
3624-
3625-
return i::JSReceiver::DeletePropertyOrElement(receiver, name, language_mode);
3626-
}
3627-
3628-
36293610
MaybeLocal<Value> v8::Object::Get(Local<v8::Context> context,
36303611
Local<Value> key) {
36313612
PREPARE_FOR_EXECUTION(context, "v8::Object::Get()", Value);
@@ -3883,7 +3864,8 @@ Maybe<bool> v8::Object::Delete(Local<Context> context, Local<Value> key) {
38833864
auto key_obj = Utils::OpenHandle(*key);
38843865
i::Handle<i::Object> obj;
38853866
has_pending_exception =
3886-
!DeleteObjectProperty(isolate, self, key_obj, i::SLOPPY).ToHandle(&obj);
3867+
!i::Runtime::DeleteObjectProperty(isolate, self, key_obj, i::SLOPPY)
3868+
.ToHandle(&obj);
38873869
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
38883870
return Just(obj->IsTrue());
38893871
}

‎src/array.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ function ArrayPop() {
478478

479479
n--;
480480
var value = array[n];
481-
Delete(array, $toName(n), true);
481+
Delete(array, n, true);
482482
array.length = n;
483483
return value;
484484
}

‎src/messages.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,6 @@ class CallSite {
259259
"In strong mode, using an undeclared global variable '%' is not allowed") \
260260
T(UnsupportedSuper, "Unsupported reference to 'super'") \
261261
/* RangeError */ \
262-
T(ArrayLengthOutOfRange, "defineProperty() array length out of range") \
263262
T(DateRange, "Provided date is not in valid range.") \
264263
T(ExpectedLocation, "Expected Area/Location for time zone, got %") \
265264
T(InvalidArrayBufferLength, "Invalid array buffer length") \

‎src/objects.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5276,7 +5276,8 @@ MaybeHandle<Object> JSReceiver::DeleteProperty(LookupIterator* it,
52765276
MaybeHandle<Object> JSReceiver::DeleteElement(Handle<JSReceiver> object,
52775277
uint32_t index,
52785278
LanguageMode language_mode) {
5279-
LookupIterator it(object->GetIsolate(), object, index);
5279+
LookupIterator it(object->GetIsolate(), object, index,
5280+
LookupIterator::HIDDEN);
52805281
return DeleteProperty(&it, language_mode);
52815282
}
52825283

‎src/runtime.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ SHR_STRONG = function SHR_STRONG(y) {
509509

510510
// ECMA-262, section 11.4.1, page 46.
511511
DELETE = function DELETE(key, language_mode) {
512-
return %DeleteProperty(%$toObject(this), %$toName(key), language_mode);
512+
return %DeleteProperty(%$toObject(this), key, language_mode);
513513
}
514514

515515

‎src/runtime/runtime-object.cc‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,23 @@ MaybeHandle<Object> Runtime::KeyedGetObjectProperty(
162162
}
163163

164164

165+
MaybeHandle<Object> Runtime::DeleteObjectProperty(Isolate* isolate,
166+
Handle<JSReceiver> receiver,
167+
Handle<Object> key,
168+
LanguageMode language_mode) {
169+
// Check if the given key is an array index.
170+
uint32_t index = 0;
171+
if (key->ToArrayIndex(&index)) {
172+
return JSReceiver::DeleteElement(receiver, index, language_mode);
173+
}
174+
175+
Handle<Name> name;
176+
ASSIGN_RETURN_ON_EXCEPTION(isolate, name, ToName(isolate, key), Object);
177+
178+
return JSReceiver::DeletePropertyOrElement(receiver, name, language_mode);
179+
}
180+
181+
165182
MaybeHandle<Object> Runtime::SetObjectProperty(Isolate* isolate,
166183
Handle<Object> object,
167184
Handle<Object> key,
@@ -591,13 +608,13 @@ RUNTIME_FUNCTION(Runtime_SetProperty) {
591608
RUNTIME_FUNCTION(Runtime_DeleteProperty) {
592609
HandleScope scope(isolate);
593610
DCHECK(args.length() == 3);
594-
CONVERT_ARG_HANDLE_CHECKED(JSReceiver, object, 0);
595-
CONVERT_ARG_HANDLE_CHECKED(Name, key, 1);
611+
CONVERT_ARG_HANDLE_CHECKED(JSReceiver, receiver, 0);
612+
CONVERT_ARG_HANDLE_CHECKED(Object, key, 1);
596613
CONVERT_LANGUAGE_MODE_ARG_CHECKED(language_mode, 2);
597614
Handle<Object> result;
598615
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
599616
isolate, result,
600-
JSReceiver::DeletePropertyOrElement(object, key, language_mode));
617+
Runtime::DeleteObjectProperty(isolate, receiver, key, language_mode));
601618
return *result;
602619
}
603620

‎src/runtime/runtime.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,10 @@ class Runtime : public AllStatic {
822822
Isolate* isolate, Handle<Object> object, uint32_t index,
823823
LanguageMode language_mode = SLOPPY);
824824

825+
MUST_USE_RESULT static MaybeHandle<Object> DeleteObjectProperty(
826+
Isolate* isolate, Handle<JSReceiver> receiver, Handle<Object> key,
827+
LanguageMode language_mode);
828+
825829
MUST_USE_RESULT static MaybeHandle<Object> SetObjectProperty(
826830
Isolate* isolate, Handle<Object> object, Handle<Object> key,
827831
Handle<Object> value, LanguageMode language_mode);

‎src/v8natives.js‎

Lines changed: 1 addition & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -802,72 +802,7 @@ function DefineObjectProperty(obj, p, desc, should_throw) {
802802

803803
// ES5 section 15.4.5.1.
804804
function DefineArrayProperty(obj, p, desc, should_throw) {
805-
// Note that the length of an array is not actually stored as part of the
806-
// property, hence we use generated code throughout this function instead of
807-
// DefineObjectProperty() to modify its value.
808-
809-
// Step 3 - Special handling for length property.
810-
if (p === "length") {
811-
var length = obj.length;
812-
var old_length = length;
813-
if (!desc.hasValue()) {
814-
return DefineObjectProperty(obj, "length", desc, should_throw);
815-
}
816-
var new_length = $toUint32(desc.getValue());
817-
if (new_length != $toNumber(desc.getValue())) {
818-
throw MakeRangeError(kArrayLengthOutOfRange);
819-
}
820-
var length_desc = GetOwnPropertyJS(obj, "length");
821-
if (new_length != length && !length_desc.isWritable()) {
822-
if (should_throw) {
823-
throw MakeTypeError(kRedefineDisallowed, p);
824-
} else {
825-
return false;
826-
}
827-
}
828-
var threw = false;
829-
830-
var emit_splice = %IsObserved(obj) && new_length !== old_length;
831-
var removed;
832-
if (emit_splice) {
833-
$observeBeginPerformSplice(obj);
834-
removed = [];
835-
if (new_length < old_length)
836-
removed.length = old_length - new_length;
837-
}
838-
839-
while (new_length < length--) {
840-
var index = $toString(length);
841-
if (emit_splice) {
842-
var deletedDesc = GetOwnPropertyJS(obj, index);
843-
if (deletedDesc && deletedDesc.hasValue())
844-
removed[length - new_length] = deletedDesc.getValue();
845-
}
846-
if (!Delete(obj, index, false)) {
847-
new_length = length + 1;
848-
threw = true;
849-
break;
850-
}
851-
}
852-
threw = !DefineObjectProperty(obj, "length", desc, should_throw) || threw;
853-
if (emit_splice) {
854-
$observeEndPerformSplice(obj);
855-
$observeEnqueueSpliceRecord(obj,
856-
new_length < old_length ? new_length : old_length,
857-
removed,
858-
new_length > old_length ? new_length - old_length : 0);
859-
}
860-
if (threw) {
861-
if (should_throw) {
862-
throw MakeTypeError(kRedefineDisallowed, p);
863-
} else {
864-
return false;
865-
}
866-
}
867-
return true;
868-
}
869-
870-
// Step 4 - Special handling for array index.
805+
// Step 3 - Special handling for array index.
871806
if (!IS_SYMBOL(p)) {
872807
var index = $toUint32(p);
873808
var emit_splice = false;

‎test/mjsunit/messages.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ test(function() {
405405
test(function() {
406406
"use strict";
407407
Object.defineProperty([], "length", { value: 1E100 });
408-
}, "defineProperty() array length out of range", RangeError);
408+
}, "Invalid array length", RangeError);
409409

410410
// kInvalidArrayBufferLength
411411
test(function() {

‎test/test262-es6/test262-es6.status‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,30 @@
3333
'intl402/11.2.3_b': [FAIL],
3434
'intl402/12.2.3_b': [FAIL],
3535

36+
# BUG(v8:4267)
37+
'built-ins/Object/defineProperty/15.2.3.6-4-116': [FAIL],
38+
'built-ins/Object/defineProperty/15.2.3.6-4-117': [FAIL],
39+
'built-ins/Object/defineProperty/15.2.3.6-4-168': [FAIL],
40+
'built-ins/Object/defineProperty/15.2.3.6-4-169': [FAIL],
41+
'built-ins/Object/defineProperty/15.2.3.6-4-170': [FAIL],
42+
'built-ins/Object/defineProperty/15.2.3.6-4-172': [FAIL],
43+
'built-ins/Object/defineProperty/15.2.3.6-4-173': [FAIL],
44+
'built-ins/Object/defineProperty/15.2.3.6-4-174': [FAIL],
45+
'built-ins/Object/defineProperty/15.2.3.6-4-176': [FAIL],
46+
'built-ins/Object/defineProperty/15.2.3.6-4-177': [FAIL],
47+
'built-ins/Object/defineProperties/15.2.3.7-6-a-112': [FAIL],
48+
'built-ins/Object/defineProperties/15.2.3.7-6-a-113': [FAIL],
49+
'built-ins/Object/defineProperties/15.2.3.7-6-a-164': [FAIL],
50+
'built-ins/Object/defineProperties/15.2.3.7-6-a-165': [FAIL],
51+
'built-ins/Object/defineProperties/15.2.3.7-6-a-166': [FAIL],
52+
'built-ins/Object/defineProperties/15.2.3.7-6-a-168': [FAIL],
53+
'built-ins/Object/defineProperties/15.2.3.7-6-a-169': [FAIL],
54+
'built-ins/Object/defineProperties/15.2.3.7-6-a-170': [FAIL],
55+
'built-ins/Object/defineProperties/15.2.3.7-6-a-172': [FAIL],
56+
'built-ins/Object/defineProperties/15.2.3.7-6-a-173': [FAIL],
57+
'built-ins/Object/defineProperties/15.2.3.7-6-a-175': [FAIL],
58+
'built-ins/Object/defineProperties/15.2.3.7-6-a-176': [FAIL],
59+
3660
# Unicode canonicalization is not available with i18n turned off.
3761
'built-ins/String/prototype/localeCompare/15.5.4.9_CE': [['no_i18n', SKIP]],
3862

0 commit comments

Comments
 (0)