Skip to content

Commit

Permalink
src: do proper Maybe usage
Browse files Browse the repository at this point in the history
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: #47588 (comment)
Signed-off-by: Darshan Sen <raisinten@gmail.com>
  • Loading branch information
RaisinTen committed Apr 27, 2023
1 parent a18efc1 commit 4bd6caf
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 20 deletions.
24 changes: 13 additions & 11 deletions src/json_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ namespace node {
using v8::ArrayBuffer;
using v8::Context;
using v8::Isolate;
using v8::Just;
using v8::Local;
using v8::Maybe;
using v8::Nothing;
using v8::Object;
using v8::String;
using v8::Value;
Expand Down Expand Up @@ -58,8 +61,7 @@ bool JSONParser::Parse(const std::string& content) {
return true;
}

std::optional<std::string> JSONParser::GetTopLevelStringField(
std::string_view field) {
Maybe<std::string> JSONParser::GetTopLevelStringField(std::string_view field) {
Isolate* isolate = isolate_.get();
Local<Context> context = context_.Get(isolate);
Local<Object> content_object = content_.Get(isolate);
Expand All @@ -69,17 +71,17 @@ std::optional<std::string> JSONParser::GetTopLevelStringField(
isolate, errors::PrinterTryCatch::kDontPrintSourceLine);
Local<Value> field_local;
if (!ToV8Value(context, field, isolate).ToLocal(&field_local)) {
return {};
return Nothing<std::string>();
}
if (!content_object->Get(context, field_local).ToLocal(&value) ||
!value->IsString()) {
return {};
return Nothing<std::string>();
}
Utf8Value utf8_value(isolate, value);
return utf8_value.ToString();
return Just(utf8_value.ToString());
}

std::optional<bool> JSONParser::GetTopLevelBoolField(std::string_view field) {
Maybe<bool> JSONParser::GetTopLevelBoolField(std::string_view field) {
Isolate* isolate = isolate_.get();
Local<Context> context = context_.Get(isolate);
Local<Object> content_object = content_.Get(isolate);
Expand All @@ -90,19 +92,19 @@ std::optional<bool> JSONParser::GetTopLevelBoolField(std::string_view field) {
isolate, errors::PrinterTryCatch::kDontPrintSourceLine);
Local<Value> field_local;
if (!ToV8Value(context, field, isolate).ToLocal(&field_local)) {
return {};
return Nothing<bool>();
}
if (!content_object->Has(context, field_local).To(&has_field)) {
return {};
return Nothing<bool>();
}
if (!has_field) {
return false;
return Just(false);
}
if (!content_object->Get(context, field_local).ToLocal(&value) ||
!value->IsBoolean()) {
return {};
return Nothing<bool>();
}
return value->BooleanValue(isolate);
return Just(value->BooleanValue(isolate));
}

} // namespace node
5 changes: 2 additions & 3 deletions src/json_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include <memory>
#include <optional>
#include <string>
#include "util.h"
#include "v8.h"
Expand All @@ -18,8 +17,8 @@ class JSONParser {
JSONParser();
~JSONParser() {}
bool Parse(const std::string& content);
std::optional<std::string> GetTopLevelStringField(std::string_view field);
std::optional<bool> GetTopLevelBoolField(std::string_view field);
v8::Maybe<std::string> GetTopLevelStringField(std::string_view field);
v8::Maybe<bool> GetTopLevelBoolField(std::string_view field);

private:
// We might want a lighter-weight JSON parser for this use case. But for now
Expand Down
12 changes: 6 additions & 6 deletions src/node_sea.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ std::optional<SeaConfig> ParseSingleExecutableConfig(
}

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

result.output_path =
parser.GetTopLevelStringField("output").value_or(std::string());
parser.GetTopLevelStringField("output").FromMaybe(std::string());
if (result.output_path.empty()) {
FPrintF(stderr,
"\"output\" field of %s is not a non-empty string\n",
config_path);
return std::nullopt;
}

std::optional<bool> disable_experimental_sea_warning =
parser.GetTopLevelBoolField("disableExperimentalSEAWarning");
if (!disable_experimental_sea_warning.has_value()) {
bool disable_experimental_sea_warning;
if (!parser.GetTopLevelBoolField("disableExperimentalSEAWarning")
.To(&disable_experimental_sea_warning)) {
FPrintF(stderr,
"\"disableExperimentalSEAWarning\" field of %s is not a Boolean\n",
config_path);
return std::nullopt;
}
if (disable_experimental_sea_warning.value()) {
if (disable_experimental_sea_warning) {
result.flags |= SeaFlags::kDisableExperimentalSeaWarning;
}

Expand Down

0 comments on commit 4bd6caf

Please sign in to comment.