Skip to content

V
V8
Commit bdfcc613 authored by machenbach's avatar machenbach Committed by Commit bot
Browse files
Options

Revert of [es7] refactor and fix Object.values() / Object.entries() (patchset... 

Revert of [es7] refactor and fix Object.values() / Object.entries() (patchset #6 id:100001 of https://codereview.chromium.org/1637753004/ )

Reason for revert:
[Sheriff] Breaks gc stress:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20gc%20stress/builds/1642

Original issue's description:
> [es7] refactor and fix Object.values() / Object.entries()
>
> Previously, Object.values() and Object.entries() were piggy-backing on
> Object.keys(). This meant that they would pre-filter non-enumerable properties,
> violating the runtime behaviour of the methods. Unfortunately, this does not
> match the current proposal text.
>
> Also incorporates several tests verifying this behaviour based on tests included
> in the ChakraCore implementation.
>
> BUG=v8:4663
> LOG=N
> R=adamk@chromium.org, rossberg@chromium.org, littledan@chromium.org
>
> Committed: https://crrev.com/5c5ccd9d7f8693990d1a9eb26ba3a94f376dcf0b
> Cr-Commit-Position: refs/heads/master@{#33782}

TBR=littledan@chromium.org,adamk@chromium.org,cbruni@chromium.org,rossberg@chromium.org,caitpotter88@gmail.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4663

Review URL: https://codereview.chromium.org/1675663002

Cr-Commit-Position: refs/heads/master@{#33787}
parent 8b4e1042
Hide whitespace changes
Inline Side-by-side
Showing with 37 additions and 416 deletions
src/builtins.cc
View file @ bdfcc613
......@@ -1777,10 +1777,22 @@ BUILTIN(ObjectValues) {
Handle<JSReceiver> receiver;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, receiver,
Object::ToObject(isolate, object));
Handle<FixedArray> values;
Handle<FixedArray> keys;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, values, JSReceiver::GetOwnValues(receiver, ENUMERABLE_STRINGS));
return *isolate->factory()->NewJSArrayWithElements(values);
isolate, keys, JSReceiver::GetKeys(receiver, OWN_ONLY, ENUMERABLE_STRINGS,
CONVERT_TO_STRING));
for (int i = 0; i < keys->length(); ++i) {
auto key = Handle<Name>::cast(FixedArray::get(*keys, i, isolate));
Handle<Object> value;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, value, Object::GetPropertyOrElement(receiver, key, STRICT));
keys->set(i, *value);
}
return *isolate->factory()->NewJSArrayWithElements(keys);
}
......@@ -1790,11 +1802,26 @@ BUILTIN(ObjectEntries) {
Handle<JSReceiver> receiver;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, receiver,
Object::ToObject(isolate, object));
Handle<FixedArray> entries;
Handle<FixedArray> keys;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, entries,
JSReceiver::GetOwnEntries(receiver, ENUMERABLE_STRINGS));
return *isolate->factory()->NewJSArrayWithElements(entries);
isolate, keys, JSReceiver::GetKeys(receiver, OWN_ONLY, ENUMERABLE_STRINGS,
CONVERT_TO_STRING));
for (int i = 0; i < keys->length(); ++i) {
auto key = Handle<Name>::cast(FixedArray::get(*keys, i, isolate));
Handle<Object> value;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, value, Object::GetPropertyOrElement(receiver, key, STRICT));
auto entry_storage = isolate->factory()->NewUninitializedFixedArray(2);
entry_storage->set(0, *key);
entry_storage->set(1, *value);
auto entry = isolate->factory()->NewJSArrayWithElements(entry_storage);
keys->set(i, *entry);
}
return *isolate->factory()->NewJSArrayWithElements(keys);
}
BUILTIN(ObjectGetOwnPropertyDescriptors) {
......
src/objects-inl.h
View file @ bdfcc613
......@@ -2375,8 +2375,7 @@ void FixedArray::set(int index, Smi* value) {
void FixedArray::set(int index, Object* value) {
DCHECK_NE(GetHeap()->fixed_cow_array_map(), map());
DCHECK(IsFixedArray());
DCHECK_GE(index, 0);
DCHECK_LT(index, this->length());
DCHECK(index >= 0 && index < this->length());
int offset = kHeaderSize + index * kPointerSize;
WRITE_FIELD(this, offset, value);
WRITE_BARRIER(GetHeap(), this, offset, value);
......
src/objects.cc
View file @ bdfcc613
......@@ -8944,64 +8944,6 @@ MaybeHandle<FixedArray> JSReceiver::GetKeys(Handle<JSReceiver> object,
return keys;
}
MaybeHandle<FixedArray> GetOwnValuesOrEntries(Isolate* isolate,
Handle<JSReceiver> object,
PropertyFilter filter,
bool get_entries) {
PropertyFilter key_filter =
static_cast<PropertyFilter>(filter & ~ONLY_ENUMERABLE);
KeyAccumulator accumulator(isolate, OWN_ONLY, key_filter);
MAYBE_RETURN(GetKeys_Internal(isolate, object, object, OWN_ONLY, key_filter,
&accumulator),
MaybeHandle<FixedArray>());
Handle<FixedArray> keys = accumulator.GetKeys(CONVERT_TO_STRING);
DCHECK(ContainsOnlyValidKeys(keys));
Handle<FixedArray> values_or_entries =
isolate->factory()->NewUninitializedFixedArray(keys->length());
int length = 0;
for (int i = 0; i < keys->length(); ++i) {
Handle<Name> key = Handle<Name>::cast(handle(keys->get(i), isolate));
if (filter & ONLY_ENUMERABLE) {
PropertyDescriptor descriptor;
Maybe<bool> did_get_descriptor = JSReceiver::GetOwnPropertyDescriptor(
isolate, object, key, &descriptor);
MAYBE_RETURN(did_get_descriptor, MaybeHandle<FixedArray>());
if (!did_get_descriptor.FromJust() || !descriptor.enumerable()) continue;
}
Handle<Object> value;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate, value, JSReceiver::GetPropertyOrElement(object, key, STRICT),
MaybeHandle<FixedArray>());
if (get_entries) {
Handle<FixedArray> entry_storage =
isolate->factory()->NewUninitializedFixedArray(2);
entry_storage->set(0, *key);
entry_storage->set(1, *value);
value = isolate->factory()->NewJSArrayWithElements(entry_storage,
FAST_ELEMENTS, 2);
}
values_or_entries->set(length, *value);
length++;
}
if (length < values_or_entries->length()) values_or_entries->Shrink(length);
return values_or_entries;
}
MaybeHandle<FixedArray> JSReceiver::GetOwnValues(Handle<JSReceiver> object,
PropertyFilter filter) {
return GetOwnValuesOrEntries(object->GetIsolate(), object, filter, false);
}
MaybeHandle<FixedArray> JSReceiver::GetOwnEntries(Handle<JSReceiver> object,
PropertyFilter filter) {
return GetOwnValuesOrEntries(object->GetIsolate(), object, filter, true);
}
bool Map::DictionaryElementsInPrototypeChainOnly() {
if (IsDictionaryElementsKind(elements_kind())) {
......
src/objects.h
View file @ bdfcc613
......@@ -1959,12 +1959,6 @@ class JSReceiver: public HeapObject {
Handle<JSReceiver> object, KeyCollectionType type, PropertyFilter filter,
GetKeysConversion keys_conversion = KEEP_NUMBERS);
MUST_USE_RESULT static MaybeHandle<FixedArray> GetOwnValues(
Handle<JSReceiver> object, PropertyFilter filter);
MUST_USE_RESULT static MaybeHandle<FixedArray> GetOwnEntries(
Handle<JSReceiver> object, PropertyFilter filter);
// Layout description.
static const int kPropertiesOffset = HeapObject::kHeaderSize;
static const int kHeaderSize = HeapObject::kHeaderSize + kPointerSize;
......
test/mjsunit/harmony/object-entries.js
View file @ bdfcc613
......@@ -3,19 +3,10 @@
// found in the LICENSE file.
// Flags: --harmony-object-values-entries --harmony-proxies --harmony-reflect
// Flags: --allow-natives-syntax
function TestMeta() {
assertEquals(1, Object.entries.length);
assertEquals(Function.prototype, Object.getPrototypeOf(Object.entries));
assertEquals("entries", Object.entries.name);
var descriptor = Object.getOwnPropertyDescriptor(Object, "entries");
assertTrue(descriptor.writable);
assertFalse(descriptor.enumerable);
assertTrue(descriptor.configurable);
assertThrows(() => new Object.entries({}), TypeError);
}
TestMeta();
......@@ -45,21 +36,10 @@ function TestBasic() {
["b", 4]
], Object.entries(O));
assertEquals(Object.entries(O), Object.keys(O).map(key => [key, O[key]]));
assertTrue(Array.isArray(Object.entries({})));
assertEquals(0, Object.entries({}).length);
}
TestBasic();
function TestToObject() {
assertThrows(function() { Object.entries(); }, TypeError);
assertThrows(function() { Object.entries(null); }, TypeError);
assertThrows(function() { Object.entries(void 0); }, TypeError);
}
TestToObject();
function TestOrder() {
var O = {
a: 1,
......@@ -67,8 +47,6 @@ function TestOrder() {
};
O[456] = 123;
Object.defineProperty(O, "HIDDEN", { enumerable: false, value: NaN });
var priv = %CreatePrivateSymbol("Secret");
O[priv] = 56;
var log = [];
var P = new Proxy(O, {
......@@ -80,10 +58,6 @@ function TestOrder() {
log.push(`[[Get]](${JSON.stringify(name)})`);
return Reflect.get(target, name);
},
getOwnPropertyDescriptor(target, name) {
log.push(`[[GetOwnProperty]](${JSON.stringify(name)})`);
return Reflect.getOwnPropertyDescriptor(target, name);
},
set(target, name, value) {
assertUnreachable();
}
......@@ -92,158 +66,8 @@ function TestOrder() {
assertEquals([["456", 123], ["a", 1]], Object.entries(P));
assertEquals([
"[[OwnPropertyKeys]]",
"[[GetOwnProperty]](\"456\")",
"[[Get]](\"456\")",
"[[GetOwnProperty]](\"a\")",
"[[Get]](\"a\")",
"[[GetOwnProperty]](\"HIDDEN\")"
"[[Get]](\"a\")"
], log);
}
TestOrder();
function TestOrderWithDuplicates() {
var O = {
a: 1,
[Symbol.iterator]: null
};
O[456] = 123;
Object.defineProperty(O, "HIDDEN", { enumerable: false, value: NaN });
var priv = %CreatePrivateSymbol("Secret");
O[priv] = 56;
var log = [];
var P = new Proxy(O, {
ownKeys(target) {
log.push("[[OwnPropertyKeys]]");
return ["a", Symbol.iterator, "a", "456", "HIDDEN", "HIDDEN", "456"];
},
get(target, name) {
log.push(`[[Get]](${JSON.stringify(name)})`);
return Reflect.get(target, name);
},
getOwnPropertyDescriptor(target, name) {
log.push(`[[GetOwnProperty]](${JSON.stringify(name)})`);
return Reflect.getOwnPropertyDescriptor(target, name);
},
set(target, name, value) {
assertUnreachable();
}
});
assertEquals([
["a", 1],
["a", 1],
["456", 123],
["456", 123]
], Object.entries(P));
assertEquals([
"[[OwnPropertyKeys]]",
"[[GetOwnProperty]](\"a\")",
"[[Get]](\"a\")",
"[[GetOwnProperty]](\"a\")",
"[[Get]](\"a\")",
"[[GetOwnProperty]](\"456\")",
"[[Get]](\"456\")",
"[[GetOwnProperty]](\"HIDDEN\")",
"[[GetOwnProperty]](\"HIDDEN\")",
"[[GetOwnProperty]](\"456\")",
"[[Get]](\"456\")"
], log);
}
TestOrderWithDuplicates();
function TestPropertyFilter() {
var object = { prop3: 30 };
object[2] = 40;
object["prop4"] = 50;
Object.defineProperty(object, "prop5", { value: 60, enumerable: true });
Object.defineProperty(object, "prop6", { value: 70, enumerable: false });
Object.defineProperty(object, "prop7", {
enumerable: true, get() { return 80; }});
var sym = Symbol("prop8");
object[sym] = 90;
values = Object.entries(object);
assertEquals(5, values.length);
assertEquals([
[ "2", 40 ],
[ "prop3", 30 ],
[ "prop4", 50 ],
[ "prop5", 60 ],
[ "prop7", 80 ]
], values);
}
TestPropertyFilter();
function TestWithProxy() {
var obj1 = {prop1:10};
var proxy1 = new Proxy(obj1, { });
assertEquals([ [ "prop1", 10 ] ], Object.entries(proxy1));
var obj2 = {};
Object.defineProperty(obj2, "prop2", { value: 20, enumerable: true });
Object.defineProperty(obj2, "prop3", {
get() { return 30; }, enumerable: true });
var proxy2 = new Proxy(obj2, {
getOwnPropertyDescriptor(target, name) {
return Reflect.getOwnPropertyDescriptor(target, name);
}
});
assertEquals([ [ "prop2", 20 ], [ "prop3", 30 ] ], Object.entries(proxy2));
var obj3 = {};
var count = 0;
var proxy3 = new Proxy(obj3, {
get(target, property, receiver) {
return count++ * 5;
},
getOwnPropertyDescriptor(target, property) {
return { configurable: true, enumerable: true };
},
ownKeys(target) {
return [ "prop0", "prop1", Symbol("prop2"), Symbol("prop5") ];
}
});
assertEquals([ [ "prop0", 0 ], [ "prop1", 5 ] ], Object.entries(proxy3));
}
TestWithProxy();
function TestMutateDuringEnumeration() {
var aDeletesB = {
get a() {
delete this.b;
return 1;
},
b: 2
};
assertEquals([ [ "a", 1 ] ], Object.entries(aDeletesB));
var aRemovesB = {
get a() {
Object.defineProperty(this, "b", { enumerable: false });
return 1;
},
b: 2
};
assertEquals([ [ "a", 1 ] ], Object.entries(aRemovesB));
var aAddsB = { get a() { this.b = 2; return 1; } };
assertEquals([ [ "a", 1 ] ], Object.entries(aAddsB));
var aMakesBEnumerable = {};
Object.defineProperty(aMakesBEnumerable, "a", {
get() {
Object.defineProperty(this, "b", { enumerable: true });
return 1;
},
enumerable: true
});
Object.defineProperty(aMakesBEnumerable, "b", {
value: 2, configurable:true, enumerable: false });
assertEquals([ [ "a", 1 ], [ "b", 2 ] ], Object.entries(aMakesBEnumerable));
}
TestMutateDuringEnumeration();
test/mjsunit/harmony/object-values.js
View file @ bdfcc613
......@@ -3,19 +3,10 @@
// found in the LICENSE file.
// Flags: --harmony-object-values-entries --harmony-proxies --harmony-reflect
// Flags: --allow-natives-syntax
function TestMeta() {
assertEquals(1, Object.values.length);
assertEquals(Function.prototype, Object.getPrototypeOf(Object.values));
assertEquals("values", Object.values.name);
var descriptor = Object.getOwnPropertyDescriptor(Object, "values");
assertTrue(descriptor.writable);
assertFalse(descriptor.enumerable);
assertTrue(descriptor.configurable);
assertThrows(() => new Object.values({}), TypeError);
}
TestMeta();
......@@ -36,21 +27,10 @@ function TestBasic() {
Object.defineProperty(O, "HIDDEN", { enumerable: false, value: NaN });
assertEquals([123, "ducks", 456, 1, 3, "quack", 2, 4], Object.values(O));
assertEquals(Object.values(O), Object.keys(O).map(key => O[key]));
assertTrue(Array.isArray(Object.values({})));
assertEquals(0, Object.values({}).length);
}
TestBasic();
function TestToObject() {
assertThrows(function() { Object.values(); }, TypeError);
assertThrows(function() { Object.values(null); }, TypeError);
assertThrows(function() { Object.values(void 0); }, TypeError);
}
TestToObject();
function TestOrder() {
var O = {
a: 1,
......@@ -58,8 +38,6 @@ function TestOrder() {
};
O[456] = 123;
Object.defineProperty(O, "HIDDEN", { enumerable: false, value: NaN });
var priv = %CreatePrivateSymbol("Secret");
O[priv] = 56;
var log = [];
var P = new Proxy(O, {
......@@ -71,10 +49,6 @@ function TestOrder() {
log.push(`[[Get]](${JSON.stringify(name)})`);
return Reflect.get(target, name);
},
getOwnPropertyDescriptor(target, name) {
log.push(`[[GetOwnProperty]](${JSON.stringify(name)})`);
return Reflect.getOwnPropertyDescriptor(target, name);
},
set(target, name, value) {
assertUnreachable();
}
......@@ -83,147 +57,8 @@ function TestOrder() {
assertEquals([123, 1], Object.values(P));
assertEquals([
"[[OwnPropertyKeys]]",
"[[GetOwnProperty]](\"456\")",
"[[Get]](\"456\")",
"[[GetOwnProperty]](\"a\")",
"[[Get]](\"a\")",
"[[GetOwnProperty]](\"HIDDEN\")"
"[[Get]](\"a\")"
], log);
}
TestOrder();
function TestOrderWithDuplicates() {
var O = {
a: 1,
[Symbol.iterator]: null
};
O[456] = 123;
Object.defineProperty(O, "HIDDEN", { enumerable: false, value: NaN });
O[priv] = 56;
var priv = %CreatePrivateSymbol("private");
var log = [];
var P = new Proxy(O, {
ownKeys(target) {
log.push("[[OwnPropertyKeys]]");
return [ "a", Symbol.iterator, "a", "456", "HIDDEN", "HIDDEN", "456" ];
},
get(target, name) {
log.push(`[[Get]](${JSON.stringify(name)})`);
return Reflect.get(target, name);
},
getOwnPropertyDescriptor(target, name) {
log.push(`[[GetOwnProperty]](${JSON.stringify(name)})`);
return Reflect.getOwnPropertyDescriptor(target, name);
},
set(target, name, value) {
assertUnreachable();
}
});
assertEquals([1, 1, 123, 123], Object.values(P));
assertEquals([
"[[OwnPropertyKeys]]",
"[[GetOwnProperty]](\"a\")",
"[[Get]](\"a\")",
"[[GetOwnProperty]](\"a\")",
"[[Get]](\"a\")",
"[[GetOwnProperty]](\"456\")",
"[[Get]](\"456\")",
"[[GetOwnProperty]](\"HIDDEN\")",
"[[GetOwnProperty]](\"HIDDEN\")",
"[[GetOwnProperty]](\"456\")",
"[[Get]](\"456\")",
], log);
}
TestOrderWithDuplicates();
function TestPropertyFilter() {
var object = { prop3: 30 };
object[2] = 40;
object["prop4"] = 50;
Object.defineProperty(object, "prop5", { value: 60, enumerable: true });
Object.defineProperty(object, "prop6", { value: 70, enumerable: false });
Object.defineProperty(object, "prop7", {
enumerable: true, get() { return 80; }});
var sym = Symbol("prop8");
object[sym] = 90;
values = Object.values(object);
assertEquals(5, values.length);
assertEquals([40,30,50,60,80], values);
}
TestPropertyFilter();
function TestWithProxy() {
var obj1 = {prop1:10};
var proxy1 = new Proxy(obj1, { });
assertEquals([10], Object.values(proxy1));
var obj2 = {};
Object.defineProperty(obj2, "prop2", { value: 20, enumerable: true });
Object.defineProperty(obj2, "prop3", {
get() { return 30; }, enumerable: true });
var proxy2 = new Proxy(obj2, {
getOwnPropertyDescriptor(target, name) {
return Reflect.getOwnPropertyDescriptor(target, name);
}
});
assertEquals([20, 30], Object.values(proxy2));
var obj3 = {};
var count = 0;
var proxy3 = new Proxy(obj3, {
get(target, property, receiver) {
return count++ * 5;
},
getOwnPropertyDescriptor(target, property) {
return { configurable: true, enumerable: true };
},
ownKeys(target) {
return [ "prop0", "prop1", Symbol("prop2"), Symbol("prop5") ];
}
});
assertEquals([0, 5], Object.values(proxy3));
}
TestWithProxy();
function TestMutateDuringEnumeration() {
var aDeletesB = {
get a() {
delete this.b;
return 1;
},
b: 2
};
assertEquals([1], Object.values(aDeletesB));
var aRemovesB = {
get a() {
Object.defineProperty(this, "b", { enumerable: false });
return 1;
},
b: 2
};
assertEquals([1], Object.values(aRemovesB));
var aAddsB = { get a() { this.b = 2; return 1; } };
assertEquals([1], Object.values(aAddsB));
var aMakesBEnumerable = {};
Object.defineProperty(aMakesBEnumerable, "a", {
get() {
Object.defineProperty(this, "b", { enumerable: true });
return 1;
},
enumerable: true
});
Object.defineProperty(aMakesBEnumerable, "b", {
value: 2, configurable:true, enumerable: false });
assertEquals([1, 2], Object.values(aMakesBEnumerable));
}
TestMutateDuringEnumeration();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment