Skip to content

Commit fb7a775

Browse files
addaleaxMylesBorins
authored andcommitted
lib,src: use V8 API for collection inspection
Use a new public V8 API for inspecting weak collections and collection iterators, rather than using V8-internal functions to achieve this. This currently comes with a slight modification of the output for inspecting iterators generated by `Set().entries()`. Fixes: #20409 PR-URL: #20719 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent e56716e commit fb7a775

File tree

8 files changed

+47
-92
lines changed

8 files changed

+47
-92
lines changed

lib/console.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const {
2929
ERR_INVALID_ARG_VALUE,
3030
},
3131
} = require('internal/errors');
32-
const { previewMapIterator, previewSetIterator } = require('internal/v8');
32+
const { previewEntries } = process.binding('util');
3333
const { Buffer: { isBuffer } } = require('buffer');
3434
const util = require('util');
3535
const {
@@ -341,7 +341,7 @@ Console.prototype.table = function(tabularData, properties) {
341341

342342
const mapIter = isMapIterator(tabularData);
343343
if (mapIter)
344-
tabularData = previewMapIterator(tabularData);
344+
tabularData = previewEntries(tabularData);
345345

346346
if (mapIter || isMap(tabularData)) {
347347
const keys = [];
@@ -363,7 +363,7 @@ Console.prototype.table = function(tabularData, properties) {
363363

364364
const setIter = isSetIterator(tabularData);
365365
if (setIter)
366-
tabularData = previewSetIterator(tabularData);
366+
tabularData = previewEntries(tabularData);
367367

368368
const setlike = setIter || isSet(tabularData);
369369
if (setlike) {

lib/internal/bootstrap/node.js

-17
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
// Do this good and early, since it handles errors.
3030
setupProcessFatal();
3131

32-
setupV8();
3332
setupProcessICUVersions();
3433

3534
setupGlobalVariables();
@@ -478,22 +477,6 @@
478477
};
479478
}
480479

481-
function setupV8() {
482-
// Warm up the map and set iterator preview functions. V8 compiles
483-
// functions lazily (unless --nolazy is set) so we need to do this
484-
// before we turn off --allow_natives_syntax again.
485-
const v8 = NativeModule.require('internal/v8');
486-
v8.previewMapIterator(new Map().entries());
487-
v8.previewSetIterator(new Set().entries());
488-
v8.previewWeakMap(new WeakMap(), 1);
489-
v8.previewWeakSet(new WeakSet(), 1);
490-
// Disable --allow_natives_syntax again unless it was explicitly
491-
// specified on the command line.
492-
const re = /^--allow[-_]natives[-_]syntax$/;
493-
if (!process.execArgv.some((s) => re.test(s)))
494-
process.binding('v8').setFlagsFromString('--noallow_natives_syntax');
495-
}
496-
497480
function setupProcessICUVersions() {
498481
const icu = process.binding('config').hasIntl ?
499482
process.binding('icu') : undefined;

lib/internal/v8.js

-39
This file was deleted.

lib/util.js

+11-21
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,12 @@ const {
3030
const { TextDecoder, TextEncoder } = require('internal/encoding');
3131
const { isBuffer } = require('buffer').Buffer;
3232

33-
const {
34-
previewMapIterator,
35-
previewSetIterator,
36-
previewWeakMap,
37-
previewWeakSet
38-
} = require('internal/v8');
39-
4033
const {
4134
getPromiseDetails,
4235
getProxyDetails,
4336
kPending,
4437
kRejected,
38+
previewEntries
4539
} = process.binding('util');
4640

4741
const { internalBinding } = require('internal/bootstrap/loaders');
@@ -912,7 +906,7 @@ function formatMap(ctx, value, recurseTimes, keys) {
912906

913907
function formatWeakSet(ctx, value, recurseTimes, keys) {
914908
const maxArrayLength = Math.max(ctx.maxArrayLength, 0);
915-
const entries = previewWeakSet(value, maxArrayLength + 1);
909+
const entries = previewEntries(value).slice(0, maxArrayLength + 1);
916910
const maxLength = Math.min(maxArrayLength, entries.length);
917911
let output = new Array(maxLength);
918912
for (var i = 0; i < maxLength; ++i)
@@ -929,16 +923,14 @@ function formatWeakSet(ctx, value, recurseTimes, keys) {
929923

930924
function formatWeakMap(ctx, value, recurseTimes, keys) {
931925
const maxArrayLength = Math.max(ctx.maxArrayLength, 0);
932-
const entries = previewWeakMap(value, maxArrayLength + 1);
933-
// Entries exist as [key1, val1, key2, val2, ...]
934-
const remainder = entries.length / 2 > maxArrayLength;
935-
const len = entries.length / 2 - (remainder ? 1 : 0);
926+
const entries = previewEntries(value).slice(0, maxArrayLength + 1);
927+
const remainder = entries.length > maxArrayLength;
928+
const len = entries.length - (remainder ? 1 : 0);
936929
const maxLength = Math.min(maxArrayLength, len);
937930
let output = new Array(maxLength);
938931
for (var i = 0; i < len; i++) {
939-
const pos = i * 2;
940-
output[i] = `${formatValue(ctx, entries[pos], recurseTimes)} => ` +
941-
formatValue(ctx, entries[pos + 1], recurseTimes);
932+
output[i] = `${formatValue(ctx, entries[i][0], recurseTimes)} => ` +
933+
formatValue(ctx, entries[i][1], recurseTimes);
942934
}
943935
// Sort all entries to have a halfway reliable output (if more entries than
944936
// retrieved ones exist, we can not reliably return the same output).
@@ -950,9 +942,9 @@ function formatWeakMap(ctx, value, recurseTimes, keys) {
950942
return output;
951943
}
952944

953-
function formatCollectionIterator(preview, ctx, value, recurseTimes, keys) {
945+
function formatCollectionIterator(ctx, value, recurseTimes, keys) {
954946
const output = [];
955-
for (const entry of preview(value)) {
947+
for (const entry of previewEntries(value)) {
956948
if (ctx.maxArrayLength === output.length) {
957949
output.push('... more items');
958950
break;
@@ -966,13 +958,11 @@ function formatCollectionIterator(preview, ctx, value, recurseTimes, keys) {
966958
}
967959

968960
function formatMapIterator(ctx, value, recurseTimes, keys) {
969-
return formatCollectionIterator(previewMapIterator, ctx, value, recurseTimes,
970-
keys);
961+
return formatCollectionIterator(ctx, value, recurseTimes, keys);
971962
}
972963

973964
function formatSetIterator(ctx, value, recurseTimes, keys) {
974-
return formatCollectionIterator(previewSetIterator, ctx, value, recurseTimes,
975-
keys);
965+
return formatCollectionIterator(ctx, value, recurseTimes, keys);
976966
}
977967

978968
function formatPromise(ctx, value, recurseTimes, keys) {

node.gyp

-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,6 @@
145145
'lib/internal/http2/core.js',
146146
'lib/internal/http2/compat.js',
147147
'lib/internal/http2/util.js',
148-
'lib/internal/v8.js',
149148
'lib/internal/v8_prof_polyfill.js',
150149
'lib/internal/v8_prof_processor.js',
151150
'lib/internal/stream_base_commons.js',

src/node.cc

-6
Original file line numberDiff line numberDiff line change
@@ -4369,12 +4369,6 @@ void Init(int* argc,
43694369
}
43704370
#endif
43714371

4372-
// Needed for access to V8 intrinsics. Disabled again during bootstrapping,
4373-
// see lib/internal/bootstrap/node.js.
4374-
const char allow_natives_syntax[] = "--allow_natives_syntax";
4375-
V8::SetFlagsFromString(allow_natives_syntax,
4376-
sizeof(allow_natives_syntax) - 1);
4377-
43784372
// We should set node_is_initialized here instead of in node::Start,
43794373
// otherwise embedders using node::Init to initialize everything will not be
43804374
// able to set it and native modules will not load for them.

src/node_util.cc

+30
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,35 @@ static void GetProxyDetails(const FunctionCallbackInfo<Value>& args) {
4949
args.GetReturnValue().Set(ret);
5050
}
5151

52+
static void PreviewEntries(const FunctionCallbackInfo<Value>& args) {
53+
if (!args[0]->IsObject())
54+
return;
55+
56+
bool is_key_value;
57+
Local<Array> entries;
58+
if (!args[0].As<Object>()->PreviewEntries(&is_key_value).ToLocal(&entries))
59+
return;
60+
if (!is_key_value)
61+
return args.GetReturnValue().Set(entries);
62+
63+
uint32_t length = entries->Length();
64+
CHECK_EQ(length % 2, 0);
65+
66+
Environment* env = Environment::GetCurrent(args);
67+
Local<Context> context = env->context();
68+
69+
Local<Array> pairs = Array::New(env->isolate(), length / 2);
70+
for (uint32_t i = 0; i < length / 2; i++) {
71+
Local<Array> pair = Array::New(env->isolate(), 2);
72+
pair->Set(context, 0, entries->Get(context, i * 2).ToLocalChecked())
73+
.FromJust();
74+
pair->Set(context, 1, entries->Get(context, i * 2 + 1).ToLocalChecked())
75+
.FromJust();
76+
pairs->Set(context, i, pair).FromJust();
77+
}
78+
args.GetReturnValue().Set(pairs);
79+
}
80+
5281
// Side effect-free stringification that will never throw exceptions.
5382
static void SafeToString(const FunctionCallbackInfo<Value>& args) {
5483
auto context = args.GetIsolate()->GetCurrentContext();
@@ -188,6 +217,7 @@ void Initialize(Local<Object> target,
188217
env->SetMethod(target, "getPromiseDetails", GetPromiseDetails);
189218
env->SetMethod(target, "getProxyDetails", GetProxyDetails);
190219
env->SetMethod(target, "safeToString", SafeToString);
220+
env->SetMethod(target, "previewEntries", PreviewEntries);
191221

192222
env->SetMethod(target, "startSigintWatchdog", StartSigintWatchdog);
193223
env->SetMethod(target, "stopSigintWatchdog", StopSigintWatchdog);

test/parallel/test-util-inspect.js

+3-5
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,13 @@
1919
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
2020
// USE OR OTHER DEALINGS IN THE SOFTWARE.
2121

22-
// Flags: --expose_internals
2322
'use strict';
2423
const common = require('../common');
2524
const assert = require('assert');
2625
const JSStream = process.binding('js_stream').JSStream;
2726
const util = require('util');
2827
const vm = require('vm');
29-
const { previewMapIterator } = require('internal/v8');
28+
const { previewEntries } = process.binding('util');
3029

3130
assert.strictEqual(util.inspect(1), '1');
3231
assert.strictEqual(util.inspect(false), 'false');
@@ -448,7 +447,7 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
448447
{
449448
const map = new Map();
450449
map.set(1, 2);
451-
const vals = previewMapIterator(map.entries());
450+
const vals = previewEntries(map.entries());
452451
const valsOutput = [];
453452
for (const o of vals) {
454453
valsOutput.push(o);
@@ -935,8 +934,7 @@ if (typeof Symbol !== 'undefined') {
935934
const aSet = new Set([1, 3]);
936935
assert.strictEqual(util.inspect(aSet.keys()), '[Set Iterator] { 1, 3 }');
937936
assert.strictEqual(util.inspect(aSet.values()), '[Set Iterator] { 1, 3 }');
938-
assert.strictEqual(util.inspect(aSet.entries()),
939-
'[Set Iterator] { [ 1, 1 ], [ 3, 3 ] }');
937+
assert.strictEqual(util.inspect(aSet.entries()), '[Set Iterator] { 1, 3 }');
940938
// Make sure the iterator doesn't get consumed.
941939
const keys = aSet.keys();
942940
assert.strictEqual(util.inspect(keys), '[Set Iterator] { 1, 3 }');

0 commit comments

Comments
 (0)