Skip to content

Commit 206e4b0

Browse files
BridgeARrvagg
authored andcommitted
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 831aa9a commit 206e4b0

7 files changed

+279
-30
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030

3131
# Reset this number to 0 on major V8 upgrades.
3232
# Increment by one for each non-official patch applied to deps/v8.
33-
'v8_embedder_string': '-node.17',
33+
'v8_embedder_string': '-node.18',
3434

3535
# Enable disassembler for `--print-code` v8 options
3636
'v8_enable_disassembler': 1,

deps/v8/AUTHORS

+1
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ Rick Waldron <waldron.rick@gmail.com>
142142
Rob Wu <rob@robwu.nl>
143143
Robert Mustacchi <rm@fingolfin.org>
144144
Robert Nagy <robert.nagy@gmail.com>
145+
Ruben Bridgewater <ruben@bridgewater.de>
145146
Ryan Dahl <ry@tinyclouds.org>
146147
Sakthipriyan Vairamani (thefourtheye) <thechargingvolcano@gmail.com>
147148
Sander Mathijs van Veen <sander@leaningtech.com>

deps/v8/src/api.cc

+20-11
Original file line numberDiff line numberDiff line change
@@ -7098,6 +7098,11 @@ enum class MapAsArrayKind {
70987098
kValues = i::JS_MAP_VALUE_ITERATOR_TYPE
70997099
};
71007100

7101+
enum class SetAsArrayKind {
7102+
kEntries = i::JS_SET_KEY_VALUE_ITERATOR_TYPE,
7103+
kValues = i::JS_SET_VALUE_ITERATOR_TYPE
7104+
};
7105+
71017106
i::Handle<i::JSArray> MapAsArray(i::Isolate* isolate, i::Object* table_obj,
71027107
int offset, MapAsArrayKind kind) {
71037108
i::Factory* factory = isolate->factory();
@@ -7207,13 +7212,14 @@ Maybe<bool> Set::Delete(Local<Context> context, Local<Value> key) {
72077212

72087213
namespace {
72097214
i::Handle<i::JSArray> SetAsArray(i::Isolate* isolate, i::Object* table_obj,
7210-
int offset) {
7215+
int offset, SetAsArrayKind kind) {
72117216
i::Factory* factory = isolate->factory();
72127217
i::Handle<i::OrderedHashSet> table(i::OrderedHashSet::cast(table_obj),
72137218
isolate);
72147219
// Elements skipped by |offset| may already be deleted.
72157220
int capacity = table->UsedCapacity();
7216-
int max_length = capacity - offset;
7221+
const bool collect_key_values = kind == SetAsArrayKind::kEntries;
7222+
int max_length = (capacity - offset) * (collect_key_values ? 2 : 1);
72177223
if (max_length == 0) return factory->NewJSArray(0);
72187224
i::Handle<i::FixedArray> result = factory->NewFixedArray(max_length);
72197225
int result_index = 0;
@@ -7224,6 +7230,7 @@ i::Handle<i::JSArray> SetAsArray(i::Isolate* isolate, i::Object* table_obj,
72247230
i::Object* key = table->KeyAt(i);
72257231
if (key == the_hole) continue;
72267232
result->set(result_index++, key);
7233+
if (collect_key_values) result->set(result_index++, key);
72277234
}
72287235
}
72297236
DCHECK_GE(max_length, result_index);
@@ -7239,7 +7246,8 @@ Local<Array> Set::AsArray() const {
72397246
i::Isolate* isolate = obj->GetIsolate();
72407247
LOG_API(isolate, Set, AsArray);
72417248
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
7242-
return Utils::ToLocal(SetAsArray(isolate, obj->table(), 0));
7249+
return Utils::ToLocal(
7250+
SetAsArray(isolate, obj->table(), 0, SetAsArrayKind::kValues));
72437251
}
72447252

72457253

@@ -9585,21 +9593,22 @@ v8::MaybeLocal<v8::Array> v8::Object::PreviewEntries(bool* is_key_value) {
95859593
i::Handle<i::JSWeakCollection>::cast(object), 0));
95869594
}
95879595
if (object->IsJSMapIterator()) {
9588-
i::Handle<i::JSMapIterator> iterator =
9589-
i::Handle<i::JSMapIterator>::cast(object);
9596+
i::Handle<i::JSMapIterator> it = i::Handle<i::JSMapIterator>::cast(object);
95909597
MapAsArrayKind const kind =
9591-
static_cast<MapAsArrayKind>(iterator->map()->instance_type());
9598+
static_cast<MapAsArrayKind>(it->map()->instance_type());
95929599
*is_key_value = kind == MapAsArrayKind::kEntries;
9593-
if (!iterator->HasMore()) return v8::Array::New(v8_isolate);
9594-
return Utils::ToLocal(MapAsArray(isolate, iterator->table(),
9595-
i::Smi::ToInt(iterator->index()), kind));
9600+
if (!it->HasMore()) return v8::Array::New(v8_isolate);
9601+
return Utils::ToLocal(
9602+
MapAsArray(isolate, it->table(), i::Smi::ToInt(it->index()), kind));
95969603
}
95979604
if (object->IsJSSetIterator()) {
95989605
i::Handle<i::JSSetIterator> it = i::Handle<i::JSSetIterator>::cast(object);
9599-
*is_key_value = false;
9606+
SetAsArrayKind const kind =
9607+
static_cast<SetAsArrayKind>(it->map()->instance_type());
9608+
*is_key_value = kind == SetAsArrayKind::kEntries;
96009609
if (!it->HasMore()) return v8::Array::New(v8_isolate);
96019610
return Utils::ToLocal(
9602-
SetAsArray(isolate, it->table(), i::Smi::ToInt(it->index())));
9611+
SetAsArray(isolate, it->table(), i::Smi::ToInt(it->index()), kind));
96039612
}
96049613
return v8::MaybeLocal<v8::Array>();
96059614
}

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

+228-2
Original file line numberDiff line numberDiff line change
@@ -28778,7 +28778,7 @@ TEST(TestSetWasmThreadsEnabledCallback) {
2877828778
CHECK(i_isolate->AreWasmThreadsEnabled(i_context));
2877928779
}
2878028780

28781-
TEST(PreviewSetIteratorEntriesWithDeleted) {
28781+
TEST(PreviewSetKeysIteratorEntriesWithDeleted) {
2878228782
LocalContext env;
2878328783
v8::HandleScope handle_scope(env->GetIsolate());
2878428784
v8::Local<v8::Context> context = env.local();
@@ -28877,7 +28877,142 @@ TEST(PreviewSetIteratorEntriesWithDeleted) {
2887728877
}
2887828878
}
2887928879

28880-
TEST(PreviewMapIteratorEntriesWithDeleted) {
28880+
TEST(PreviewSetValuesIteratorEntriesWithDeleted) {
28881+
LocalContext env;
28882+
v8::HandleScope handle_scope(env->GetIsolate());
28883+
v8::Local<v8::Context> context = env.local();
28884+
28885+
{
28886+
// Create set, delete entry, create iterator, preview.
28887+
v8::Local<v8::Object> iterator =
28888+
CompileRun("var set = new Set([1,2,3]); set.delete(1); set.values()")
28889+
->ToObject(context)
28890+
.ToLocalChecked();
28891+
bool is_key;
28892+
v8::Local<v8::Array> entries =
28893+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28894+
CHECK(!is_key);
28895+
CHECK_EQ(2, entries->Length());
28896+
CHECK_EQ(2, entries->Get(context, 0)
28897+
.ToLocalChecked()
28898+
->Int32Value(context)
28899+
.FromJust());
28900+
CHECK_EQ(3, entries->Get(context, 1)
28901+
.ToLocalChecked()
28902+
->Int32Value(context)
28903+
.FromJust());
28904+
}
28905+
{
28906+
// Create set, create iterator, delete entry, preview.
28907+
v8::Local<v8::Object> iterator =
28908+
CompileRun("var set = new Set([1,2,3]); set.values()")
28909+
->ToObject(context)
28910+
.ToLocalChecked();
28911+
CompileRun("set.delete(1);");
28912+
bool is_key;
28913+
v8::Local<v8::Array> entries =
28914+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28915+
CHECK(!is_key);
28916+
CHECK_EQ(2, entries->Length());
28917+
CHECK_EQ(2, entries->Get(context, 0)
28918+
.ToLocalChecked()
28919+
->Int32Value(context)
28920+
.FromJust());
28921+
CHECK_EQ(3, entries->Get(context, 1)
28922+
.ToLocalChecked()
28923+
->Int32Value(context)
28924+
.FromJust());
28925+
}
28926+
{
28927+
// Create set, create iterator, delete entry, iterate, preview.
28928+
v8::Local<v8::Object> iterator =
28929+
CompileRun("var set = new Set([1,2,3]); var it = set.values(); it")
28930+
->ToObject(context)
28931+
.ToLocalChecked();
28932+
CompileRun("set.delete(1); it.next();");
28933+
bool is_key;
28934+
v8::Local<v8::Array> entries =
28935+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28936+
CHECK(!is_key);
28937+
CHECK_EQ(1, entries->Length());
28938+
CHECK_EQ(3, entries->Get(context, 0)
28939+
.ToLocalChecked()
28940+
->Int32Value(context)
28941+
.FromJust());
28942+
}
28943+
{
28944+
// Create set, create iterator, delete entry, iterate until empty, preview.
28945+
v8::Local<v8::Object> iterator =
28946+
CompileRun("var set = new Set([1,2,3]); var it = set.values(); it")
28947+
->ToObject(context)
28948+
.ToLocalChecked();
28949+
CompileRun("set.delete(1); it.next(); it.next();");
28950+
bool is_key;
28951+
v8::Local<v8::Array> entries =
28952+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28953+
CHECK(!is_key);
28954+
CHECK_EQ(0, entries->Length());
28955+
}
28956+
{
28957+
// Create set, create iterator, delete entry, iterate, trigger rehash,
28958+
// preview.
28959+
v8::Local<v8::Object> iterator =
28960+
CompileRun("var set = new Set([1,2,3]); var it = set.values(); it")
28961+
->ToObject(context)
28962+
.ToLocalChecked();
28963+
CompileRun("set.delete(1); it.next();");
28964+
CompileRun("for (var i = 4; i < 20; i++) set.add(i);");
28965+
bool is_key;
28966+
v8::Local<v8::Array> entries =
28967+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28968+
CHECK(!is_key);
28969+
CHECK_EQ(17, entries->Length());
28970+
for (uint32_t i = 0; i < 17; i++) {
28971+
CHECK_EQ(i + 3, entries->Get(context, i)
28972+
.ToLocalChecked()
28973+
->Int32Value(context)
28974+
.FromJust());
28975+
}
28976+
}
28977+
}
28978+
28979+
TEST(PreviewMapEntriesIteratorEntries) {
28980+
LocalContext env;
28981+
v8::HandleScope handle_scope(env->GetIsolate());
28982+
v8::Local<v8::Context> context = env.local();
28983+
{
28984+
// Create set, delete entry, create entries iterator, preview.
28985+
v8::Local<v8::Object> iterator =
28986+
CompileRun("var set = new Set([1,2,3]); set.delete(2); set.entries()")
28987+
->ToObject(context)
28988+
.ToLocalChecked();
28989+
bool is_key;
28990+
v8::Local<v8::Array> entries =
28991+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28992+
CHECK(is_key);
28993+
CHECK_EQ(4, entries->Length());
28994+
uint32_t first = entries->Get(context, 0)
28995+
.ToLocalChecked()
28996+
->Int32Value(context)
28997+
.FromJust();
28998+
uint32_t second = entries->Get(context, 2)
28999+
.ToLocalChecked()
29000+
->Int32Value(context)
29001+
.FromJust();
29002+
CHECK_EQ(1, first);
29003+
CHECK_EQ(3, second);
29004+
CHECK_EQ(first, entries->Get(context, 1)
29005+
.ToLocalChecked()
29006+
->Int32Value(context)
29007+
.FromJust());
29008+
CHECK_EQ(second, entries->Get(context, 3)
29009+
.ToLocalChecked()
29010+
->Int32Value(context)
29011+
.FromJust());
29012+
}
29013+
}
29014+
29015+
TEST(PreviewMapValuesIteratorEntriesWithDeleted) {
2888129016
LocalContext env;
2888229017
v8::HandleScope handle_scope(env->GetIsolate());
2888329018
v8::Local<v8::Context> context = env.local();
@@ -28991,3 +29126,94 @@ TEST(PreviewMapIteratorEntriesWithDeleted) {
2899129126
}
2899229127
}
2899329128
}
29129+
29130+
TEST(PreviewMapKeysIteratorEntriesWithDeleted) {
29131+
LocalContext env;
29132+
v8::HandleScope handle_scope(env->GetIsolate());
29133+
v8::Local<v8::Context> context = env.local();
29134+
29135+
{
29136+
// Create map, delete entry, create iterator, preview.
29137+
v8::Local<v8::Object> iterator = CompileRun(
29138+
"var map = new Map();"
29139+
"var key = 1; map.set(key, {});"
29140+
"map.set(2, {}); map.set(3, {});"
29141+
"map.delete(key);"
29142+
"map.keys()")
29143+
->ToObject(context)
29144+
.ToLocalChecked();
29145+
bool is_key;
29146+
v8::Local<v8::Array> entries =
29147+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29148+
CHECK(!is_key);
29149+
CHECK_EQ(2, entries->Length());
29150+
CHECK_EQ(2, entries->Get(context, 0)
29151+
.ToLocalChecked()
29152+
->Int32Value(context)
29153+
.FromJust());
29154+
CHECK_EQ(3, entries->Get(context, 1)
29155+
.ToLocalChecked()
29156+
->Int32Value(context)
29157+
.FromJust());
29158+
}
29159+
{
29160+
// Create map, create iterator, delete entry, preview.
29161+
v8::Local<v8::Object> iterator = CompileRun(
29162+
"var map = new Map();"
29163+
"var key = 1; map.set(key, {});"
29164+
"map.set(2, {}); map.set(3, {});"
29165+
"map.keys()")
29166+
->ToObject(context)
29167+
.ToLocalChecked();
29168+
CompileRun("map.delete(key);");
29169+
bool is_key;
29170+
v8::Local<v8::Array> entries =
29171+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29172+
CHECK(!is_key);
29173+
CHECK_EQ(2, entries->Length());
29174+
CHECK_EQ(2, entries->Get(context, 0)
29175+
.ToLocalChecked()
29176+
->Int32Value(context)
29177+
.FromJust());
29178+
CHECK_EQ(3, entries->Get(context, 1)
29179+
.ToLocalChecked()
29180+
->Int32Value(context)
29181+
.FromJust());
29182+
}
29183+
{
29184+
// Create map, create iterator, delete entry, iterate, preview.
29185+
v8::Local<v8::Object> iterator = CompileRun(
29186+
"var map = new Map();"
29187+
"var key = 1; map.set(key, {});"
29188+
"map.set(2, {}); map.set(3, {});"
29189+
"var it = map.keys(); it")
29190+
->ToObject(context)
29191+
.ToLocalChecked();
29192+
CompileRun("map.delete(key); it.next();");
29193+
bool is_key;
29194+
v8::Local<v8::Array> entries =
29195+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29196+
CHECK(!is_key);
29197+
CHECK_EQ(1, entries->Length());
29198+
CHECK_EQ(3, entries->Get(context, 0)
29199+
.ToLocalChecked()
29200+
->Int32Value(context)
29201+
.FromJust());
29202+
}
29203+
{
29204+
// Create map, create iterator, delete entry, iterate until empty, preview.
29205+
v8::Local<v8::Object> iterator = CompileRun(
29206+
"var map = new Map();"
29207+
"var key = 1; map.set(key, {});"
29208+
"map.set(2, {}); map.set(3, {});"
29209+
"var it = map.keys(); it")
29210+
->ToObject(context)
29211+
.ToLocalChecked();
29212+
CompileRun("map.delete(key); it.next(); it.next();");
29213+
bool is_key;
29214+
v8::Local<v8::Array> entries =
29215+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29216+
CHECK(!is_key);
29217+
CHECK_EQ(0, entries->Length());
29218+
}
29219+
}

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

+26-14
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)