src: remove unneeded Environment error methods

They seem to have been introduced as "convenience methods" in commit
75adde0 ("src: remove `node_isolate` from source") for reasons I can
only guess at but they can be removed without much hassle.

PR-URL: https://github.com/nodejs/node/pull/8427
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

3 files changed, 37 insertions, 57 deletions

diff --git a/src/env-inl.h b/src/env-inl.h
index bdd3a0585c..612b3c48af 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -356,39 +356,23 @@ inline IsolateData* Environment::isolate_data() const {
   return isolate_data_;
 }
 
-// this would have been a template function were it not for the fact that g++
-// sometimes fails to resolve it...
-#define THROW_ERROR(fun)                                                      \
-  do {                                                                        \
-    v8::HandleScope scope(isolate);                                           \
-    isolate->ThrowException(fun(OneByteString(isolate, errmsg)));             \
-  }                                                                           \
-  while (0)
-
-inline void Environment::ThrowError(v8::Isolate* isolate, const char* errmsg) {
-  THROW_ERROR(v8::Exception::Error);
-}
-
-inline void Environment::ThrowTypeError(v8::Isolate* isolate,
-                                        const char* errmsg) {
-  THROW_ERROR(v8::Exception::TypeError);
-}
-
-inline void Environment::ThrowRangeError(v8::Isolate* isolate,
-                                         const char* errmsg) {
-  THROW_ERROR(v8::Exception::RangeError);
-}
-
 inline void Environment::ThrowError(const char* errmsg) {
-  ThrowError(isolate(), errmsg);
+  ThrowError(v8::Exception::Error, errmsg);
 }
 
 inline void Environment::ThrowTypeError(const char* errmsg) {
-  ThrowTypeError(isolate(), errmsg);
+  ThrowError(v8::Exception::TypeError, errmsg);
 }
 
 inline void Environment::ThrowRangeError(const char* errmsg) {
-  ThrowRangeError(isolate(), errmsg);
+  ThrowError(v8::Exception::RangeError, errmsg);
+}
+
+inline void Environment::ThrowError(
+    v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
+    const char* errmsg) {
+  v8::HandleScope handle_scope(isolate());
+  isolate()->ThrowException(fun(OneByteString(isolate(), errmsg)));
 }
 
 inline void Environment::ThrowErrnoException(int errorno,
diff --git a/src/env.h b/src/env.h
index 0e301fb274..e715095f1b 100644
--- a/src/env.h
+++ b/src/env.h
@@ -485,11 +485,6 @@ class Environment {
                                const char* path = nullptr,
                                const char* dest = nullptr);
 
-  // Convenience methods for contextify
-  inline static void ThrowError(v8::Isolate* isolate, const char* errmsg);
-  inline static void ThrowTypeError(v8::Isolate* isolate, const char* errmsg);
-  inline static void ThrowRangeError(v8::Isolate* isolate, const char* errmsg);
-
   inline v8::Local<v8::FunctionTemplate>
       NewFunctionTemplate(v8::FunctionCallback callback,
                           v8::Local<v8::Signature> signature =
@@ -546,6 +541,9 @@ class Environment {
   static const int kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX;
 
  private:
+  inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
+                         const char* errmsg);
+
   v8::Isolate* const isolate_;
   IsolateData* const isolate_data_;
   uv_check_t immediate_check_handle_;
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 091b2a5ee3..3262b15c7b 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -485,10 +485,10 @@ class ContextifyScript : public BaseObject {
 
     TryCatch try_catch(env->isolate());
     Local<String> code = args[0]->ToString(env->isolate());
-    Local<String> filename = GetFilenameArg(args, 1);
+    Local<String> filename = GetFilenameArg(env, args, 1);
     Local<Integer> lineOffset = GetLineOffsetArg(args, 1);
     Local<Integer> columnOffset = GetColumnOffsetArg(args, 1);
-    bool display_errors = GetDisplayErrorsArg(args, 1);
+    bool display_errors = GetDisplayErrorsArg(env, args, 1);
     MaybeLocal<Uint8Array> cached_data_buf = GetCachedData(env, args, 1);
     bool produce_cached_data = GetProduceCachedData(env, args, 1);
     if (try_catch.HasCaught()) {
@@ -559,18 +559,19 @@ class ContextifyScript : public BaseObject {
 
   // args: [options]
   static void RunInThisContext(const FunctionCallbackInfo<Value>& args) {
+    Environment* env = Environment::GetCurrent(args);
+
     // Assemble arguments
     TryCatch try_catch(args.GetIsolate());
-    uint64_t timeout = GetTimeoutArg(args, 0);
-    bool display_errors = GetDisplayErrorsArg(args, 0);
-    bool break_on_sigint = GetBreakOnSigintArg(args, 0);
+    uint64_t timeout = GetTimeoutArg(env, args, 0);
+    bool display_errors = GetDisplayErrorsArg(env, args, 0);
+    bool break_on_sigint = GetBreakOnSigintArg(env, args, 0);
     if (try_catch.HasCaught()) {
       try_catch.ReThrow();
       return;
     }
 
     // Do the eval within this context
-    Environment* env = Environment::GetCurrent(args);
     EvalMachine(env, timeout, display_errors, break_on_sigint, args,
                 &try_catch);
   }
@@ -592,9 +593,9 @@ class ContextifyScript : public BaseObject {
     Local<Object> sandbox = args[0].As<Object>();
     {
       TryCatch try_catch(env->isolate());
-      timeout = GetTimeoutArg(args, 1);
-      display_errors = GetDisplayErrorsArg(args, 1);
-      break_on_sigint = GetBreakOnSigintArg(args, 1);
+      timeout = GetTimeoutArg(env, args, 1);
+      display_errors = GetDisplayErrorsArg(env, args, 1);
+      break_on_sigint = GetBreakOnSigintArg(env, args, 1);
       if (try_catch.HasCaught()) {
         try_catch.ReThrow();
         return;
@@ -668,14 +669,14 @@ class ContextifyScript : public BaseObject {
         True(env->isolate()));
   }
 
-  static bool GetBreakOnSigintArg(const FunctionCallbackInfo<Value>& args,
+  static bool GetBreakOnSigintArg(Environment* env,
+                                  const FunctionCallbackInfo<Value>& args,
                                   const int i) {
     if (args[i]->IsUndefined() || args[i]->IsString()) {
       return false;
     }
     if (!args[i]->IsObject()) {
-      Environment::ThrowTypeError(args.GetIsolate(),
-                                  "options must be an object");
+      env->ThrowTypeError("options must be an object");
       return false;
     }
 
@@ -685,14 +686,14 @@ class ContextifyScript : public BaseObject {
     return value->IsTrue();
   }
 
-  static int64_t GetTimeoutArg(const FunctionCallbackInfo<Value>& args,
+  static int64_t GetTimeoutArg(Environment* env,
+                               const FunctionCallbackInfo<Value>& args,
                                const int i) {
     if (args[i]->IsUndefined() || args[i]->IsString()) {
       return -1;
     }
     if (!args[i]->IsObject()) {
-      Environment::ThrowTypeError(args.GetIsolate(),
-                                  "options must be an object");
+      env->ThrowTypeError("options must be an object");
       return -1;
     }
 
@@ -704,22 +705,21 @@ class ContextifyScript : public BaseObject {
     int64_t timeout = value->IntegerValue();
 
     if (timeout <= 0) {
-      Environment::ThrowRangeError(args.GetIsolate(),
-                                   "timeout must be a positive number");
+      env->ThrowRangeError("timeout must be a positive number");
       return -1;
     }
     return timeout;
   }
 
 
-  static bool GetDisplayErrorsArg(const FunctionCallbackInfo<Value>& args,
+  static bool GetDisplayErrorsArg(Environment* env,
+                                  const FunctionCallbackInfo<Value>& args,
                                   const int i) {
     if (args[i]->IsUndefined() || args[i]->IsString()) {
       return true;
     }
     if (!args[i]->IsObject()) {
-      Environment::ThrowTypeError(args.GetIsolate(),
-                                  "options must be an object");
+      env->ThrowTypeError("options must be an object");
       return false;
     }
 
@@ -731,7 +731,8 @@ class ContextifyScript : public BaseObject {
   }
 
 
-  static Local<String> GetFilenameArg(const FunctionCallbackInfo<Value>& args,
+  static Local<String> GetFilenameArg(Environment* env,
+                                      const FunctionCallbackInfo<Value>& args,
                                       const int i) {
     Local<String> defaultFilename =
         FIXED_ONE_BYTE_STRING(args.GetIsolate(), "evalmachine.<anonymous>");
@@ -743,8 +744,7 @@ class ContextifyScript : public BaseObject {
       return args[i].As<String>();
     }
     if (!args[i]->IsObject()) {
-      Environment::ThrowTypeError(args.GetIsolate(),
-                                  "options must be an object");
+      env->ThrowTypeError("options must be an object");
       return Local<String>();
     }
 
@@ -770,9 +770,7 @@ class ContextifyScript : public BaseObject {
     }
 
     if (!value->IsUint8Array()) {
-      Environment::ThrowTypeError(
-          args.GetIsolate(),
-          "options.cachedData must be a Buffer instance");
+      env->ThrowTypeError("options.cachedData must be a Buffer instance");
       return MaybeLocal<Uint8Array>();
     }