src,deps: add isolate parameter to String::Concat

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 <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Refael Ackermann <[email protected]>
nodejs · Aug 28, 2018 · 7ce8a98 · 7ce8a98
1 parent b058601
commit 7ce8a98
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 48 deletions.
diff --git a/common.gypi b/common.gypi
@@ -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,

diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h
@@ -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

diff --git a/deps/v8/src/api.cc b/deps/v8/src/api.cc
@@ -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);
@@ -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) {

diff --git a/src/exceptions.cc b/src/exceptions.cc
@@ -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;
@@ -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();
@@ -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);
@@ -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);

diff --git a/src/node.cc b/src/node.cc
@@ -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;

diff --git a/src/node_api.cc b/src/node_api.cc
@@ -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,

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -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(

diff --git a/src/string_decoder.cc b/src/string_decoder.cc
@@ -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);