Repository: nodejs/node
Pull request: src: add missing qualifiers to env.cc

Adds several missing `const` qualifiers and replaces `std::vector` usage with `v8::LocalVector<Value>` for a function used for testing.

FYI: `v8::LocalVector` is recommended to be used by the V8 team when storing them on heap. Ex: https://groups.google.com/g/v8-reviews/c/gM-dP-mASPI?pli=1

Changed files:
filename: src/env.cc
patch: @@ -87,13 +87,13 @@ void AsyncHooks::ResetPromiseHooks(Local<Function> init,
   js_promise_hooks_[3].Reset(env()->isolate(), resolve);
 }
 
-Local<Array> AsyncHooks::GetPromiseHooks(Isolate* isolate) {
-  std::vector<Local<Value>> values;
+Local<Array> AsyncHooks::GetPromiseHooks(Isolate* isolate) const {
+  v8::LocalVector<Value> values(isolate, js_promise_hooks_.size());
   for (size_t i = 0; i < js_promise_hooks_.size(); ++i) {
     if (js_promise_hooks_[i].IsEmpty()) {
-      values.push_back(Undefined(isolate));
+      values[i] = Undefined(isolate);
     } else {
-      values.push_back(js_promise_hooks_[i].Get(isolate));
+      values[i] = js_promise_hooks_[i].Get(isolate);
     }
   }
   return Array::New(isolate, values.data(), values.size());
@@ -236,13 +236,10 @@ void Environment::TrackContext(Local<Context> context) {
 
 void Environment::UntrackContext(Local<Context> context) {
   HandleScope handle_scope(isolate_);
-  contexts_.erase(std::remove_if(contexts_.begin(),
-                                 contexts_.end(),
-                                 [&](auto&& el) { return el.IsEmpty(); }),
-                  contexts_.end());
+  std::erase_if(contexts_, [&](auto&& el) { return el.IsEmpty(); });
   for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
-    Local<Context> saved_context = PersistentToLocal::Weak(isolate_, *it);
-    if (saved_context == context) {
+    if (Local<Context> saved_context = PersistentToLocal::Weak(isolate_, *it);
+        saved_context == context) {
       it->Reset();
       contexts_.erase(it);
       break;
@@ -351,9 +348,11 @@ IsolateDataSerializeInfo IsolateData::Serialize(SnapshotCreator* creator) {
 #undef VS
 #undef VP
 
-  for (size_t i = 0; i < AsyncWrap::PROVIDERS_LENGTH; i++)
+  info.primitive_values.reserve(info.primitive_values.size() +
+                                AsyncWrap::PROVIDERS_LENGTH);
+  for (size_t i = 0; i < AsyncWrap::PROVIDERS_LENGTH; i++) {
     info.primitive_values.push_back(creator->AddData(async_wrap_provider(i)));
-
+  }
   uint32_t id = 0;
 #define VM(PropertyName) V(PropertyName##_binding_template, ObjectTemplate)
 #define V(PropertyName, TypeName)                                              \
@@ -375,7 +374,7 @@ IsolateDataSerializeInfo IsolateData::Serialize(SnapshotCreator* creator) {
 void IsolateData::DeserializeProperties(const IsolateDataSerializeInfo* info) {
   size_t i = 0;
 
-  v8::Isolate::Scope isolate_scope(isolate_);
+  Isolate::Scope isolate_scope(isolate_);
   HandleScope handle_scope(isolate_);
 
   if (per_process::enabled_debug_list.enabled(DebugCategory::MKSNAPSHOT)) {
@@ -721,9 +720,8 @@ void Environment::TryLoadAddon(
 std::string Environment::GetCwd(const std::string& exec_path) {
   char cwd[PATH_MAX_BYTES];
   size_t size = PATH_MAX_BYTES;
-  const int err = uv_cwd(cwd, &size);
 
-  if (err == 0) {
+  if (uv_cwd(cwd, &size) == 0) {
     CHECK_GT(size, 0);
     return cwd;
   }
@@ -769,7 +767,7 @@ std::string Environment::GetExecPath(const std::vector<std::string>& argv) {
   std::string exec_path;
   if (uv_exepath(exec_path_buf, &exec_path_len) == 0) {
     exec_path = std::string(exec_path_buf, exec_path_len);
-  } else if (argv.size() > 0) {
+  } else if (!argv.empty()) {
     exec_path = argv[0];
   }
 
@@ -1239,7 +1237,8 @@ void Environment::StartProfilerIdleNotifier() {
 }
 
 void Environment::PrintSyncTrace() const {
-  if (!trace_sync_io_) return;
+  if (!trace_sync_io_) [[likely]]
+    return;
 
   HandleScope handle_scope(isolate());
 
@@ -1314,7 +1313,7 @@ void Environment::AtExit(void (*cb)(void* arg), void* arg) {
   at_exit_functions_.push_front(ExitCallback{cb, arg});
 }
 
-Maybe<bool> Environment::CheckUnsettledTopLevelAwait() {
+Maybe<bool> Environment::CheckUnsettledTopLevelAwait() const {
   HandleScope scope(isolate_);
   Local<Context> ctx = context();
   Local<Value> value;
@@ -1516,7 +1515,7 @@ void Environment::RunTimers(uv_timer_t* handle) {
   int64_t expiry_ms =
       ret.ToLocalChecked()->IntegerValue(env->context()).FromJust();
 
-  uv_handle_t* h = reinterpret_cast<uv_handle_t*>(handle);
+  auto* h = reinterpret_cast<uv_handle_t*>(handle);
 
   if (expiry_ms != 0) {
     int64_t duration_ms =
@@ -1582,8 +1581,7 @@ Local<Value> Environment::GetNow() {
   uint64_t now = GetNowUint64();
   if (now <= 0xffffffff)
     return Integer::NewFromUnsigned(isolate(), static_cast<uint32_t>(now));
-  else
-    return Number::New(isolate(), static_cast<double>(now));
+  return Number::New(isolate(), static_cast<double>(now));
 }
 
 void CollectExceptionInfo(Environment* env,
@@ -1642,8 +1640,8 @@ void Environment::CollectUVExceptionInfo(Local<Value> object,
     message = uv_strerror(errorno);
   }
 
-  node::CollectExceptionInfo(this, obj, errorno, err_string,
-                             syscall, message, path, dest);
+  CollectExceptionInfo(
+      this, obj, errorno, err_string, syscall, message, path, dest);
 }
 
 ImmediateInfo::ImmediateInfo(Isolate* isolate, const SerializeInfo* info)
@@ -1973,7 +1971,7 @@ void Environment::BuildEmbedderGraph(Isolate* isolate,
                                      EmbedderGraph* graph,
                                      void* data) {
   MemoryTracker tracker(isolate, graph);
-  Environment* env = static_cast<Environment*>(data);
+  auto* env = static_cast<Environment*>(data);
   // Start traversing embedder objects from the root Environment object.
   tracker.Track(env);
 }
@@ -2035,7 +2033,7 @@ void Environment::TracePromises(PromiseHookType type,
 size_t Environment::NearHeapLimitCallback(void* data,
                                           size_t current_heap_limit,
                                           size_t initial_heap_limit) {
-  Environment* env = static_cast<Environment*>(data);
+  auto* env = static_cast<Environment*>(data);
 
   Debug(env,
         DebugCategory::DIAGNOSTICS,
@@ -2081,8 +2079,8 @@ size_t Environment::NearHeapLimitCallback(void* data,
         DebugCategory::DIAGNOSTICS,
         "Estimated available memory=%" PRIu64 ", "
         "estimated overhead=%" PRIu64 "\n",
-        static_cast<uint64_t>(available),
-        static_cast<uint64_t>(estimated_overhead));
+        available,
+        estimated_overhead);
 
   // This might be hit when the snapshot is being taken in another
   // NearHeapLimitCallback invocation.

filename: src/env.h
patch: @@ -329,7 +329,7 @@ class AsyncHooks : public MemoryRetainer {
                          v8::Local<v8::Function> resolve);
   // Used for testing since V8 doesn't provide API for retrieving configured
   // JS promise hooks.
-  v8::Local<v8::Array> GetPromiseHooks(v8::Isolate* isolate);
+  v8::Local<v8::Array> GetPromiseHooks(v8::Isolate* isolate) const;
   inline v8::Local<v8::String> provider_string(int idx);
 
   inline void no_force_checks();
@@ -848,7 +848,7 @@ class Environment final : public MemoryRetainer {
   void AtExit(void (*cb)(void* arg), void* arg);
   void RunAtExitCallbacks();
 
-  v8::Maybe<bool> CheckUnsettledTopLevelAwait();
+  v8::Maybe<bool> CheckUnsettledTopLevelAwait() const;
   void RunWeakRefCleanup();
 
   v8::MaybeLocal<v8::Value> RunSnapshotSerializeCallback() const;