Skip to content

Commit cf649c9

Browse files
targostargos
committed
deps: V8: cherry-pick 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: #25852 Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
1 parent 44d5401 commit cf649c9

File tree

7 files changed

+279
-30
lines changed

7 files changed

+279
-30
lines changed

‎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.1',
40+
'v8_embedder_string': '-node.2',
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
@@ -148,6 +148,7 @@ Rick Waldron <waldron.rick@gmail.com>
148148
Rob Wu <rob@robwu.nl>
149149
Robert Mustacchi <rm@fingolfin.org>
150150
Robert Nagy <robert.nagy@gmail.com>
151+
Ruben Bridgewater <ruben@bridgewater.de>
151152
Ryan Dahl <ry@tinyclouds.org>
152153
Sakthipriyan Vairamani (thefourtheye) <thechargingvolcano@gmail.com>
153154
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
@@ -6958,6 +6958,11 @@ enum class MapAsArrayKind {
69586958
kValues = i::JS_MAP_VALUE_ITERATOR_TYPE
69596959
};
69606960

6961+
enum class SetAsArrayKind {
6962+
kEntries = i::JS_SET_KEY_VALUE_ITERATOR_TYPE,
6963+
kValues = i::JS_SET_VALUE_ITERATOR_TYPE
6964+
};
6965+
69616966
i::Handle<i::JSArray> MapAsArray(i::Isolate* isolate, i::Object table_obj,
69626967
int offset, MapAsArrayKind kind) {
69636968
i::Factory* factory = isolate->factory();
@@ -7067,13 +7072,14 @@ Maybe<bool> Set::Delete(Local<Context> context, Local<Value> key) {
70677072

70687073
namespace {
70697074
i::Handle<i::JSArray> SetAsArray(i::Isolate* isolate, i::Object table_obj,
7070-
int offset) {
7075+
int offset, SetAsArrayKind kind) {
70717076
i::Factory* factory = isolate->factory();
70727077
i::Handle<i::OrderedHashSet> table(i::OrderedHashSet::cast(table_obj),
70737078
isolate);
70747079
// Elements skipped by |offset| may already be deleted.
70757080
int capacity = table->UsedCapacity();
7076-
int max_length = capacity - offset;
7081+
const bool collect_key_values = kind == SetAsArrayKind::kEntries;
7082+
int max_length = (capacity - offset) * (collect_key_values ? 2 : 1);
70777083
if (max_length == 0) return factory->NewJSArray(0);
70787084
i::Handle<i::FixedArray> result = factory->NewFixedArray(max_length);
70797085
int result_index = 0;
@@ -7084,6 +7090,7 @@ i::Handle<i::JSArray> SetAsArray(i::Isolate* isolate, i::Object table_obj,
70847090
i::Object key = table->KeyAt(i);
70857091
if (key == the_hole) continue;
70867092
result->set(result_index++, key);
7093+
if (collect_key_values) result->set(result_index++, key);
70877094
}
70887095
}
70897096
DCHECK_GE(max_length, result_index);
@@ -7099,7 +7106,8 @@ Local<Array> Set::AsArray() const {
70997106
i::Isolate* isolate = obj->GetIsolate();
71007107
LOG_API(isolate, Set, AsArray);
71017108
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
7102-
return Utils::ToLocal(SetAsArray(isolate, obj->table(), 0));
7109+
return Utils::ToLocal(
7110+
SetAsArray(isolate, obj->table(), 0, SetAsArrayKind::kValues));
71037111
}
71047112

71057113

@@ -9516,21 +9524,22 @@ v8::MaybeLocal<v8::Array> v8::Object::PreviewEntries(bool* is_key_value) {
95169524
i::Handle<i::JSWeakCollection>::cast(object), 0));
95179525
}
95189526
if (object->IsJSMapIterator()) {
9519-
i::Handle<i::JSMapIterator> iterator =
9520-
i::Handle<i::JSMapIterator>::cast(object);
9527+
i::Handle<i::JSMapIterator> it = i::Handle<i::JSMapIterator>::cast(object);
95219528
MapAsArrayKind const kind =
9522-
static_cast<MapAsArrayKind>(iterator->map()->instance_type());
9529+
static_cast<MapAsArrayKind>(it->map()->instance_type());
95239530
*is_key_value = kind == MapAsArrayKind::kEntries;
9524-
if (!iterator->HasMore()) return v8::Array::New(v8_isolate);
9525-
return Utils::ToLocal(MapAsArray(isolate, iterator->table(),
9526-
i::Smi::ToInt(iterator->index()), kind));
9531+
if (!it->HasMore()) return v8::Array::New(v8_isolate);
9532+
return Utils::ToLocal(
9533+
MapAsArray(isolate, it->table(), i::Smi::ToInt(it->index()), kind));
95279534
}
95289535
if (object->IsJSSetIterator()) {
95299536
i::Handle<i::JSSetIterator> it = i::Handle<i::JSSetIterator>::cast(object);
9530-
*is_key_value = false;
9537+
SetAsArrayKind const kind =
9538+
static_cast<SetAsArrayKind>(it->map()->instance_type());
9539+
*is_key_value = kind == SetAsArrayKind::kEntries;
95319540
if (!it->HasMore()) return v8::Array::New(v8_isolate);
95329541
return Utils::ToLocal(
9533-
SetAsArray(isolate, it->table(), i::Smi::ToInt(it->index())));
9542+
SetAsArray(isolate, it->table(), i::Smi::ToInt(it->index()), kind));
95349543
}
95359544
return v8::MaybeLocal<v8::Array>();
95369545
}

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

Lines changed: 228 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28644,7 +28644,7 @@ TEST(MicrotaskContextShouldBeNativeContext) {
2864428644
isolate->RunMicrotasks();
2864528645
}
2864628646

28647-
TEST(PreviewSetIteratorEntriesWithDeleted) {
28647+
TEST(PreviewSetKeysIteratorEntriesWithDeleted) {
2864828648
LocalContext env;
2864928649
v8::HandleScope handle_scope(env->GetIsolate());
2865028650
v8::Local<v8::Context> context = env.local();
@@ -28743,7 +28743,142 @@ TEST(PreviewSetIteratorEntriesWithDeleted) {
2874328743
}
2874428744
}
2874528745

28746-
TEST(PreviewMapIteratorEntriesWithDeleted) {
28746+
TEST(PreviewSetValuesIteratorEntriesWithDeleted) {
28747+
LocalContext env;
28748+
v8::HandleScope handle_scope(env->GetIsolate());
28749+
v8::Local<v8::Context> context = env.local();
28750+
28751+
{
28752+
// Create set, delete entry, create iterator, preview.
28753+
v8::Local<v8::Object> iterator =
28754+
CompileRun("var set = new Set([1,2,3]); set.delete(1); set.values()")
28755+
->ToObject(context)
28756+
.ToLocalChecked();
28757+
bool is_key;
28758+
v8::Local<v8::Array> entries =
28759+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28760+
CHECK(!is_key);
28761+
CHECK_EQ(2, entries->Length());
28762+
CHECK_EQ(2, entries->Get(context, 0)
28763+
.ToLocalChecked()
28764+
->Int32Value(context)
28765+
.FromJust());
28766+
CHECK_EQ(3, entries->Get(context, 1)
28767+
.ToLocalChecked()
28768+
->Int32Value(context)
28769+
.FromJust());
28770+
}
28771+
{
28772+
// Create set, create iterator, delete entry, preview.
28773+
v8::Local<v8::Object> iterator =
28774+
CompileRun("var set = new Set([1,2,3]); set.values()")
28775+
->ToObject(context)
28776+
.ToLocalChecked();
28777+
CompileRun("set.delete(1);");
28778+
bool is_key;
28779+
v8::Local<v8::Array> entries =
28780+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28781+
CHECK(!is_key);
28782+
CHECK_EQ(2, entries->Length());
28783+
CHECK_EQ(2, entries->Get(context, 0)
28784+
.ToLocalChecked()
28785+
->Int32Value(context)
28786+
.FromJust());
28787+
CHECK_EQ(3, entries->Get(context, 1)
28788+
.ToLocalChecked()
28789+
->Int32Value(context)
28790+
.FromJust());
28791+
}
28792+
{
28793+
// Create set, create iterator, delete entry, iterate, preview.
28794+
v8::Local<v8::Object> iterator =
28795+
CompileRun("var set = new Set([1,2,3]); var it = set.values(); it")
28796+
->ToObject(context)
28797+
.ToLocalChecked();
28798+
CompileRun("set.delete(1); it.next();");
28799+
bool is_key;
28800+
v8::Local<v8::Array> entries =
28801+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28802+
CHECK(!is_key);
28803+
CHECK_EQ(1, entries->Length());
28804+
CHECK_EQ(3, entries->Get(context, 0)
28805+
.ToLocalChecked()
28806+
->Int32Value(context)
28807+
.FromJust());
28808+
}
28809+
{
28810+
// Create set, create iterator, delete entry, iterate until empty, preview.
28811+
v8::Local<v8::Object> iterator =
28812+
CompileRun("var set = new Set([1,2,3]); var it = set.values(); it")
28813+
->ToObject(context)
28814+
.ToLocalChecked();
28815+
CompileRun("set.delete(1); it.next(); it.next();");
28816+
bool is_key;
28817+
v8::Local<v8::Array> entries =
28818+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28819+
CHECK(!is_key);
28820+
CHECK_EQ(0, entries->Length());
28821+
}
28822+
{
28823+
// Create set, create iterator, delete entry, iterate, trigger rehash,
28824+
// preview.
28825+
v8::Local<v8::Object> iterator =
28826+
CompileRun("var set = new Set([1,2,3]); var it = set.values(); it")
28827+
->ToObject(context)
28828+
.ToLocalChecked();
28829+
CompileRun("set.delete(1); it.next();");
28830+
CompileRun("for (var i = 4; i < 20; i++) set.add(i);");
28831+
bool is_key;
28832+
v8::Local<v8::Array> entries =
28833+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28834+
CHECK(!is_key);
28835+
CHECK_EQ(17, entries->Length());
28836+
for (uint32_t i = 0; i < 17; i++) {
28837+
CHECK_EQ(i + 3, entries->Get(context, i)
28838+
.ToLocalChecked()
28839+
->Int32Value(context)
28840+
.FromJust());
28841+
}
28842+
}
28843+
}
28844+
28845+
TEST(PreviewMapEntriesIteratorEntries) {
28846+
LocalContext env;
28847+
v8::HandleScope handle_scope(env->GetIsolate());
28848+
v8::Local<v8::Context> context = env.local();
28849+
{
28850+
// Create set, delete entry, create entries iterator, preview.
28851+
v8::Local<v8::Object> iterator =
28852+
CompileRun("var set = new Set([1,2,3]); set.delete(2); set.entries()")
28853+
->ToObject(context)
28854+
.ToLocalChecked();
28855+
bool is_key;
28856+
v8::Local<v8::Array> entries =
28857+
iterator->PreviewEntries(&is_key).ToLocalChecked();
28858+
CHECK(is_key);
28859+
CHECK_EQ(4, entries->Length());
28860+
uint32_t first = entries->Get(context, 0)
28861+
.ToLocalChecked()
28862+
->Int32Value(context)
28863+
.FromJust();
28864+
uint32_t second = entries->Get(context, 2)
28865+
.ToLocalChecked()
28866+
->Int32Value(context)
28867+
.FromJust();
28868+
CHECK_EQ(1, first);
28869+
CHECK_EQ(3, second);
28870+
CHECK_EQ(first, entries->Get(context, 1)
28871+
.ToLocalChecked()
28872+
->Int32Value(context)
28873+
.FromJust());
28874+
CHECK_EQ(second, entries->Get(context, 3)
28875+
.ToLocalChecked()
28876+
->Int32Value(context)
28877+
.FromJust());
28878+
}
28879+
}
28880+
28881+
TEST(PreviewMapValuesIteratorEntriesWithDeleted) {
2874728882
LocalContext env;
2874828883
v8::HandleScope handle_scope(env->GetIsolate());
2874928884
v8::Local<v8::Context> context = env.local();
@@ -28858,6 +28993,97 @@ TEST(PreviewMapIteratorEntriesWithDeleted) {
2885828993
}
2885928994
}
2886028995

28996+
TEST(PreviewMapKeysIteratorEntriesWithDeleted) {
28997+
LocalContext env;
28998+
v8::HandleScope handle_scope(env->GetIsolate());
28999+
v8::Local<v8::Context> context = env.local();
29000+
29001+
{
29002+
// Create map, delete entry, create iterator, preview.
29003+
v8::Local<v8::Object> iterator = CompileRun(
29004+
"var map = new Map();"
29005+
"var key = 1; map.set(key, {});"
29006+
"map.set(2, {}); map.set(3, {});"
29007+
"map.delete(key);"
29008+
"map.keys()")
29009+
->ToObject(context)
29010+
.ToLocalChecked();
29011+
bool is_key;
29012+
v8::Local<v8::Array> entries =
29013+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29014+
CHECK(!is_key);
29015+
CHECK_EQ(2, entries->Length());
29016+
CHECK_EQ(2, entries->Get(context, 0)
29017+
.ToLocalChecked()
29018+
->Int32Value(context)
29019+
.FromJust());
29020+
CHECK_EQ(3, entries->Get(context, 1)
29021+
.ToLocalChecked()
29022+
->Int32Value(context)
29023+
.FromJust());
29024+
}
29025+
{
29026+
// Create map, create iterator, delete entry, preview.
29027+
v8::Local<v8::Object> iterator = CompileRun(
29028+
"var map = new Map();"
29029+
"var key = 1; map.set(key, {});"
29030+
"map.set(2, {}); map.set(3, {});"
29031+
"map.keys()")
29032+
->ToObject(context)
29033+
.ToLocalChecked();
29034+
CompileRun("map.delete(key);");
29035+
bool is_key;
29036+
v8::Local<v8::Array> entries =
29037+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29038+
CHECK(!is_key);
29039+
CHECK_EQ(2, entries->Length());
29040+
CHECK_EQ(2, entries->Get(context, 0)
29041+
.ToLocalChecked()
29042+
->Int32Value(context)
29043+
.FromJust());
29044+
CHECK_EQ(3, entries->Get(context, 1)
29045+
.ToLocalChecked()
29046+
->Int32Value(context)
29047+
.FromJust());
29048+
}
29049+
{
29050+
// Create map, create iterator, delete entry, iterate, preview.
29051+
v8::Local<v8::Object> iterator = CompileRun(
29052+
"var map = new Map();"
29053+
"var key = 1; map.set(key, {});"
29054+
"map.set(2, {}); map.set(3, {});"
29055+
"var it = map.keys(); it")
29056+
->ToObject(context)
29057+
.ToLocalChecked();
29058+
CompileRun("map.delete(key); it.next();");
29059+
bool is_key;
29060+
v8::Local<v8::Array> entries =
29061+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29062+
CHECK(!is_key);
29063+
CHECK_EQ(1, entries->Length());
29064+
CHECK_EQ(3, entries->Get(context, 0)
29065+
.ToLocalChecked()
29066+
->Int32Value(context)
29067+
.FromJust());
29068+
}
29069+
{
29070+
// Create map, create iterator, delete entry, iterate until empty, preview.
29071+
v8::Local<v8::Object> iterator = CompileRun(
29072+
"var map = new Map();"
29073+
"var key = 1; map.set(key, {});"
29074+
"map.set(2, {}); map.set(3, {});"
29075+
"var it = map.keys(); it")
29076+
->ToObject(context)
29077+
.ToLocalChecked();
29078+
CompileRun("map.delete(key); it.next(); it.next();");
29079+
bool is_key;
29080+
v8::Local<v8::Array> entries =
29081+
iterator->PreviewEntries(&is_key).ToLocalChecked();
29082+
CHECK(!is_key);
29083+
CHECK_EQ(0, entries->Length());
29084+
}
29085+
}
29086+
2886129087
namespace {
2886229088
static v8::Isolate* isolate_1;
2886329089
static v8::Isolate* isolate_2;

‎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
@@ -166,27 +166,39 @@ expression: (new Map([[1,2]])).entries()
166166
}
167167
]
168168

169-
expression: (new Set([[1,2]])).entries()
169+
expression: (new Set([1,2])).entries()
170170
[[Entries]]:
171171
[
172172
[0] : {
173+
key : {
174+
description : 1
175+
overflow : false
176+
properties : [
177+
]
178+
type : number
179+
}
173180
value : {
174-
description : Array(2)
181+
description : 1
175182
overflow : false
176183
properties : [
177-
[0] : {
178-
name : 0
179-
type : number
180-
value : 1
181-
}
182-
[1] : {
183-
name : 1
184-
type : number
185-
value : 2
186-
}
187184
]
188-
subtype : array
189-
type : object
185+
type : number
186+
}
187+
}
188+
[1] : {
189+
key : {
190+
description : 2
191+
overflow : false
192+
properties : [
193+
]
194+
type : number
195+
}
196+
value : {
197+
description : 2
198+
overflow : false
199+
properties : [
200+
]
201+
type : number
190202
}
191203
}
192204
]

0 commit comments

Comments
 (0)