From 8b95729c716ce8dc294fe5c7099f51153ee5f184 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Sat, 19 Nov 2022 18:32:01 +0900 Subject: [PATCH 1/3] src,lib: group properties used as constants from `util` binding Properties used as constants in `util` internal binding are scattered. This suggests using an object holding all of them for better maintenance. Signed-off-by: Daeyeon Jeong --- lib/buffer.js | 6 ++-- lib/internal/bootstrap/node.js | 8 +++-- lib/internal/util/comparisons.js | 8 ++--- lib/internal/util/inspect.js | 12 +++---- lib/internal/webstreams/util.js | 4 ++- lib/repl.js | 8 ++--- src/node_util.cc | 61 ++++++++++++++++++-------------- 7 files changed, 60 insertions(+), 47 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 898bc5032e5f8d..08aa52b3883ea1 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -67,11 +67,11 @@ const { kStringMaxLength } = internalBinding('buffer'); const { - getOwnNonIndexProperties, - propertyFilter: { + constants: { ALL_PROPERTIES, - ONLY_ENUMERABLE + ONLY_ENUMERABLE, }, + getOwnNonIndexProperties, } = internalBinding('util'); const { customInspectSymbol, diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 0c903fb32877d2..6da0150b1b2fa3 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -80,12 +80,14 @@ const { validateInteger, } = require('internal/validators'); const { + constants: { + kExitCode, + kExiting, + kHasExitCode, + }, privateSymbols: { exit_info_private_symbol, }, - kExitCode, - kExiting, - kHasExitCode, } = internalBinding('util'); setupProcessObject(); diff --git a/lib/internal/util/comparisons.js b/lib/internal/util/comparisons.js index c126bd6346dae7..e41fcf2daf92a0 100644 --- a/lib/internal/util/comparisons.js +++ b/lib/internal/util/comparisons.js @@ -46,11 +46,11 @@ const { isFloat64Array, } = types; const { - getOwnNonIndexProperties, - propertyFilter: { + constants: { ONLY_ENUMERABLE, - SKIP_SYMBOLS - } + SKIP_SYMBOLS, + }, + getOwnNonIndexProperties, } = internalBinding('util'); const kStrict = true; diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index fed5b8fd068fa3..abbb0d9c8ebc86 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -97,18 +97,18 @@ const { } = primordials; const { + constants: { + ALL_PROPERTIES, + ONLY_ENUMERABLE, + kPending, + kRejected, + }, getOwnNonIndexProperties, getPromiseDetails, getProxyDetails, - kPending, - kRejected, previewEntries, getConstructorName: internalGetConstructorName, getExternalValue, - propertyFilter: { - ALL_PROPERTIES, - ONLY_ENUMERABLE - } } = internalBinding('util'); const { diff --git a/lib/internal/webstreams/util.js b/lib/internal/webstreams/util.js index 0e260d074c73c2..702ae5b8e7045a 100644 --- a/lib/internal/webstreams/util.js +++ b/lib/internal/webstreams/util.js @@ -40,8 +40,10 @@ const { } = require('util'); const { + constants: { + kPending, + }, getPromiseDetails, - kPending, } = internalBinding('util'); const assert = require('internal/assert'); diff --git a/lib/repl.js b/lib/repl.js index 256d910d17e454..0780b5a54743c6 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -166,11 +166,11 @@ const { setupReverseSearch, } = require('internal/repl/utils'); const { - getOwnNonIndexProperties, - propertyFilter: { + constants: { ALL_PROPERTIES, - SKIP_SYMBOLS - } + SKIP_SYMBOLS, + }, + getOwnNonIndexProperties, } = internalBinding('util'); const { startSigintWatchdog, diff --git a/src/node_util.cc b/src/node_util.cc index e9028c3edc4d4f..40a8e90cfc4ded 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -23,6 +23,7 @@ using v8::Isolate; using v8::KeyCollectionMode; using v8::Local; using v8::Object; +using v8::ObjectTemplate; using v8::ONLY_CONFIGURABLE; using v8::ONLY_ENUMERABLE; using v8::ONLY_WRITABLE; @@ -379,27 +380,45 @@ void Initialize(Local target, .Check(); } -#define V(name) \ - target->Set(context, \ - FIXED_ONE_BYTE_STRING(env->isolate(), #name), \ - Integer::New(env->isolate(), Promise::PromiseState::name)) \ - .FromJust() - V(kPending); - V(kFulfilled); - V(kRejected); + { + Local tmpl = ObjectTemplate::New(isolate); +#define V(name) \ + tmpl->Set(FIXED_ONE_BYTE_STRING(isolate, #name), \ + Integer::New(isolate, Promise::PromiseState::name)); + + V(kPending); + V(kFulfilled); + V(kRejected); +#undef V + +#define V(name) \ + tmpl->Set(FIXED_ONE_BYTE_STRING(isolate, #name), \ + Integer::New(isolate, Environment::ExitInfoField::name)); + + V(kExiting); + V(kExitCode); + V(kHasExitCode); #undef V #define V(name) \ - target \ - ->Set(context, \ - FIXED_ONE_BYTE_STRING(env->isolate(), #name), \ - Integer::New(env->isolate(), Environment::ExitInfoField::name)) \ - .FromJust() - V(kExiting); - V(kExitCode); - V(kHasExitCode); + tmpl->Set(FIXED_ONE_BYTE_STRING(isolate, #name), \ + Integer::New(isolate, PropertyFilter::name)); + + V(ALL_PROPERTIES); + V(ONLY_WRITABLE); + V(ONLY_ENUMERABLE); + V(ONLY_CONFIGURABLE); + V(SKIP_STRINGS); + V(SKIP_SYMBOLS); #undef V + target + ->Set(context, + env->constants_string(), + tmpl->NewInstance(context).ToLocalChecked()) + .Check(); + } + SetMethodNoSideEffect( context, target, "getPromiseDetails", GetPromiseDetails); SetMethodNoSideEffect(context, target, "getProxyDetails", GetProxyDetails); @@ -413,16 +432,6 @@ void Initialize(Local target, SetMethod( context, target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer); - Local constants = Object::New(env->isolate()); - NODE_DEFINE_CONSTANT(constants, ALL_PROPERTIES); - NODE_DEFINE_CONSTANT(constants, ONLY_WRITABLE); - NODE_DEFINE_CONSTANT(constants, ONLY_ENUMERABLE); - NODE_DEFINE_CONSTANT(constants, ONLY_CONFIGURABLE); - NODE_DEFINE_CONSTANT(constants, SKIP_STRINGS); - NODE_DEFINE_CONSTANT(constants, SKIP_SYMBOLS); - target->Set(context, - FIXED_ONE_BYTE_STRING(env->isolate(), "propertyFilter"), - constants).Check(); Local should_abort_on_uncaught_toggle = FIXED_ONE_BYTE_STRING(env->isolate(), "shouldAbortOnUncaughtToggle"); From 21e2ae890d1af70e1944ff19e964b69619e29f58 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Wed, 23 Nov 2022 15:13:18 +0900 Subject: [PATCH 2/3] fixup! src,lib: group properties used as constants from `util` binding Signed-off-by: Daeyeon Jeong --- src/node_util.cc | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/node_util.cc b/src/node_util.cc index 40a8e90cfc4ded..48769c0ad816c2 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -381,10 +381,13 @@ void Initialize(Local target, } { - Local tmpl = ObjectTemplate::New(isolate); + Local constants = Object::New(env->isolate()); #define V(name) \ - tmpl->Set(FIXED_ONE_BYTE_STRING(isolate, #name), \ - Integer::New(isolate, Promise::PromiseState::name)); + constants \ + ->Set(context, \ + FIXED_ONE_BYTE_STRING(isolate, #name), \ + Integer::New(isolate, Promise::PromiseState::name)) \ + .Check(); V(kPending); V(kFulfilled); @@ -392,8 +395,11 @@ void Initialize(Local target, #undef V #define V(name) \ - tmpl->Set(FIXED_ONE_BYTE_STRING(isolate, #name), \ - Integer::New(isolate, Environment::ExitInfoField::name)); + constants \ + ->Set(context, \ + FIXED_ONE_BYTE_STRING(isolate, #name), \ + Integer::New(isolate, Environment::ExitInfoField::name)) \ + .Check(); V(kExiting); V(kExitCode); @@ -401,8 +407,11 @@ void Initialize(Local target, #undef V #define V(name) \ - tmpl->Set(FIXED_ONE_BYTE_STRING(isolate, #name), \ - Integer::New(isolate, PropertyFilter::name)); + constants \ + ->Set(context, \ + FIXED_ONE_BYTE_STRING(isolate, #name), \ + Integer::New(isolate, PropertyFilter::name)) \ + .Check(); V(ALL_PROPERTIES); V(ONLY_WRITABLE); @@ -412,11 +421,7 @@ void Initialize(Local target, V(SKIP_SYMBOLS); #undef V - target - ->Set(context, - env->constants_string(), - tmpl->NewInstance(context).ToLocalChecked()) - .Check(); + target->Set(context, env->constants_string(), constants).Check(); } SetMethodNoSideEffect( From 397662ec87ad634804e2b7d348d66ff5bfb0d9c0 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Wed, 23 Nov 2022 15:37:01 +0900 Subject: [PATCH 3/3] fixup! fixup! src,lib: group properties used as constants from `util` binding Signed-off-by: Daeyeon Jeong --- src/node_util.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_util.cc b/src/node_util.cc index 48769c0ad816c2..ea51c80399224f 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -365,7 +365,7 @@ void Initialize(Local target, Isolate* isolate = env->isolate(); { - Local tmpl = v8::ObjectTemplate::New(isolate); + Local tmpl = ObjectTemplate::New(isolate); #define V(PropertyName, _) \ tmpl->Set(FIXED_ONE_BYTE_STRING(env->isolate(), #PropertyName), \ env->PropertyName()); @@ -381,7 +381,7 @@ void Initialize(Local target, } { - Local constants = Object::New(env->isolate()); + Local constants = Object::New(isolate); #define V(name) \ constants \ ->Set(context, \