Skip to content

Commit 0e6c336

Browse files
bnoordhuisbnoordhuis
committed
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: #8427 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent e57ff45 commit 0e6c336

File tree

3 files changed

+37
-57
lines changed

3 files changed

+37
-57
lines changed

‎src/env-inl.h‎

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -356,39 +356,23 @@ inline IsolateData* Environment::isolate_data() const {
356356
return isolate_data_;
357357
}
358358

359-
// this would have been a template function were it not for the fact that g++
360-
// sometimes fails to resolve it...
361-
#define THROW_ERROR(fun) \
362-
do { \
363-
v8::HandleScope scope(isolate); \
364-
isolate->ThrowException(fun(OneByteString(isolate, errmsg))); \
365-
} \
366-
while (0)
367-
368-
inline void Environment::ThrowError(v8::Isolate* isolate, const char* errmsg) {
369-
THROW_ERROR(v8::Exception::Error);
370-
}
371-
372-
inline void Environment::ThrowTypeError(v8::Isolate* isolate,
373-
const char* errmsg) {
374-
THROW_ERROR(v8::Exception::TypeError);
375-
}
376-
377-
inline void Environment::ThrowRangeError(v8::Isolate* isolate,
378-
const char* errmsg) {
379-
THROW_ERROR(v8::Exception::RangeError);
380-
}
381-
382359
inline void Environment::ThrowError(const char* errmsg) {
383-
ThrowError(isolate(), errmsg);
360+
ThrowError(v8::Exception::Error, errmsg);
384361
}
385362

386363
inline void Environment::ThrowTypeError(const char* errmsg) {
387-
ThrowTypeError(isolate(), errmsg);
364+
ThrowError(v8::Exception::TypeError, errmsg);
388365
}
389366

390367
inline void Environment::ThrowRangeError(const char* errmsg) {
391-
ThrowRangeError(isolate(), errmsg);
368+
ThrowError(v8::Exception::RangeError, errmsg);
369+
}
370+
371+
inline void Environment::ThrowError(
372+
v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
373+
const char* errmsg) {
374+
v8::HandleScope handle_scope(isolate());
375+
isolate()->ThrowException(fun(OneByteString(isolate(), errmsg)));
392376
}
393377

394378
inline void Environment::ThrowErrnoException(int errorno,

‎src/env.h‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -485,11 +485,6 @@ class Environment {
485485
const char* path = nullptr,
486486
const char* dest = nullptr);
487487

488-
// Convenience methods for contextify
489-
inline static void ThrowError(v8::Isolate* isolate, const char* errmsg);
490-
inline static void ThrowTypeError(v8::Isolate* isolate, const char* errmsg);
491-
inline static void ThrowRangeError(v8::Isolate* isolate, const char* errmsg);
492-
493488
inline v8::Local<v8::FunctionTemplate>
494489
NewFunctionTemplate(v8::FunctionCallback callback,
495490
v8::Local<v8::Signature> signature =
@@ -546,6 +541,9 @@ class Environment {
546541
static const int kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX;
547542

548543
private:
544+
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
545+
const char* errmsg);
546+
549547
v8::Isolate* const isolate_;
550548
IsolateData* const isolate_data_;
551549
uv_check_t immediate_check_handle_;

‎src/node_contextify.cc‎

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -485,10 +485,10 @@ class ContextifyScript : public BaseObject {
485485

486486
TryCatch try_catch(env->isolate());
487487
Local<String> code = args[0]->ToString(env->isolate());
488-
Local<String> filename = GetFilenameArg(args, 1);
488+
Local<String> filename = GetFilenameArg(env, args, 1);
489489
Local<Integer> lineOffset = GetLineOffsetArg(args, 1);
490490
Local<Integer> columnOffset = GetColumnOffsetArg(args, 1);
491-
bool display_errors = GetDisplayErrorsArg(args, 1);
491+
bool display_errors = GetDisplayErrorsArg(env, args, 1);
492492
MaybeLocal<Uint8Array> cached_data_buf = GetCachedData(env, args, 1);
493493
bool produce_cached_data = GetProduceCachedData(env, args, 1);
494494
if (try_catch.HasCaught()) {
@@ -559,18 +559,19 @@ class ContextifyScript : public BaseObject {
559559

560560
// args: [options]
561561
static void RunInThisContext(const FunctionCallbackInfo<Value>& args) {
562+
Environment* env = Environment::GetCurrent(args);
563+
562564
// Assemble arguments
563565
TryCatch try_catch(args.GetIsolate());
564-
uint64_t timeout = GetTimeoutArg(args, 0);
565-
bool display_errors = GetDisplayErrorsArg(args, 0);
566-
bool break_on_sigint = GetBreakOnSigintArg(args, 0);
566+
uint64_t timeout = GetTimeoutArg(env, args, 0);
567+
bool display_errors = GetDisplayErrorsArg(env, args, 0);
568+
bool break_on_sigint = GetBreakOnSigintArg(env, args, 0);
567569
if (try_catch.HasCaught()) {
568570
try_catch.ReThrow();
569571
return;
570572
}
571573

572574
// Do the eval within this context
573-
Environment* env = Environment::GetCurrent(args);
574575
EvalMachine(env, timeout, display_errors, break_on_sigint, args,
575576
&try_catch);
576577
}
@@ -592,9 +593,9 @@ class ContextifyScript : public BaseObject {
592593
Local<Object> sandbox = args[0].As<Object>();
593594
{
594595
TryCatch try_catch(env->isolate());
595-
timeout = GetTimeoutArg(args, 1);
596-
display_errors = GetDisplayErrorsArg(args, 1);
597-
break_on_sigint = GetBreakOnSigintArg(args, 1);
596+
timeout = GetTimeoutArg(env, args, 1);
597+
display_errors = GetDisplayErrorsArg(env, args, 1);
598+
break_on_sigint = GetBreakOnSigintArg(env, args, 1);
598599
if (try_catch.HasCaught()) {
599600
try_catch.ReThrow();
600601
return;
@@ -668,14 +669,14 @@ class ContextifyScript : public BaseObject {
668669
True(env->isolate()));
669670
}
670671

671-
static bool GetBreakOnSigintArg(const FunctionCallbackInfo<Value>& args,
672+
static bool GetBreakOnSigintArg(Environment* env,
673+
const FunctionCallbackInfo<Value>& args,
672674
const int i) {
673675
if (args[i]->IsUndefined() || args[i]->IsString()) {
674676
return false;
675677
}
676678
if (!args[i]->IsObject()) {
677-
Environment::ThrowTypeError(args.GetIsolate(),
678-
"options must be an object");
679+
env->ThrowTypeError("options must be an object");
679680
return false;
680681
}
681682

@@ -685,14 +686,14 @@ class ContextifyScript : public BaseObject {
685686
return value->IsTrue();
686687
}
687688

688-
static int64_t GetTimeoutArg(const FunctionCallbackInfo<Value>& args,
689+
static int64_t GetTimeoutArg(Environment* env,
690+
const FunctionCallbackInfo<Value>& args,
689691
const int i) {
690692
if (args[i]->IsUndefined() || args[i]->IsString()) {
691693
return -1;
692694
}
693695
if (!args[i]->IsObject()) {
694-
Environment::ThrowTypeError(args.GetIsolate(),
695-
"options must be an object");
696+
env->ThrowTypeError("options must be an object");
696697
return -1;
697698
}
698699

@@ -704,22 +705,21 @@ class ContextifyScript : public BaseObject {
704705
int64_t timeout = value->IntegerValue();
705706

706707
if (timeout <= 0) {
707-
Environment::ThrowRangeError(args.GetIsolate(),
708-
"timeout must be a positive number");
708+
env->ThrowRangeError("timeout must be a positive number");
709709
return -1;
710710
}
711711
return timeout;
712712
}
713713

714714

715-
static bool GetDisplayErrorsArg(const FunctionCallbackInfo<Value>& args,
715+
static bool GetDisplayErrorsArg(Environment* env,
716+
const FunctionCallbackInfo<Value>& args,
716717
const int i) {
717718
if (args[i]->IsUndefined() || args[i]->IsString()) {
718719
return true;
719720
}
720721
if (!args[i]->IsObject()) {
721-
Environment::ThrowTypeError(args.GetIsolate(),
722-
"options must be an object");
722+
env->ThrowTypeError("options must be an object");
723723
return false;
724724
}
725725

@@ -731,7 +731,8 @@ class ContextifyScript : public BaseObject {
731731
}
732732

733733

734-
static Local<String> GetFilenameArg(const FunctionCallbackInfo<Value>& args,
734+
static Local<String> GetFilenameArg(Environment* env,
735+
const FunctionCallbackInfo<Value>& args,
735736
const int i) {
736737
Local<String> defaultFilename =
737738
FIXED_ONE_BYTE_STRING(args.GetIsolate(), "evalmachine.<anonymous>");
@@ -743,8 +744,7 @@ class ContextifyScript : public BaseObject {
743744
return args[i].As<String>();
744745
}
745746
if (!args[i]->IsObject()) {
746-
Environment::ThrowTypeError(args.GetIsolate(),
747-
"options must be an object");
747+
env->ThrowTypeError("options must be an object");
748748
return Local<String>();
749749
}
750750

@@ -770,9 +770,7 @@ class ContextifyScript : public BaseObject {
770770
}
771771

772772
if (!value->IsUint8Array()) {
773-
Environment::ThrowTypeError(
774-
args.GetIsolate(),
775-
"options.cachedData must be a Buffer instance");
773+
env->ThrowTypeError("options.cachedData must be a Buffer instance");
776774
return MaybeLocal<Uint8Array>();
777775
}
778776

0 commit comments

Comments
 (0)