Skip to content

Commit 4bd6caf

Browse files
committed
src: do proper Maybe usage
It makes more sense to use a Maybe here because that conveys the meaning that it is unsafe to call into V8 if an exception is pending. Using std::optional does not make that obvious. Refs: nodejs#47588 (comment) Signed-off-by: Darshan Sen <raisinten@gmail.com>
1 parent a18efc1 commit 4bd6caf

File tree

3 files changed

+21
-20
lines changed

3 files changed

+21
-20
lines changed

src/json_parser.cc

+13-11
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ namespace node {
77
using v8::ArrayBuffer;
88
using v8::Context;
99
using v8::Isolate;
10+
using v8::Just;
1011
using v8::Local;
12+
using v8::Maybe;
13+
using v8::Nothing;
1114
using v8::Object;
1215
using v8::String;
1316
using v8::Value;
@@ -58,8 +61,7 @@ bool JSONParser::Parse(const std::string& content) {
5861
return true;
5962
}
6063

61-
std::optional<std::string> JSONParser::GetTopLevelStringField(
62-
std::string_view field) {
64+
Maybe<std::string> JSONParser::GetTopLevelStringField(std::string_view field) {
6365
Isolate* isolate = isolate_.get();
6466
Local<Context> context = context_.Get(isolate);
6567
Local<Object> content_object = content_.Get(isolate);
@@ -69,17 +71,17 @@ std::optional<std::string> JSONParser::GetTopLevelStringField(
6971
isolate, errors::PrinterTryCatch::kDontPrintSourceLine);
7072
Local<Value> field_local;
7173
if (!ToV8Value(context, field, isolate).ToLocal(&field_local)) {
72-
return {};
74+
return Nothing<std::string>();
7375
}
7476
if (!content_object->Get(context, field_local).ToLocal(&value) ||
7577
!value->IsString()) {
76-
return {};
78+
return Nothing<std::string>();
7779
}
7880
Utf8Value utf8_value(isolate, value);
79-
return utf8_value.ToString();
81+
return Just(utf8_value.ToString());
8082
}
8183

82-
std::optional<bool> JSONParser::GetTopLevelBoolField(std::string_view field) {
84+
Maybe<bool> JSONParser::GetTopLevelBoolField(std::string_view field) {
8385
Isolate* isolate = isolate_.get();
8486
Local<Context> context = context_.Get(isolate);
8587
Local<Object> content_object = content_.Get(isolate);
@@ -90,19 +92,19 @@ std::optional<bool> JSONParser::GetTopLevelBoolField(std::string_view field) {
9092
isolate, errors::PrinterTryCatch::kDontPrintSourceLine);
9193
Local<Value> field_local;
9294
if (!ToV8Value(context, field, isolate).ToLocal(&field_local)) {
93-
return {};
95+
return Nothing<bool>();
9496
}
9597
if (!content_object->Has(context, field_local).To(&has_field)) {
96-
return {};
98+
return Nothing<bool>();
9799
}
98100
if (!has_field) {
99-
return false;
101+
return Just(false);
100102
}
101103
if (!content_object->Get(context, field_local).ToLocal(&value) ||
102104
!value->IsBoolean()) {
103-
return {};
105+
return Nothing<bool>();
104106
}
105-
return value->BooleanValue(isolate);
107+
return Just(value->BooleanValue(isolate));
106108
}
107109

108110
} // namespace node

src/json_parser.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
55

66
#include <memory>
7-
#include <optional>
87
#include <string>
98
#include "util.h"
109
#include "v8.h"
@@ -18,8 +17,8 @@ class JSONParser {
1817
JSONParser();
1918
~JSONParser() {}
2019
bool Parse(const std::string& content);
21-
std::optional<std::string> GetTopLevelStringField(std::string_view field);
22-
std::optional<bool> GetTopLevelBoolField(std::string_view field);
20+
v8::Maybe<std::string> GetTopLevelStringField(std::string_view field);
21+
v8::Maybe<bool> GetTopLevelBoolField(std::string_view field);
2322

2423
private:
2524
// We might want a lighter-weight JSON parser for this use case. But for now

src/node_sea.cc

+6-6
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ std::optional<SeaConfig> ParseSingleExecutableConfig(
161161
}
162162

163163
result.main_path =
164-
parser.GetTopLevelStringField("main").value_or(std::string());
164+
parser.GetTopLevelStringField("main").FromMaybe(std::string());
165165
if (result.main_path.empty()) {
166166
FPrintF(stderr,
167167
"\"main\" field of %s is not a non-empty string\n",
@@ -170,23 +170,23 @@ std::optional<SeaConfig> ParseSingleExecutableConfig(
170170
}
171171

172172
result.output_path =
173-
parser.GetTopLevelStringField("output").value_or(std::string());
173+
parser.GetTopLevelStringField("output").FromMaybe(std::string());
174174
if (result.output_path.empty()) {
175175
FPrintF(stderr,
176176
"\"output\" field of %s is not a non-empty string\n",
177177
config_path);
178178
return std::nullopt;
179179
}
180180

181-
std::optional<bool> disable_experimental_sea_warning =
182-
parser.GetTopLevelBoolField("disableExperimentalSEAWarning");
183-
if (!disable_experimental_sea_warning.has_value()) {
181+
bool disable_experimental_sea_warning;
182+
if (!parser.GetTopLevelBoolField("disableExperimentalSEAWarning")
183+
.To(&disable_experimental_sea_warning)) {
184184
FPrintF(stderr,
185185
"\"disableExperimentalSEAWarning\" field of %s is not a Boolean\n",
186186
config_path);
187187
return std::nullopt;
188188
}
189-
if (disable_experimental_sea_warning.value()) {
189+
if (disable_experimental_sea_warning) {
190190
result.flags |= SeaFlags::kDisableExperimentalSeaWarning;
191191
}
192192

0 commit comments

Comments
 (0)