From 4bd6caf9f83ef82425a588f5ed5772448b3c3b06 Mon Sep 17 00:00:00 2001
From: Darshan Sen <raisinten@gmail.com>
Date: Thu, 27 Apr 2023 18:07:26 +0530
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/47588#discussion_r1168752918
Signed-off-by: Darshan Sen <raisinten@gmail.com>
---
 src/json_parser.cc | 24 +++++++++++++-----------
 src/json_parser.h  |  5 ++---
 src/node_sea.cc    | 12 ++++++------
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/src/json_parser.cc b/src/json_parser.cc
index a9973c099087e5..f7710e3fd8e11c 100644
--- a/src/json_parser.cc
+++ b/src/json_parser.cc
@@ -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;
@@ -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);
@@ -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);
@@ -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
diff --git a/src/json_parser.h b/src/json_parser.h
index 555f539acf3076..37bd9b4327da12 100644
--- a/src/json_parser.h
+++ b/src/json_parser.h
@@ -4,7 +4,6 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include <memory>
-#include <optional>
 #include <string>
 #include "util.h"
 #include "v8.h"
@@ -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
diff --git a/src/node_sea.cc b/src/node_sea.cc
index c92490508a3d28..ef6b2dd28b006d 100644
--- a/src/node_sea.cc
+++ b/src/node_sea.cc
@@ -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",
@@ -170,7 +170,7 @@ 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",
@@ -178,15 +178,15 @@ std::optional<SeaConfig> ParseSingleExecutableConfig(
     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;
   }