Skip to content

Commit

Permalink
src,deps: add isolate parameter to String::Concat
Browse files Browse the repository at this point in the history
Partially backport an upstream commit that deprecates String::Concat
without the isolate parameter. This overload has already been removed
in V8 7.0.

PR-URL: #22521
Refs: v8/v8@8a011b5
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
targos committed Sep 3, 2018
1 parent d27e463 commit 383d578
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 48 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.17',
'v8_embedder_string': '-node.18',

# Enable disassembler for `--print-code` v8 options
'v8_enable_disassembler': 1,
Expand Down
6 changes: 5 additions & 1 deletion deps/v8/include/v8.h
Original file line number Diff line number Diff line change
Expand Up @@ -2807,7 +2807,11 @@ class V8_EXPORT String : public Name {
* Creates a new string by concatenating the left and the right strings
* passed in as parameters.
*/
static Local<String> Concat(Local<String> left, Local<String> right);
static Local<String> Concat(Isolate* isolate, Local<String> left,
Local<String> right);
static V8_DEPRECATE_SOON("Use Isolate* version",
Local<String> Concat(Local<String> left,
Local<String> right));

/**
* Creates a new external string using the data defined in the given
Expand Down
11 changes: 8 additions & 3 deletions deps/v8/src/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6637,10 +6637,10 @@ MaybeLocal<String> String::NewFromTwoByte(Isolate* isolate,
return result;
}


Local<String> v8::String::Concat(Local<String> left, Local<String> right) {
Local<String> v8::String::Concat(Isolate* v8_isolate, Local<String> left,
Local<String> right) {
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
i::Handle<i::String> left_string = Utils::OpenHandle(*left);
i::Isolate* isolate = left_string->GetIsolate();
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
LOG_API(isolate, String, Concat);
i::Handle<i::String> right_string = Utils::OpenHandle(*right);
Expand All @@ -6654,6 +6654,11 @@ Local<String> v8::String::Concat(Local<String> left, Local<String> right) {
return Utils::ToLocal(result);
}

Local<String> v8::String::Concat(Local<String> left, Local<String> right) {
i::Handle<i::String> left_string = Utils::OpenHandle(*left);
i::Isolate* isolate = left_string->GetIsolate();
return Concat(reinterpret_cast<Isolate*>(isolate), left, right);
}

MaybeLocal<String> v8::String::NewExternalTwoByte(
Isolate* isolate, v8::String::ExternalStringResource* resource) {
Expand Down
72 changes: 40 additions & 32 deletions src/exceptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,40 +26,40 @@ Local<Value> ErrnoException(Isolate* isolate,
Environment* env = Environment::GetCurrent(isolate);

Local<Value> e;
Local<String> estring = OneByteString(env->isolate(), errno_string(errorno));
Local<String> estring = OneByteString(isolate, errno_string(errorno));
if (msg == nullptr || msg[0] == '\0') {
msg = strerror(errorno);
}
Local<String> message = OneByteString(env->isolate(), msg);
Local<String> message = OneByteString(isolate, msg);

Local<String> cons =
String::Concat(estring, FIXED_ONE_BYTE_STRING(env->isolate(), ", "));
cons = String::Concat(cons, message);
String::Concat(isolate, estring, FIXED_ONE_BYTE_STRING(isolate, ", "));
cons = String::Concat(isolate, cons, message);

Local<String> path_string;
if (path != nullptr) {
// FIXME(bnoordhuis) It's questionable to interpret the file path as UTF-8.
path_string = String::NewFromUtf8(env->isolate(), path,
v8::NewStringType::kNormal).ToLocalChecked();
path_string = String::NewFromUtf8(isolate, path, v8::NewStringType::kNormal)
.ToLocalChecked();
}

if (path_string.IsEmpty() == false) {
cons = String::Concat(cons, FIXED_ONE_BYTE_STRING(env->isolate(), " '"));
cons = String::Concat(cons, path_string);
cons = String::Concat(cons, FIXED_ONE_BYTE_STRING(env->isolate(), "'"));
cons = String::Concat(isolate, cons, FIXED_ONE_BYTE_STRING(isolate, " '"));
cons = String::Concat(isolate, cons, path_string);
cons = String::Concat(isolate, cons, FIXED_ONE_BYTE_STRING(isolate, "'"));
}
e = Exception::Error(cons);

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

if (path_string.IsEmpty() == false) {
obj->Set(env->path_string(), path_string);
}

if (syscall != nullptr) {
obj->Set(env->syscall_string(), OneByteString(env->isolate(), syscall));
obj->Set(env->syscall_string(), OneByteString(isolate, syscall));
}

return e;
Expand All @@ -68,10 +68,11 @@ Local<Value> ErrnoException(Isolate* isolate,
static Local<String> StringFromPath(Isolate* isolate, const char* path) {
#ifdef _WIN32
if (strncmp(path, "\\\\?\\UNC\\", 8) == 0) {
return String::Concat(FIXED_ONE_BYTE_STRING(isolate, "\\\\"),
String::NewFromUtf8(isolate, path + 8,
v8::NewStringType::kNormal)
.ToLocalChecked());
return String::Concat(
isolate,
FIXED_ONE_BYTE_STRING(isolate, "\\\\"),
String::NewFromUtf8(isolate, path + 8, v8::NewStringType::kNormal)
.ToLocalChecked());
} else if (strncmp(path, "\\\\?\\", 4) == 0) {
return String::NewFromUtf8(isolate, path + 4, v8::NewStringType::kNormal)
.ToLocalChecked();
Expand Down Expand Up @@ -109,25 +110,31 @@ Local<Value> UVException(Isolate* isolate,
Local<String> js_dest;

Local<String> js_msg = js_code;
js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, ": "));
js_msg = String::Concat(js_msg, OneByteString(isolate, msg));
js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, ", "));
js_msg = String::Concat(js_msg, js_syscall);
js_msg =
String::Concat(isolate, js_msg, FIXED_ONE_BYTE_STRING(isolate, ": "));
js_msg = String::Concat(isolate, js_msg, OneByteString(isolate, msg));
js_msg =
String::Concat(isolate, js_msg, FIXED_ONE_BYTE_STRING(isolate, ", "));
js_msg = String::Concat(isolate, js_msg, js_syscall);

if (path != nullptr) {
js_path = StringFromPath(isolate, path);

js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, " '"));
js_msg = String::Concat(js_msg, js_path);
js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, "'"));
js_msg =
String::Concat(isolate, js_msg, FIXED_ONE_BYTE_STRING(isolate, " '"));
js_msg = String::Concat(isolate, js_msg, js_path);
js_msg =
String::Concat(isolate, js_msg, FIXED_ONE_BYTE_STRING(isolate, "'"));
}

if (dest != nullptr) {
js_dest = StringFromPath(isolate, dest);

js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, " -> '"));
js_msg = String::Concat(js_msg, js_dest);
js_msg = String::Concat(js_msg, FIXED_ONE_BYTE_STRING(isolate, "'"));
js_msg = String::Concat(
isolate, js_msg, FIXED_ONE_BYTE_STRING(isolate, " -> '"));
js_msg = String::Concat(isolate, js_msg, js_dest);
js_msg =
String::Concat(isolate, js_msg, FIXED_ONE_BYTE_STRING(isolate, "'"));
}

Local<Object> e = Exception::Error(js_msg)->ToObject(isolate);
Expand Down Expand Up @@ -182,17 +189,18 @@ Local<Value> WinapiErrnoException(Isolate* isolate,
if (!msg || !msg[0]) {
msg = winapi_strerror(errorno, &must_free);
}
Local<String> message = OneByteString(env->isolate(), msg);
Local<String> message = OneByteString(isolate, msg);

if (path) {
Local<String> cons1 =
String::Concat(message, FIXED_ONE_BYTE_STRING(isolate, " '"));
Local<String> cons2 =
String::Concat(cons1,
String::NewFromUtf8(isolate, path, v8::NewStringType::kNormal)
.ToLocalChecked());
String::Concat(isolate, message, FIXED_ONE_BYTE_STRING(isolate, " '"));
Local<String> cons2 = String::Concat(
isolate,
cons1,
String::NewFromUtf8(isolate, path, v8::NewStringType::kNormal)
.ToLocalChecked());
Local<String> cons3 =
String::Concat(cons2, FIXED_ONE_BYTE_STRING(isolate, "'"));
String::Concat(isolate, cons2, FIXED_ONE_BYTE_STRING(isolate, "'"));
e = Exception::Error(cons3);
} else {
e = Exception::Error(message);
Expand Down
4 changes: 2 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1379,8 +1379,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(context).ToLocalChecked());
errmsg = String::Concat(
env->isolate(), errmsg, args[1]->ToString(context).ToLocalChecked());
#endif // _WIN32
env->isolate()->ThrowException(Exception::Error(errmsg));
return;
Expand Down
14 changes: 8 additions & 6 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1838,14 +1838,16 @@ static napi_status set_error_code(napi_env env,
if (!maybe_name.IsEmpty()) {
v8::Local<v8::Value> name = maybe_name.ToLocalChecked();
if (name->IsString()) {
name_string = v8::String::Concat(name_string, name.As<v8::String>());
name_string =
v8::String::Concat(isolate, name_string, name.As<v8::String>());
}
}
name_string = v8::String::Concat(name_string,
FIXED_ONE_BYTE_STRING(isolate, " ["));
name_string = v8::String::Concat(name_string, code_value.As<v8::String>());
name_string = v8::String::Concat(name_string,
FIXED_ONE_BYTE_STRING(isolate, "]"));
name_string = v8::String::Concat(
isolate, name_string, FIXED_ONE_BYTE_STRING(isolate, " ["));
name_string =
v8::String::Concat(isolate, name_string, code_value.As<v8::String>());
name_string = v8::String::Concat(
isolate, name_string, FIXED_ONE_BYTE_STRING(isolate, "]"));

set_maybe = err_object->Set(context, name_key, name_string);
RETURN_STATUS_IF_FALSE(env,
Expand Down
6 changes: 4 additions & 2 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -897,8 +897,10 @@ class ContextifyScript : public BaseObject {
}

Local<String> decorated_stack = String::Concat(
String::Concat(arrow.As<String>(),
FIXED_ONE_BYTE_STRING(env->isolate(), "\n")),
env->isolate(),
String::Concat(env->isolate(),
arrow.As<String>(),
FIXED_ONE_BYTE_STRING(env->isolate(), "\n")),
stack.As<String>());
err_obj->Set(env->stack_string(), decorated_stack);
err_obj->SetPrivate(
Expand Down
2 changes: 1 addition & 1 deletion src/string_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ MaybeLocal<String> StringDecoder::DecodeData(Isolate* isolate,
if (prepend.IsEmpty()) {
return body;
} else {
return String::Concat(prepend, body);
return String::Concat(isolate, prepend, body);
}
} else {
CHECK(Encoding() == ASCII || Encoding() == HEX || Encoding() == LATIN1);
Expand Down

0 comments on commit 383d578

Please sign in to comment.