Skip to content

Commit 1620226

Browse files
bnoordhuisFishrock123
bnoordhuis
authored and
Fishrock123
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 <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
1 parent 6c30ddd commit 1620226

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
@@ -436,39 +436,23 @@ inline Environment::IsolateData* Environment::isolate_data() const {
436436
return isolate_data_;
437437
}
438438

439-
// this would have been a template function were it not for the fact that g++
440-
// sometimes fails to resolve it...
441-
#define THROW_ERROR(fun) \
442-
do { \
443-
v8::HandleScope scope(isolate); \
444-
isolate->ThrowException(fun(OneByteString(isolate, errmsg))); \
445-
} \
446-
while (0)
447-
448-
inline void Environment::ThrowError(v8::Isolate* isolate, const char* errmsg) {
449-
THROW_ERROR(v8::Exception::Error);
450-
}
451-
452-
inline void Environment::ThrowTypeError(v8::Isolate* isolate,
453-
const char* errmsg) {
454-
THROW_ERROR(v8::Exception::TypeError);
455-
}
456-
457-
inline void Environment::ThrowRangeError(v8::Isolate* isolate,
458-
const char* errmsg) {
459-
THROW_ERROR(v8::Exception::RangeError);
460-
}
461-
462439
inline void Environment::ThrowError(const char* errmsg) {
463-
ThrowError(isolate(), errmsg);
440+
ThrowError(v8::Exception::Error, errmsg);
464441
}
465442

466443
inline void Environment::ThrowTypeError(const char* errmsg) {
467-
ThrowTypeError(isolate(), errmsg);
444+
ThrowError(v8::Exception::TypeError, errmsg);
468445
}
469446

470447
inline void Environment::ThrowRangeError(const char* errmsg) {
471-
ThrowRangeError(isolate(), errmsg);
448+
ThrowError(v8::Exception::RangeError, errmsg);
449+
}
450+
451+
inline void Environment::ThrowError(
452+
v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
453+
const char* errmsg) {
454+
v8::HandleScope handle_scope(isolate());
455+
isolate()->ThrowException(fun(OneByteString(isolate(), errmsg)));
472456
}
473457

474458
inline void Environment::ThrowErrnoException(int errorno,

‎src/env.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -473,11 +473,6 @@ class Environment {
473473
const char* path = nullptr,
474474
const char* dest = nullptr);
475475

476-
// Convenience methods for contextify
477-
inline static void ThrowError(v8::Isolate* isolate, const char* errmsg);
478-
inline static void ThrowTypeError(v8::Isolate* isolate, const char* errmsg);
479-
inline static void ThrowRangeError(v8::Isolate* isolate, const char* errmsg);
480-
481476
inline v8::Local<v8::FunctionTemplate>
482477
NewFunctionTemplate(v8::FunctionCallback callback,
483478
v8::Local<v8::Signature> signature =
@@ -534,6 +529,9 @@ class Environment {
534529
static const int kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX;
535530

536531
private:
532+
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
533+
const char* errmsg);
534+
537535
static const int kIsolateSlot = NODE_ISOLATE_SLOT;
538536

539537
class IsolateData;

‎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)