FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: use better return types in KVStore · nodejs/node@4578e94 · GitHub

/ node Public

Commit 4578e94

Browse files
targos
authored and
RafaelGSS
committed
src: use better return types in KVStore
- Use `v8::Maybe<void>` instead of `v8::Maybe<bool>` and handle error from `AssignFromObject`. - An empty `v8::Maybe` is supposed to be returned when an exception is pending. Use `std::optional` instead to indicate a missing value in `Get(key)`. PR-URL: #54539 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
1 parent b5eb24c commit 4578e94

6 files changed

Lines changed: 41 additions & 32 deletions

File tree

‎src/inspector_profiler.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ void StartProfilers(Environment* env) {
472472
}, env);
473473

474474
std::string coverage_str =
475-
env->env_vars()->Get("NODE_V8_COVERAGE").FromMaybe(std::string());
475+
env->env_vars()->Get("NODE_V8_COVERAGE").value_or(std::string());
476476
if (!coverage_str.empty() || env->options()->test_runner_coverage) {
477477
CHECK_NULL(env->coverage_connection());
478478
env->set_coverage_connection(std::make_unique<V8CoverageConnection>(env));

‎src/node_credentials.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,10 @@ bool SafeGetenv(const char* key,
9191
env_vars = per_process::system_environment;
9292
}
9393

94-
return env_vars->Get(key).To(text);
94+
std::optional<std::string> value = env_vars->Get(key);
95+
if (!value.has_value()) return false;
96+
*text = value.value();
97+
return true;
9598
}
9699

97100
static void SafeGetenv(const FunctionCallbackInfo<Value>& args) {

‎src/node_dotenv.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ void Dotenv::SetEnvironment(node::Environment* env) {
5252

5353
auto existing = env->env_vars()->Get(key.data());
5454

55-
if (existing.IsNothing()) {
55+
if (!existing.has_value()) {
5656
env->env_vars()->Set(
5757
isolate,
5858
v8::String::NewFromUtf8(

‎src/node_env_var.cc‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "node_process-inl.h"
77

88
#include <time.h> // tzset(), _tzset()
9+
#include <optional>
910

1011
namespace node {
1112
using v8::Array;
@@ -19,6 +20,7 @@ using v8::Integer;
1920
using v8::Intercepted;
2021
using v8::Isolate;
2122
using v8::Just;
23+
using v8::JustVoid;
2224
using v8::Local;
2325
using v8::Maybe;
2426
using v8::MaybeLocal;
@@ -38,7 +40,7 @@ using v8::Value;
3840
class RealEnvStore final : public KVStore {
3941
public:
4042
MaybeLocal<String> Get(Isolate* isolate, Local<String> key) const override;
41-
Maybe<std::string> Get(const char* key) const override;
43+
std::optional<std::string> Get(const char* key) const override;
4244
void Set(Isolate* isolate, Local<String> key, Local<String> value) override;
4345
int32_t Query(Isolate* isolate, Local<String> key) const override;
4446
int32_t Query(const char* key) const override;
@@ -49,7 +51,7 @@ class RealEnvStore final : public KVStore {
4951
class MapKVStore final : public KVStore {
5052
public:
5153
MaybeLocal<String> Get(Isolate* isolate, Local<String> key) const override;
52-
Maybe<std::string> Get(const char* key) const override;
54+
std::optional<std::string> Get(const char* key) const override;
5355
void Set(Isolate* isolate, Local<String> key, Local<String> value) override;
5456
int32_t Query(Isolate* isolate, Local<String> key) const override;
5557
int32_t Query(const char* key) const override;
@@ -101,7 +103,7 @@ void DateTimeConfigurationChangeNotification(
101103
}
102104
}
103105

104-
Maybe<std::string> RealEnvStore::Get(const char* key) const {
106+
std::optional<std::string> RealEnvStore::Get(const char* key) const {
105107
Mutex::ScopedLock lock(per_process::env_var_mutex);
106108

107109
size_t init_sz = 256;
@@ -116,19 +118,19 @@ Maybe<std::string> RealEnvStore::Get(const char* key) const {
116118
}
117119

118120
if (ret >= 0) { // Env key value fetch success.
119-
return Just(std::string(*val, init_sz));
121+
return std::string(*val, init_sz);
120122
}
121123

122-
return Nothing<std::string>();
124+
return std::nullopt;
123125
}
124126

125127
MaybeLocal<String> RealEnvStore::Get(Isolate* isolate,
126128
Local<String> property) const {
127129
node::Utf8Value key(isolate, property);
128-
Maybe<std::string> value = Get(*key);
130+
std::optional<std::string> value = Get(*key);
129131

130-
if (value.IsJust()) {
131-
std::string val = value.FromJust();
132+
if (value.has_value()) {
133+
std::string val = value.value();
132134
return String::NewFromUtf8(
133135
isolate, val.data(), NewStringType::kNormal, val.size());
134136
}
@@ -229,17 +231,17 @@ std::shared_ptr<KVStore> KVStore::Clone(Isolate* isolate) const {
229231
return copy;
230232
}
231233

232-
Maybe<std::string> MapKVStore::Get(const char* key) const {
234+
std::optional<std::string> MapKVStore::Get(const char* key) const {
233235
Mutex::ScopedLock lock(mutex_);
234236
auto it = map_.find(key);
235-
return it == map_.end() ? Nothing<std::string>() : Just(it->second);
237+
return it == map_.end() ? std::nullopt : std::make_optional(it->second);
236238
}
237239

238240
MaybeLocal<String> MapKVStore::Get(Isolate* isolate, Local<String> key) const {
239241
Utf8Value str(isolate, key);
240-
Maybe<std::string> value = Get(*str);
241-
if (value.IsNothing()) return Local<String>();
242-
std::string val = value.FromJust();
242+
std::optional<std::string> value = Get(*str);
243+
if (!value.has_value()) return MaybeLocal<String>();
244+
std::string val = value.value();
243245
return String::NewFromUtf8(
244246
isolate, val.data(), NewStringType::kNormal, val.size());
245247
}
@@ -291,30 +293,29 @@ std::shared_ptr<KVStore> KVStore::CreateMapKVStore() {
291293
return std::make_shared<MapKVStore>();
292294
}
293295

294-
Maybe<bool> KVStore::AssignFromObject(Local<Context> context,
296+
Maybe<void> KVStore::AssignFromObject(Local<Context> context,
295297
Local<Object> entries) {
296298
Isolate* isolate = context->GetIsolate();
297299
HandleScope handle_scope(isolate);
298300
Local<Array> keys;
299301
if (!entries->GetOwnPropertyNames(context).ToLocal(&keys))
300-
return Nothing<bool>();
302+
return Nothing<void>();
301303
uint32_t keys_length = keys->Length();
302304
for (uint32_t i = 0; i < keys_length; i++) {
303305
Local<Value> key;
304-
if (!keys->Get(context, i).ToLocal(&key))
305-
return Nothing<bool>();
306+
if (!keys->Get(context, i).ToLocal(&key)) return Nothing<void>();
306307
if (!key->IsString()) continue;
307308

308309
Local<Value> value;
309310
Local<String> value_string;
310311
if (!entries->Get(context, key).ToLocal(&value) ||
311312
!value->ToString(context).ToLocal(&value_string)) {
312-
return Nothing<bool>();
313+
return Nothing<void>();
313314
}
314315

315316
Set(isolate, key.As<String>(), value_string);
316317
}
317-
return Just(true);
318+
return JustVoid();
318319
}
319320

320321
// TODO(bnoordhuis) Not super efficient but called infrequently. Not worth

‎src/node_worker.cc‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -531,8 +531,12 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
531531
} else if (args[1]->IsObject()) {
532532
// User provided env.
533533
env_vars = KVStore::CreateMapKVStore();
534-
env_vars->AssignFromObject(isolate->GetCurrentContext(),
535-
args[1].As<Object>());
534+
if (env_vars
535+
->AssignFromObject(isolate->GetCurrentContext(),
536+
args[1].As<Object>())
537+
.IsNothing()) {
538+
return;
539+
}
536540
} else {
537541
// Env is shared.
538542
env_vars = env->env_vars();
@@ -542,21 +546,21 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
542546
per_isolate_opts.reset(new PerIsolateOptions());
543547

544548
HandleEnvOptions(per_isolate_opts->per_env, [&env_vars](const char* name) {
545-
return env_vars->Get(name).FromMaybe("");
549+
return env_vars->Get(name).value_or("");
546550
});
547551

548552
#ifndef NODE_WITHOUT_NODE_OPTIONS
549-
std::string node_options;
550-
if (env_vars->Get("NODE_OPTIONS").To(&node_options)) {
553+
std::optional<std::string> node_options = env_vars->Get("NODE_OPTIONS");
554+
if (node_options.has_value()) {
551555
std::vector<std::string> errors{};
552556
std::vector<std::string> env_argv =
553-
ParseNodeOptionsEnvVar(node_options, &errors);
557+
ParseNodeOptionsEnvVar(node_options.value(), &errors);
554558
// [0] is expected to be the program name, add dummy string.
555559
env_argv.insert(env_argv.begin(), "");
556560
std::vector<std::string> invalid_args{};
557561

558-
std::string parent_node_options;
559-
USE(env->env_vars()->Get("NODE_OPTIONS").To(&parent_node_options));
562+
std::optional<std::string> parent_node_options =
563+
env->env_vars()->Get("NODE_OPTIONS");
560564

561565
// If the worker code passes { env: { ...process.env, ... } } or
562566
// the NODE_OPTIONS is otherwise character-for-character equal to the

‎src/util.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include <bit>
4141
#include <limits>
4242
#include <memory>
43+
#include <optional>
4344
#include <set>
4445
#include <string>
4546
#include <string_view>
@@ -306,7 +307,7 @@ class KVStore {
306307

307308
virtual v8::MaybeLocal<v8::String> Get(v8::Isolate* isolate,
308309
v8::Local<v8::String> key) const = 0;
309-
virtual v8::Maybe<std::string> Get(const char* key) const = 0;
310+
virtual std::optional<std::string> Get(const char* key) const = 0;
310311
virtual void Set(v8::Isolate* isolate,
311312
v8::Local<v8::String> key,
312313
v8::Local<v8::String> value) = 0;
@@ -317,7 +318,7 @@ class KVStore {
317318
virtual v8::Local<v8::Array> Enumerate(v8::Isolate* isolate) const = 0;
318319

319320
virtual std::shared_ptr<KVStore> Clone(v8::Isolate* isolate) const;
320-
virtual v8::Maybe<bool> AssignFromObject(v8::Local<v8::Context> context,
321+
virtual v8::Maybe<void> AssignFromObject(v8::Local<v8::Context> context,
321322
v8::Local<v8::Object> entries);
322323
v8::Maybe<bool> AssignToObject(v8::Isolate* isolate,
323324
v8::Local<v8::Context> context,

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL