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 <timothygu99@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Author: addaleax

lib/console.js

@@ -29,7 +29,7 @@ const {
 ERR_INVALID_ARG_VALUE,
 },
 }=require('internal/errors');
-const{previewMapIterator, previewSetIterator }=require('internal/v8');
+const{previewEntries }=process.binding('util');
 const{Buffer: { isBuffer }}=require('buffer');
 constutil=require('util');
 const{
@@ -341,7 +341,7 @@ Console.prototype.table = function(tabularData, properties) {
 
 constmapIter=isMapIterator(tabularData);
 if(mapIter)
-tabularData=previewMapIterator(tabularData);
+tabularData=previewEntries(tabularData);
 
 if(mapIter||isMap(tabularData)){
 constkeys=[];
@@ -363,7 +363,7 @@ Console.prototype.table = function(tabularData, properties) {
 
 constsetIter=isSetIterator(tabularData);
 if(setIter)
-tabularData=previewSetIterator(tabularData);
+tabularData=previewEntries(tabularData);
 
 constsetlike=setIter||isSet(tabularData);
 if(setlike){

lib/internal/bootstrap/node.js

@@ -29,7 +29,6 @@
 // Do this good and early, since it handles errors.
 setupProcessFatal();
 
-setupV8();
 setupProcessICUVersions();
 
 setupGlobalVariables();
@@ -478,22 +477,6 @@
 };
 }
 
-functionsetupV8(){
-// Warm up the map and set iterator preview functions. V8 compiles
-// functions lazily (unless --nolazy is set) so we need to do this
-// before we turn off --allow_natives_syntax again.
-constv8=NativeModule.require('internal/v8');
-v8.previewMapIterator(newMap().entries());
-v8.previewSetIterator(newSet().entries());
-v8.previewWeakMap(newWeakMap(),1);
-v8.previewWeakSet(newWeakSet(),1);
-// Disable --allow_natives_syntax again unless it was explicitly
-// specified on the command line.
-constre=/^--allow[-_]natives[-_]syntax$/;
-if(!process.execArgv.some((s)=>re.test(s)))
-process.binding('v8').setFlagsFromString('--noallow_natives_syntax');
-}
-
 functionsetupProcessICUVersions(){
 consticu=process.binding('config').hasIntl ?
 process.binding('icu') : undefined;

lib/internal/v8.js

@@ -30,18 +30,12 @@ const {
 const{ TextDecoder, TextEncoder }=require('internal/encoding');
 const{ isBuffer }=require('buffer').Buffer;
 
-const{
- previewMapIterator,
- previewSetIterator,
- previewWeakMap,
- previewWeakSet
-}=require('internal/v8');
-
 const{
  getPromiseDetails,
  getProxyDetails,
  kPending,
  kRejected,
+ previewEntries
 }=process.binding('util');
 
 const{ internalBinding }=require('internal/bootstrap/loaders');
@@ -912,7 +906,7 @@ function formatMap(ctx, value, recurseTimes, keys) {
 
 functionformatWeakSet(ctx,value,recurseTimes,keys){
 constmaxArrayLength=Math.max(ctx.maxArrayLength,0);
-constentries=previewWeakSet(value,maxArrayLength+1);
+constentries=previewEntries(value).slice(0,maxArrayLength+1);
 constmaxLength=Math.min(maxArrayLength,entries.length);
 letoutput=newArray(maxLength);
 for(vari=0;i<maxLength;++i)
@@ -929,16 +923,14 @@ function formatWeakSet(ctx, value, recurseTimes, keys) {
 
 functionformatWeakMap(ctx,value,recurseTimes,keys){
 constmaxArrayLength=Math.max(ctx.maxArrayLength,0);
-constentries=previewWeakMap(value,maxArrayLength+1);
-// Entries exist as [key1, val1, key2, val2, ...]
-constremainder=entries.length/2>maxArrayLength;
-constlen=entries.length/2-(remainder ? 1 : 0);
+constentries=previewEntries(value).slice(0,maxArrayLength+1);
+constremainder=entries.length>maxArrayLength;
+constlen=entries.length-(remainder ? 1 : 0);
 constmaxLength=Math.min(maxArrayLength,len);
 letoutput=newArray(maxLength);
 for(vari=0;i<len;i++){
-constpos=i*2;
-output[i]=`${formatValue(ctx,entries[pos],recurseTimes)} => `+
-formatValue(ctx,entries[pos+1],recurseTimes);
+output[i]=`${formatValue(ctx,entries[i][0],recurseTimes)} => `+
+formatValue(ctx,entries[i][1],recurseTimes);
 }
 // Sort all entries to have a halfway reliable output (if more entries than
 // retrieved ones exist, we can not reliably return the same output).
@@ -950,9 +942,9 @@ function formatWeakMap(ctx, value, recurseTimes, keys) {
 returnoutput;
 }
 
-functionformatCollectionIterator(preview,ctx,value,recurseTimes,keys){
+functionformatCollectionIterator(ctx,value,recurseTimes,keys){
 constoutput=[];
-for(constentryofpreview(value)){
+for(constentryofpreviewEntries(value)){
 if(ctx.maxArrayLength===output.length){
 output.push('... more items');
 break;
@@ -966,13 +958,11 @@ function formatCollectionIterator(preview, ctx, value, recurseTimes, keys) {
 }
 
 functionformatMapIterator(ctx,value,recurseTimes,keys){
-returnformatCollectionIterator(previewMapIterator,ctx,value,recurseTimes,
-keys);
+returnformatCollectionIterator(ctx,value,recurseTimes,keys);
 }
 
 functionformatSetIterator(ctx,value,recurseTimes,keys){
-returnformatCollectionIterator(previewSetIterator,ctx,value,recurseTimes,
-keys);
+returnformatCollectionIterator(ctx,value,recurseTimes,keys);
 }
 
 functionformatPromise(ctx,value,recurseTimes,keys){

lib/util.js

@@ -145,7 +145,6 @@
 'lib/internal/http2/core.js',
 'lib/internal/http2/compat.js',
 'lib/internal/http2/util.js',
-'lib/internal/v8.js',
 'lib/internal/v8_prof_polyfill.js',
 'lib/internal/v8_prof_processor.js',
 'lib/internal/stream_base_commons.js',

node.gyp

@@ -4369,12 +4369,6 @@ void Init(int* argc,
  }
 #endif
 
-// Needed for access to V8 intrinsics. Disabled again during bootstrapping,
-// see lib/internal/bootstrap/node.js.
-constchar allow_natives_syntax[] = "--allow_natives_syntax";
-V8::SetFlagsFromString(allow_natives_syntax,
-sizeof(allow_natives_syntax) - 1);
-
 // We should set node_is_initialized here instead of in node::Start,
 // otherwise embedders using node::Init to initialize everything will not be
 // able to set it and native modules will not load for them.

src/node.cc

@@ -49,6 +49,35 @@ static void GetProxyDetails(const FunctionCallbackInfo<Value>& args) {
  args.GetReturnValue().Set(ret);
 }
 
+staticvoidPreviewEntries(const FunctionCallbackInfo<Value>& args) {
+if (!args[0]->IsObject())
+return;
+
+bool is_key_value;
+ Local<Array> entries;
+if (!args[0].As<Object>()->PreviewEntries(&is_key_value).ToLocal(&entries))
+return;
+if (!is_key_value)
+return args.GetReturnValue().Set(entries);
+
+uint32_t length = entries->Length();
+CHECK_EQ(length % 2, 0);
+
+ Environment* env = Environment::GetCurrent(args);
+ Local<Context> context = env->context();
+
+ Local<Array> pairs = Array::New(env->isolate(), length / 2);
+for (uint32_t i = 0; i < length / 2; i++) {
+ Local<Array> pair = Array::New(env->isolate(), 2);
+ pair->Set(context, 0, entries->Get(context, i * 2).ToLocalChecked())
+ .FromJust();
+ pair->Set(context, 1, entries->Get(context, i * 2 + 1).ToLocalChecked())
+ .FromJust();
+ pairs->Set(context, i, pair).FromJust();
+ }
+ args.GetReturnValue().Set(pairs);
+}
+
 // Side effect-free stringification that will never throw exceptions.
 staticvoidSafeToString(const FunctionCallbackInfo<Value>& args) {
 auto context = args.GetIsolate()->GetCurrentContext();
@@ -188,6 +217,7 @@ void Initialize(Local<Object> target,
  env->SetMethod(target, "getPromiseDetails", GetPromiseDetails);
  env->SetMethod(target, "getProxyDetails", GetProxyDetails);
  env->SetMethod(target, "safeToString", SafeToString);
+ env->SetMethod(target, "previewEntries", PreviewEntries);
 
  env->SetMethod(target, "startSigintWatchdog", StartSigintWatchdog);
  env->SetMethod(target, "stopSigintWatchdog", StopSigintWatchdog);

src/node_util.cc

@@ -19,14 +19,13 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-// Flags: --expose_internals
 'use strict';
 constcommon=require('../common');
 constassert=require('assert');
 constJSStream=process.binding('js_stream').JSStream;
 constutil=require('util');
 constvm=require('vm');
-const{previewMapIterator}=require('internal/v8');
+const{previewEntries}=process.binding('util');
 
 assert.strictEqual(util.inspect(1),'1');
 assert.strictEqual(util.inspect(false),'false');
@@ -448,7 +447,7 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
 {
 constmap=newMap();
 map.set(1,2);
-constvals=previewMapIterator(map.entries());
+constvals=previewEntries(map.entries());
 constvalsOutput=[];
 for(constoofvals){
 valsOutput.push(o);
@@ -935,8 +934,7 @@ if (typeof Symbol !== 'undefined') {
 constaSet=newSet([1,3]);
 assert.strictEqual(util.inspect(aSet.keys()),'[Set Iterator] { 1, 3 }');
 assert.strictEqual(util.inspect(aSet.values()),'[Set Iterator] { 1, 3 }');
-assert.strictEqual(util.inspect(aSet.entries()),
-'[Set Iterator] { [ 1, 1 ], [ 3, 3 ] }');
+assert.strictEqual(util.inspect(aSet.entries()),'[Set Iterator] { 1, 3 }');
 // Make sure the iterator doesn't get consumed.
 constkeys=aSet.keys();
 assert.strictEqual(util.inspect(keys),'[Set Iterator] { 1, 3 }');