Skip to content

Commit e557647

Browse files
BridgeARBridgeAR
committed
deps: V8: backport 74571c8
Original commit message: Fix preview of set entries Set entries return an array with the value as first and second entry. As such these are considered key value pairs to align with maps entries iterator. So far the return value was identical to the values iterator and that is misleading. This also adds tests to verify the results and improves the coverage a tiny bit by testing different iterators. Refs: #24629 R=yangguo@chromium.org Change-Id: I669a724bb4afaf5a713e468b1f51691d22c25253 Reviewed-on: https://chromium-review.googlesource.com/c/1350790 Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#59311} Refs: v8/v8@74571c8 PR-URL: #25941 Fixes: #24629 Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent 14c089b commit e557647

7 files changed

Lines changed: 279 additions & 30 deletions

File tree

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737

3838
# Reset this number to 0 on major V8 upgrades.
3939
# Increment by one for each non-official patch applied to deps/v8.
40-
'v8_embedder_string': '-node.13',
40+
'v8_embedder_string': '-node.14',
4141

4242
##### V8 defaults for Node.js #####
4343

‎deps/v8/AUTHORS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ Rick Waldron <waldron.rick@gmail.com>
143143
Rob Wu <rob@robwu.nl>
144144
Robert Mustacchi <rm@fingolfin.org>
145145
Robert Nagy <robert.nagy@gmail.com>
146+
Ruben Bridgewater <ruben@bridgewater.de>
146147
Ryan Dahl <ry@tinyclouds.org>
147148
Sakthipriyan Vairamani (thefourtheye) <thechargingvolcano@gmail.com>
148149
Sander Mathijs van Veen <sander@leaningtech.com>

‎deps/v8/src/api.cc‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7028,6 +7028,11 @@ enum class MapAsArrayKind {
70287028
kValues = i::JS_MAP_VALUE_ITERATOR_TYPE
70297029
};
70307030

7031+
enum class SetAsArrayKind {
7032+
kEntries = i::JS_SET_KEY_VALUE_ITERATOR_TYPE,
7033+
kValues = i::JS_SET_VALUE_ITERATOR_TYPE
7034+
};
7035+
70317036
i::Handle<i::JSArray> MapAsArray(i::Isolate* isolate, i::Object* table_obj,
70327037
int offset, MapAsArrayKind kind) {
70337038
i::Factory* factory = isolate->factory();
@@ -7137,13 +7142,14 @@ Maybe<bool> Set::Delete(Local<Context> context, Local<Value> key) {
71377142

71387143
namespace {
71397144
i::Handle<i::JSArray> SetAsArray(i::Isolate* isolate, i::Object* table_obj,
7140-
int offset) {
7145+
int offset, SetAsArrayKind kind) {
71417146
i::Factory* factory = isolate->factory();
71427147
i::Handle<i::OrderedHashSet> table(i::OrderedHashSet::cast(table_obj),
71437148
isolate);
71447149
// Elements skipped by |offset| may already be deleted.
71457150
int capacity = table->UsedCapacity();
7146-
int max_length = capacity - offset;
7151+
const bool collect_key_values = kind == SetAsArrayKind::kEntries;
7152+
int max_length = (capacity - offset) * (collect_key_values ? 2 : 1);
71477153
if (max_length == 0) return factory->NewJSArray(0);
71487154
i::Handle<i::FixedArray> result = factory->NewFixedArray(max_length);
71497155
int result_index = 0;
@@ -7154,6 +7160,7 @@ i::Handle<i::JSArray> SetAsArray(i::Isolate* isolate, i::Object* table_obj,
71547160
i::Object* key = table->KeyAt(i);
71557161
if (key == the_hole) continue;
71567162
result->set(result_index++, key);
7163+
if (collect_key_values) result->set(result_index++, key);
71577164
}
71587165
}
71597166
DCHECK_GE(max_length, result_index);
@@ -7169,7 +7176,8 @@ Local<Array> Set::AsArray() const {
71697176
i::Isolate* isolate = obj->GetIsolate();
71707177
LOG_API(isolate, Set, AsArray);
71717178
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
7172-
return Utils::ToLocal(SetAsArray(isolate, obj->table(), 0));
7179+
return Utils::ToLocal(
7180+
SetAsArray(isolate, obj->table(), 0, SetAsArrayKind::kValues));
71737181
}
71747182

71757183

@@ -9524,21 +9532,22 @@ v8::MaybeLocal<v8::Array> v8::Object::PreviewEntries(bool* is_key_value) {
95249532
i::Handle<i::JSWeakCollection>::cast(object), 0));
95259533
}
95269534
if (object->IsJSMapIterator()) {
9527-
i::Handle<i::JSMapIterator> iterator =
9528-
i::Handle<i::JSMapIterator>::cast(object);
9535+
i::Handle<i::JSMapIterator> it = i::Handle<i::JSMapIterator>::cast(object);
95299536
MapAsArrayKind const kind =
9530-
static_cast<MapAsArrayKind>(iterator->map()->instance_type());
9537+
static_cast<MapAsArrayKind>(it->map()->instance_type());
95319538
*is_key_value = kind == MapAsArrayKind::kEntries;
9532-
if (!iterator->HasMore()) return v8::Array::New(v8_isolate);
9533-
return Utils::ToLocal(MapAsArray(isolate, iterator->table(),
9534-
i::Smi::ToInt(iterator->index()), kind));
9539+
if (!it->HasMore()) return v8::Array::New(v8_isolate);
9540+
return Utils::ToLocal(
9541+
MapAsArray(isolate, it->table(), i::Smi::ToInt(it->index()), kind));
95359542
}
95369543
if (object->IsJSSetIterator()) {
95379544
i::Handle<i::JSSetIterator> it = i::Handle<i::JSSetIterator>::cast(object);
9538-
*is_key_value = false;
9545+
SetAsArrayKind const kind =
9546+
static_cast<SetAsArrayKind>(it->map()->instance_type());
9547+
*is_key_value = kind == SetAsArrayKind::kEntries;
95399548
if (!it->HasMore()) return v8::Array::New(v8_isolate);
95409549
return Utils::ToLocal(
9541-
SetAsArray(isolate, it->table(), i::Smi::ToInt(it->index())));
9550+
SetAsArray(isolate, it->table(), i::Smi::ToInt(it->index()), kind));
95429551
}
95439552
return v8::MaybeLocal<v8::Array>();
95449553
}

‎deps/v8/test/cctest/test-api.cc‎

Lines changed: 228 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28853,7 +28853,7 @@ TEST(TestGetEmbeddedCodeRange) {
2885328853
}
2885428854
}
2885528855

28856-
TEST(PreviewSetIteratorEntriesWithDeleted) {
28856+
TEST(PreviewSetKeysIteratorEntriesWithDeleted) {
2885728857
LocalContext env;
2885828858
v8::HandleScope handle_scope(env->GetIsolate());
2885928859
v8::Local<v8::Context> context = env.local();
@@ -28952,7 +28952,142 @@ TEST(PreviewSetIteratorEntriesWithDeleted) {
2895228952
}
2895328953
}
2895428954

28955-
TEST(PreviewMapIteratorEntriesWithDeleted) {
28955+
TEST(PreviewSetValuesIteratorEntriesWithDeleted) {
28956+
LocalContext env;
28957+
v8::HandleScope handle_scope(env->GetIsolate());
28958+
v8::Local<v8::Context> context = env.local();
28959+
28960+
{
28961+
// Create set, delete entry, create iterator, preview.
28962+
v8::Local<v8::Object> iterator =
28963+
CompileRun("var set = new Set([1,2,3]); set.delete(1); set.values()")
28964+
->ToObject(context)
28965+
.ToLocalChecked();
28966+
bool is_key;
28967+
v8::Local<v8::Array> entries =
28968+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28969+
CHECK(!is_key);
28970+
CHECK_EQ(2, entries->Length());
28971+
CHECK_EQ(2, entries->Get(context, 0)
28972+
.ToLocalChecked()
28973+
->Int32Value(context)
28974+
.FromJust());
28975+
CHECK_EQ(3, entries->Get(context, 1)
28976+
.ToLocalChecked()
28977+
->Int32Value(context)
28978+
.FromJust());
28979+
}
28980+
{
28981+
// Create set, create iterator, delete entry, preview.
28982+
v8::Local<v8::Object> iterator =
28983+
CompileRun("var set = new Set([1,2,3]); set.values()")
28984+
->ToObject(context)
28985+
.ToLocalChecked();
28986+
CompileRun("set.delete(1);");
28987+
bool is_key;
28988+
v8::Local<v8::Array> entries =
28989+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28990+
CHECK(!is_key);
28991+
CHECK_EQ(2, entries->Length());
28992+
CHECK_EQ(2, entries->Get(context, 0)
28993+
.ToLocalChecked()
28994+
->Int32Value(context)
28995+
.FromJust());
28996+
CHECK_EQ(3, entries->Get(context, 1)
28997+
.ToLocalChecked()
28998+
->Int32Value(context)
28999+
.FromJust());
29000+
}
29001+
{
29002+
// Create set, create iterator, delete entry, iterate, preview.
29003+
v8::Local<v8::Object> iterator =
29004+
CompileRun("var set = new Set([1,2,3]); var it = set.values(); it")
29005+
->ToObject(context)
29006+
.ToLocalChecked();
29007+
CompileRun("set.delete(1); it.next();");
29008+
bool is_key;
29009+
v8::Local<v8::Array> entries =
29010+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29011+
CHECK(!is_key);
29012+
CHECK_EQ(1, entries->Length());
29013+
CHECK_EQ(3, entries->Get(context, 0)
29014+
.ToLocalChecked()
29015+
->Int32Value(context)
29016+
.FromJust());
29017+
}
29018+
{
29019+
// Create set, create iterator, delete entry, iterate until empty, preview.
29020+
v8::Local<v8::Object> iterator =
29021+
CompileRun("var set = new Set([1,2,3]); var it = set.values(); it")
29022+
->ToObject(context)
29023+
.ToLocalChecked();
29024+
CompileRun("set.delete(1); it.next(); it.next();");
29025+
bool is_key;
29026+
v8::Local<v8::Array> entries =
29027+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29028+
CHECK(!is_key);
29029+
CHECK_EQ(0, entries->Length());
29030+
}
29031+
{
29032+
// Create set, create iterator, delete entry, iterate, trigger rehash,
29033+
// preview.
29034+
v8::Local<v8::Object> iterator =
29035+
CompileRun("var set = new Set([1,2,3]); var it = set.values(); it")
29036+
->ToObject(context)
29037+
.ToLocalChecked();
29038+
CompileRun("set.delete(1); it.next();");
29039+
CompileRun("for (var i = 4; i < 20; i++) set.add(i);");
29040+
bool is_key;
29041+
v8::Local<v8::Array> entries =
29042+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29043+
CHECK(!is_key);
29044+
CHECK_EQ(17, entries->Length());
29045+
for (uint32_t i = 0; i < 17; i++) {
29046+
CHECK_EQ(i + 3, entries->Get(context, i)
29047+
.ToLocalChecked()
29048+
->Int32Value(context)
29049+
.FromJust());
29050+
}
29051+
}
29052+
}
29053+
29054+
TEST(PreviewMapEntriesIteratorEntries) {
29055+
LocalContext env;
29056+
v8::HandleScope handle_scope(env->GetIsolate());
29057+
v8::Local<v8::Context> context = env.local();
29058+
{
29059+
// Create set, delete entry, create entries iterator, preview.
29060+
v8::Local<v8::Object> iterator =
29061+
CompileRun("var set = new Set([1,2,3]); set.delete(2); set.entries()")
29062+
->ToObject(context)
29063+
.ToLocalChecked();
29064+
bool is_key;
29065+
v8::Local<v8::Array> entries =
29066+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29067+
CHECK(is_key);
29068+
CHECK_EQ(4, entries->Length());
29069+
uint32_t first = entries->Get(context, 0)
29070+
.ToLocalChecked()
29071+
->Int32Value(context)
29072+
.FromJust();
29073+
uint32_t second = entries->Get(context, 2)
29074+
.ToLocalChecked()
29075+
->Int32Value(context)
29076+
.FromJust();
29077+
CHECK_EQ(1, first);
29078+
CHECK_EQ(3, second);
29079+
CHECK_EQ(first, entries->Get(context, 1)
29080+
.ToLocalChecked()
29081+
->Int32Value(context)
29082+
.FromJust());
29083+
CHECK_EQ(second, entries->Get(context, 3)
29084+
.ToLocalChecked()
29085+
->Int32Value(context)
29086+
.FromJust());
29087+
}
29088+
}
29089+
29090+
TEST(PreviewMapValuesIteratorEntriesWithDeleted) {
2895629091
LocalContext env;
2895729092
v8::HandleScope handle_scope(env->GetIsolate());
2895829093
v8::Local<v8::Context> context = env.local();
@@ -29066,3 +29201,94 @@ TEST(PreviewMapIteratorEntriesWithDeleted) {
2906629201
}
2906729202
}
2906829203
}
29204+
29205+
TEST(PreviewMapKeysIteratorEntriesWithDeleted) {
29206+
LocalContext env;
29207+
v8::HandleScope handle_scope(env->GetIsolate());
29208+
v8::Local<v8::Context> context = env.local();
29209+
29210+
{
29211+
// Create map, delete entry, create iterator, preview.
29212+
v8::Local<v8::Object> iterator = CompileRun(
29213+
"var map = new Map();"
29214+
"var key = 1; map.set(key, {});"
29215+
"map.set(2, {}); map.set(3, {});"
29216+
"map.delete(key);"
29217+
"map.keys()")
29218+
->ToObject(context)
29219+
.ToLocalChecked();
29220+
bool is_key;
29221+
v8::Local<v8::Array> entries =
29222+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29223+
CHECK(!is_key);
29224+
CHECK_EQ(2, entries->Length());
29225+
CHECK_EQ(2, entries->Get(context, 0)
29226+
.ToLocalChecked()
29227+
->Int32Value(context)
29228+
.FromJust());
29229+
CHECK_EQ(3, entries->Get(context, 1)
29230+
.ToLocalChecked()
29231+
->Int32Value(context)
29232+
.FromJust());
29233+
}
29234+
{
29235+
// Create map, create iterator, delete entry, preview.
29236+
v8::Local<v8::Object> iterator = CompileRun(
29237+
"var map = new Map();"
29238+
"var key = 1; map.set(key, {});"
29239+
"map.set(2, {}); map.set(3, {});"
29240+
"map.keys()")
29241+
->ToObject(context)
29242+
.ToLocalChecked();
29243+
CompileRun("map.delete(key);");
29244+
bool is_key;
29245+
v8::Local<v8::Array> entries =
29246+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29247+
CHECK(!is_key);
29248+
CHECK_EQ(2, entries->Length());
29249+
CHECK_EQ(2, entries->Get(context, 0)
29250+
.ToLocalChecked()
29251+
->Int32Value(context)
29252+
.FromJust());
29253+
CHECK_EQ(3, entries->Get(context, 1)
29254+
.ToLocalChecked()
29255+
->Int32Value(context)
29256+
.FromJust());
29257+
}
29258+
{
29259+
// Create map, create iterator, delete entry, iterate, preview.
29260+
v8::Local<v8::Object> iterator = CompileRun(
29261+
"var map = new Map();"
29262+
"var key = 1; map.set(key, {});"
29263+
"map.set(2, {}); map.set(3, {});"
29264+
"var it = map.keys(); it")
29265+
->ToObject(context)
29266+
.ToLocalChecked();
29267+
CompileRun("map.delete(key); it.next();");
29268+
bool is_key;
29269+
v8::Local<v8::Array> entries =
29270+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29271+
CHECK(!is_key);
29272+
CHECK_EQ(1, entries->Length());
29273+
CHECK_EQ(3, entries->Get(context, 0)
29274+
.ToLocalChecked()
29275+
->Int32Value(context)
29276+
.FromJust());
29277+
}
29278+
{
29279+
// Create map, create iterator, delete entry, iterate until empty, preview.
29280+
v8::Local<v8::Object> iterator = CompileRun(
29281+
"var map = new Map();"
29282+
"var key = 1; map.set(key, {});"
29283+
"map.set(2, {}); map.set(3, {});"
29284+
"var it = map.keys(); it")
29285+
->ToObject(context)
29286+
.ToLocalChecked();
29287+
CompileRun("map.delete(key); it.next(); it.next();");
29288+
bool is_key;
29289+
v8::Local<v8::Array> entries =
29290+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29291+
CHECK(!is_key);
29292+
CHECK_EQ(0, entries->Length());
29293+
}
29294+
}

‎deps/v8/test/inspector/debugger/object-preview-internal-properties-expected.txt‎

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -177,27 +177,39 @@ expression: (new Map([[1,2]])).entries()
177177
}
178178
]
179179

180-
expression: (new Set([[1,2]])).entries()
180+
expression: (new Set([1,2])).entries()
181181
[[Entries]]:
182182
[
183183
[0] : {
184+
key : {
185+
description : 1
186+
overflow : false
187+
properties : [
188+
]
189+
type : number
190+
}
184191
value : {
185-
description : Array(2)
192+
description : 1
186193
overflow : false
187194
properties : [
188-
[0] : {
189-
name : 0
190-
type : number
191-
value : 1
192-
}
193-
[1] : {
194-
name : 1
195-
type : number
196-
value : 2
197-
}
198195
]
199-
subtype : array
200-
type : object
196+
type : number
197+
}
198+
}
199+
[1] : {
200+
key : {
201+
description : 2
202+
overflow : false
203+
properties : [
204+
]
205+
type : number
206+
}
207+
value : {
208+
description : 2
209+
overflow : false
210+
properties : [
211+
]
212+
type : number
201213
}
202214
}
203215
]

0 commit comments

Comments
 (0)