Skip to content

Commit

Permalink
src: remove unneeded Environment error methods
Browse files Browse the repository at this point in the history
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: #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>

 Conflicts:
	src/env.h
  • Loading branch information
bnoordhuis authored and Fishrock123 committed Sep 14, 2016
1 parent 6c30ddd commit 1620226
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 57 deletions.
36 changes: 10 additions & 26 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -436,39 +436,23 @@ inline Environment::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,
Expand Down
8 changes: 3 additions & 5 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -473,11 +473,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 =
Expand Down Expand Up @@ -534,6 +529,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);

static const int kIsolateSlot = NODE_ISOLATE_SLOT;

class IsolateData;
Expand Down
50 changes: 24 additions & 26 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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>");
Expand All @@ -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>();
}

Expand All @@ -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>();
}

Expand Down

0 comments on commit 1620226

Please sign in to comment.