Skip to content

src: replace ->To*(isolate) with ->To*(context).ToLocalChecked() #17343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Leko wants to merge 8 commits into from
+56 −17
Closed

src: replace ->To*(isolate) with ->To*(context).ToLocalChecked() #17343

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e41c511
src: replace ->To*(isolate) with ->To*(context).ToLocalChecked()
Leko Nov 27, 2017
f9107cc
test: use .As<Object> on Exception::Error
Leko Nov 28, 2017
60f4711
test: use .As<Object> instead of ->ToObject
Leko Nov 28, 2017
19a1b2e
test: add test case that crashes when throwing an exception with vm.S…
Leko Nov 28, 2017
1926bcf
src: use FromMaybe instead of ToLocalChecked
Leko Nov 28, 2017
096cbbf
src: pass context to Get()
Leko Nov 28, 2017
dbee7b0
test: add test case that crashes when process.dlopen with undefined
Leko Nov 30, 2017
d7cc341
src: return if Get or ToObject return an empty before call ToLocalChe…
Leko Nov 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ Local<Value> ErrnoException(Isolate* isolate,
}
e = Exception::Error(cons);

Local<Object> obj = e->ToObject(env->isolate());
Local<Object> obj = e.As<Object>();
obj->Set(env->errno_string(), Integer::New(env->isolate(), errorno));
obj->Set(env->code_string(), estring);

Expand Down Expand Up @@ -751,7 +751,7 @@ Local<Value> WinapiErrnoException(Isolate* isolate,
e = Exception::Error(message);
}

Local<Object> obj = e->ToObject(env->isolate());
Local<Object> obj = e.As<Object>();
obj->Set(env->errno_string(), Integer::New(isolate, errorno));

if (path != nullptr) {
Expand Down Expand Up @@ -1482,7 +1482,7 @@ static void ReportException(Environment* env,
if (er->IsUndefined() || er->IsNull()) {
trace_value = Undefined(env->isolate());
} else {
Local<Object> err_obj = er->ToObject(env->isolate());
Local<Object> err_obj = er->ToObject(env->context()).ToLocalChecked();

trace_value = err_obj->Get(env->stack_string());
arrow =
Expand Down Expand Up @@ -2301,7 +2301,8 @@ static void DLOpen(const FunctionCallbackInfo<Value>& args) {
return env->ThrowTypeError("flag argument must be an integer.");
}

Local<Object> module = args[0]->ToObject(env->isolate()); // Cast
Local<Object> module =
args[0]->ToObject(env->context()).ToLocalChecked(); // Cast
node::Utf8Value filename(env->isolate(), args[1]); // Cast
DLib dlib;
dlib.filename_ = *filename;
Expand All @@ -2319,7 +2320,8 @@ static void DLOpen(const FunctionCallbackInfo<Value>& args) {
dlib.Close();
#ifdef _WIN32
// Windows needs to add the filename into the error message
errmsg = String::Concat(errmsg, args[1]->ToString(env->isolate()));
errmsg = String::Concat(errmsg,
args[1]->ToString(env->context()).ToLocalChecked());
#endif // _WIN32
env->isolate()->ThrowException(Exception::Error(errmsg));
return;
Expand Down Expand Up @@ -2364,7 +2366,8 @@ static void DLOpen(const FunctionCallbackInfo<Value>& args) {
modlist_addon = mp;

Local<String> exports_string = env->exports_string();
Local<Object> exports = module->Get(exports_string)->ToObject(env->isolate());
Local<Object> exports =
module->Get(exports_string)->ToObject(env->context()).ToLocalChecked();
Copy link
Copy Markdown
Member

@addaleax addaleax Nov 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're already here, you can feel free to also change the Get() call to take a context argument :)

Copy link
Copy Markdown
Contributor Author

@Leko Leko Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Get() call to take a context argument

I didn’t understand what you said.
Do you expect this change ?

  Local<Object> exports = module->Get(exports_string).As<Object>();

Copy link
Copy Markdown
Member

@addaleax addaleax Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look at v8.h, you’ll see that there are two overloads of ->Get(), one that takes a context argument and one that doesn’t (and it deprecated):

node/deps/v8/include/v8.h

Lines 3166 to 3168 in cbaf59c

V8_DEPRECATE_SOON("Use maybe version", Local<Value> Get(Local<Value> key));
V8_WARN_UNUSED_RESULT MaybeLocal<Value> Get(Local<Context> context,
Local<Value> key);

At some point, we’re going to use the latter here instead of module->Get(exports_string). Whether you want to do the conversion in this PR or not is up to you :)

If you want to see an example of how these conversions look like, you can take a look at #16247.

Copy link
Copy Markdown
Contributor Author

@Leko Leko Nov 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for detailed information.

If you look at v8.h,

I will check the header file properly after this.

I update to add Local<Context> argument.
Please check my changes !


if (mp->nm_context_register_func != nullptr) {
mp->nm_context_register_func(exports, module, env->context(), mp->nm_priv);
Expand Down Expand Up @@ -4337,7 +4340,7 @@ void EmitBeforeExit(Environment* env) {
Local<String> exit_code = FIXED_ONE_BYTE_STRING(env->isolate(), "exitCode");
Local<Value> args[] = {
FIXED_ONE_BYTE_STRING(env->isolate(), "beforeExit"),
process_object->Get(exit_code)->ToInteger(env->isolate())
process_object->Get(exit_code)->ToInteger(env->context()).ToLocalChecked()
};
MakeCallback(env->isolate(),
process_object, "emit", arraysize(args), args,
Expand Down
4 changes: 2 additions & 2 deletions src/node_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ void Fill(const FunctionCallbackInfo<Value>& args) {
return;
}

str_obj = args[1]->ToString(env->isolate());
str_obj = args[1]->ToString(env->context()).ToLocalChecked();
enc = ParseEncoding(env->isolate(), args[4], UTF8);
str_length =
enc == UTF8 ? str_obj->Utf8Length() :
Expand Down Expand Up @@ -677,7 +677,7 @@ void StringWrite(const FunctionCallbackInfo<Value>& args) {
if (!args[0]->IsString())
return env->ThrowTypeError("Argument must be a string");

Local<String> str = args[0]->ToString(env->isolate());
Local<String> str = args[0]->ToString(env->context()).ToLocalChecked();

size_t offset;
size_t max_length;
Expand Down
3 changes: 2 additions & 1 deletion src/node_contextify.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,8 @@ class ContextifyScript : public BaseObject {
new ContextifyScript(env, args.This());

TryCatch try_catch(env->isolate());
Local<String> code = args[0]->ToString(env->isolate());
Local<String> code =
args[0]->ToString(env->context()).FromMaybe(Local<String>());

Local<Value> options = args[1];
MaybeLocal<String> filename = GetFilenameArg(env, options);
Expand Down
2 changes: 1 addition & 1 deletion src/node_file.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,7 @@ static void Read(const FunctionCallbackInfo<Value>& args) {

char * buf = nullptr;

Local<Object> buffer_obj = args[1]->ToObject(env->isolate());
Local<Object> buffer_obj = args[1].As<Object>();
char *buffer_data = Buffer::Data(buffer_obj);
size_t buffer_length = Buffer::Length(buffer_obj);

Expand Down
2 changes: 1 addition & 1 deletion src/node_http_parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ class Parser : public AsyncWrap {
enum http_errno err = HTTP_PARSER_ERRNO(&parser->parser_);

Local<Value> e = Exception::Error(env->parse_error_string());
Local<Object> obj = e->ToObject(env->isolate());
Local<Object> obj = e.As<Object>();
obj->Set(env->bytes_parsed_string(), Integer::New(env->isolate(), 0));
obj->Set(env->code_string(),
OneByteString(env->isolate(), http_errno_name(err)));
Expand Down
4 changes: 2 additions & 2 deletions src/node_zlib.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ class ZCtx : public AsyncWrap {
} else {
CHECK(Buffer::HasInstance(args[1]));
Local<Object> in_buf;
in_buf = args[1]->ToObject(env->isolate());
in_buf = args[1]->ToObject(env->context()).ToLocalChecked();
in_off = args[2]->Uint32Value();
in_len = args[3]->Uint32Value();

Expand All @@ -187,7 +187,7 @@ class ZCtx : public AsyncWrap {
}

CHECK(Buffer::HasInstance(args[4]));
Local<Object> out_buf = args[4]->ToObject(env->isolate());
Local<Object> out_buf = args[4]->ToObject(env->context()).ToLocalChecked();
out_off = args[5]->Uint32Value();
out_len = args[6]->Uint32Value();
CHECK(Buffer::IsWithinBounds(out_off, out_len, Buffer::Length(out_buf)));
Expand Down
3 changes: 2 additions & 1 deletion src/process_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ class ProcessWrap : public HandleWrap {
ProcessWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());

Local<Object> js_options = args[0]->ToObject(env->isolate());
Local<Object> js_options =
args[0]->ToObject(env->context()).ToLocalChecked();

uv_process_options_t options;
memset(&options, 0, sizeof(uv_process_options_t));
Expand Down
4 changes: 2 additions & 2 deletions src/stream_base.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
// Buffer chunk, no additional storage required

// String chunk
Local<String> string = chunk->ToString(env->isolate());
Local<String> string = chunk->ToString(env->context()).ToLocalChecked();
enum encoding encoding = ParseEncoding(env->isolate(),
chunks->Get(i * 2 + 1));
size_t chunk_size;
Expand Down Expand Up @@ -179,7 +179,7 @@ int StreamBase::Writev(const FunctionCallbackInfo<Value>& args) {
char* str_storage = req_wrap->Extra(offset);
size_t str_size = storage_size - offset;

Local<String> string = chunk->ToString(env->isolate());
Local<String> string = chunk->ToString(env->context()).ToLocalChecked();
enum encoding encoding = ParseEncoding(env->isolate(),
chunks->Get(i * 2 + 1));
str_size = StringBytes::Write(env->isolate(),
Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-vm-script-throw-in-tostring.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';

require('../common');
const assert = require('assert');

const vm = require('vm');

assert.throws(() => {
new vm.Script({
toString() {
throw new Error();
}
});
}, Error);