From f80cae8be5b4ff694e61e0b00a4a8329994714fe Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 14 Feb 2022 00:44:30 +0100 Subject: [PATCH] src: merge ToJsSet into ToV8Value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This addresses a `TODO` comment, and makes use of the opportunity to also clean up our `MaybeLocal` handling in this area and start accepting `std::string_view` where we accept `std::string`. PR-URL: https://github.com/nodejs/node/pull/41757 Reviewed-By: Darshan Sen Reviewed-By: James M Snell Reviewed-By: Tobias Nießen --- src/node_native_module_env.cc | 75 ++++++++++++++++++++++------------- src/util-inl.h | 21 +++++++++- src/util.h | 8 +++- 3 files changed, 75 insertions(+), 29 deletions(-) diff --git a/src/node_native_module_env.cc b/src/node_native_module_env.cc index b59ed7cb75cf67..b388b8cd07e583 100644 --- a/src/node_native_module_env.cc +++ b/src/node_native_module_env.cc @@ -22,17 +22,6 @@ using v8::SideEffectType; using v8::String; using v8::Value; -// TODO(joyeecheung): make these more general and put them into util.h -Local ToJsSet(Local context, const std::set& in) { - Isolate* isolate = context->GetIsolate(); - Local out = Set::New(isolate); - for (auto const& x : in) { - out->Add(context, OneByteString(isolate, x.c_str(), x.size())) - .ToLocalChecked(); - } - return out; -} - bool NativeModuleEnv::Add(const char* id, const UnionBytes& source) { return NativeModuleLoader::GetInstance()->Add(id, source); } @@ -67,16 +56,26 @@ void NativeModuleEnv::GetModuleCategories( cannot_be_required.insert("trace_events"); } - result + Local cannot_be_required_js; + Local can_be_required_js; + + if (!ToV8Value(context, cannot_be_required).ToLocal(&cannot_be_required_js)) + return; + if (result ->Set(context, OneByteString(isolate, "cannotBeRequired"), - ToJsSet(context, cannot_be_required)) - .FromJust(); - result + cannot_be_required_js) + .IsNothing()) + return; + if (!ToV8Value(context, can_be_required).ToLocal(&can_be_required_js)) + return; + if (result ->Set(context, OneByteString(isolate, "canBeRequired"), - ToJsSet(context, can_be_required)) - .FromJust(); + can_be_required_js) + .IsNothing()) { + return; + } info.GetReturnValue().Set(result); } @@ -85,23 +84,45 @@ void NativeModuleEnv::GetCacheUsage(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); Local context = env->context(); Local result = Object::New(isolate); - result + + Local native_modules_with_cache_js; + Local native_modules_without_cache_js; + Local native_modules_in_snapshot_js; + if (!ToV8Value(context, env->native_modules_with_cache) + .ToLocal(&native_modules_with_cache_js)) { + return; + } + if (result ->Set(env->context(), OneByteString(isolate, "compiledWithCache"), - ToJsSet(context, env->native_modules_with_cache)) - .FromJust(); - result + native_modules_with_cache_js) + .IsNothing()) { + return; + } + + if (!ToV8Value(context, env->native_modules_without_cache) + .ToLocal(&native_modules_without_cache_js)) { + return; + } + if (result ->Set(env->context(), OneByteString(isolate, "compiledWithoutCache"), - ToJsSet(context, env->native_modules_without_cache)) - .FromJust(); + native_modules_without_cache_js) + .IsNothing()) { + return; + } - result + if (!ToV8Value(context, env->native_modules_in_snapshot) + .ToLocal(&native_modules_without_cache_js)) { + return; + } + if (result ->Set(env->context(), OneByteString(isolate, "compiledInSnapshot"), - ToV8Value(env->context(), env->native_modules_in_snapshot) - .ToLocalChecked()) - .FromJust(); + native_modules_without_cache_js) + .IsNothing()) { + return; + } args.GetReturnValue().Set(result); } diff --git a/src/util-inl.h b/src/util-inl.h index b860c7008a340a..327893618525f8 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -404,7 +404,7 @@ inline char* UncheckedCalloc(size_t n) { return UncheckedCalloc(n); } void ThrowErrStringTooLong(v8::Isolate* isolate); v8::MaybeLocal ToV8Value(v8::Local context, - const std::string& str, + std::string_view str, v8::Isolate* isolate) { if (isolate == nullptr) isolate = context->GetIsolate(); if (UNLIKELY(str.size() >= static_cast(v8::String::kMaxLength))) { @@ -436,6 +436,25 @@ v8::MaybeLocal ToV8Value(v8::Local context, return handle_scope.Escape(v8::Array::New(isolate, arr.out(), arr.length())); } +template +v8::MaybeLocal ToV8Value(v8::Local context, + const std::set& set, + v8::Isolate* isolate) { + if (isolate == nullptr) isolate = context->GetIsolate(); + v8::Local set_js = v8::Set::New(isolate); + v8::HandleScope handle_scope(isolate); + + for (const T& entry : set) { + v8::Local value; + if (!ToV8Value(context, entry, isolate).ToLocal(&value)) + return {}; + if (set_js->Add(context, value).IsEmpty()) + return {}; + } + + return set_js; +} + template v8::MaybeLocal ToV8Value(v8::Local context, const std::unordered_map& map, diff --git a/src/util.h b/src/util.h index 06c67d273ba96b..8c21c53a1346c0 100644 --- a/src/util.h +++ b/src/util.h @@ -36,7 +36,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -644,7 +646,7 @@ using DeleteFnPtr = typename FunctionDeleter::Pointer; std::vector SplitString(const std::string& in, char delim); inline v8::MaybeLocal ToV8Value(v8::Local context, - const std::string& str, + std::string_view str, v8::Isolate* isolate = nullptr); template ::is_specialized, bool>::type> @@ -655,6 +657,10 @@ template inline v8::MaybeLocal ToV8Value(v8::Local context, const std::vector& vec, v8::Isolate* isolate = nullptr); +template +inline v8::MaybeLocal ToV8Value(v8::Local context, + const std::set& set, + v8::Isolate* isolate = nullptr); template inline v8::MaybeLocal ToV8Value(v8::Local context, const std::unordered_map& map,