[api] Create v8::String::NewFromLiteral that returns Local<String>

String::NewFromLiteral is a templated function that takes a char[N]
argument that can be used as an alternative to String::NewFromUtf8 and
returns a Local<String> rather than a MaybeLocal<String> reducing the
number of ToLocalChecked() or other checks.

Since the string length is known at compile time, it can statically
assert that the length is less than String::kMaxLength, which means that
it can never fail at runtime.

This also converts all found uses of NewFromUtf8 taking a string literal
or a variable initialized from a string literal to use the new API. In
some cases the types of stored string literals are changed from const
char* to const char[] to ensure the size is retained.

This API does introduce a small difference compared to NewFromUtf8. For
a case like "abc\0def", NewFromUtf8 (using length -1 to infer length)
would treat this as a 3 character string, whereas the new API will treat
it as a 7 character string.

As a drive-by fix, this also fixes all redundant uses of
v8::NewStringType::kNormal when passed to any of the String::New*
functions.

Change-Id: Id96a44bc068d9c4eaa634aea688e024675a0e5b3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2089935
Commit-Queue: Dan Elphick <[email protected]>
Reviewed-by: Mathias Bynens <[email protected]>
Reviewed-by: Mythri Alle <[email protected]>
Reviewed-by: Clemens Backes <[email protected]>
Reviewed-by: Ulan Degenbaev <[email protected]>
Cr-Commit-Position: refs/heads/master@{#66622}

include/v8-fast-api-calls.h

@@ -120,9 +120,8 @@
  *        v8::ObjectTemplate::New(isolate);
  *      object_template->SetInternalFieldCount(
  *        kV8EmbedderWrapperObjectIndex + 1);
- *      object_template->Set(v8::String::NewFromUtf8(isolate, "method",
- *                                     v8::NewStringType::kNormal)
- *          .ToLocalChecked(), method_template);
+ *      object_template->Set(
+            v8::String::NewFromUtf8Literal(isolate, "method"), method_template);
  *
  *      // Instantiate the wrapper JS object.
  *      v8::Local<v8::Object> object =

include/v8.h

@@ -3181,6 +3181,23 @@ class V8_EXPORT String : public Name {
 
   V8_INLINE static String* Cast(v8::Value* obj);
 
+  /**
+   * Allocates a new string from a UTF-8 literal. This is equivalent to calling
+   * String::NewFromUtf(isolate, "...").ToLocalChecked(), but without the check
+   * overhead.
+   *
+   * When called on a string literal containing '\0', the inferred length is the
+   * length of the input array minus 1 (for the final '\0') and not the value
+   * returned by strlen.
+   **/
+  template <int N>
+  static V8_WARN_UNUSED_RESULT Local<String> NewFromUtf8Literal(
+      Isolate* isolate, const char (&literal)[N],
+      NewStringType type = NewStringType::kNormal) {
+    static_assert(N <= kMaxLength, "String is too long");
+    return NewFromUtf8Literal(isolate, literal, type, N - 1);
+  }
+
   /** Allocates a new string from UTF-8 data. Only returns an empty value when
    * length > kMaxLength. **/
   static V8_WARN_UNUSED_RESULT MaybeLocal<String> NewFromUtf8(
@@ -3315,9 +3332,20 @@ class V8_EXPORT String : public Name {
   ExternalStringResourceBase* GetExternalStringResourceBaseSlow(
       String::Encoding* encoding_out) const;
 
+  static Local<v8::String> NewFromUtf8Literal(Isolate* isolate,
+                                              const char* literal,
+                                              NewStringType type, int length);
+
   static void CheckCast(v8::Value* obj);
 };
 
+// Zero-length string specialization (templated string size includes
+// terminator).
+template <>
+inline V8_WARN_UNUSED_RESULT Local<String> String::NewFromUtf8Literal(
+    Isolate* isolate, const char (&literal)[1], NewStringType type) {
+  return String::Empty(isolate);
+}
 
 /**
  * A JavaScript symbol (ECMA-262 edition 6)
@@ -6327,11 +6355,12 @@ class CFunction;
  *    proto_t->Set(isolate, "proto_const", v8::Number::New(isolate, 2));
  *
  *    v8::Local<v8::ObjectTemplate> instance_t = t->InstanceTemplate();
- *    instance_t->SetAccessor(String::NewFromUtf8(isolate, "instance_accessor"),
- *                            InstanceAccessorCallback);
+ *    instance_t->SetAccessor(
+          String::NewFromUtf8Literal(isolate, "instance_accessor"),
+ *        InstanceAccessorCallback);
  *    instance_t->SetHandler(
  *        NamedPropertyHandlerConfiguration(PropertyHandlerCallback));
- *    instance_t->Set(String::NewFromUtf8(isolate, "instance_property"),
+ *    instance_t->Set(String::NewFromUtf8Literal(isolate, "instance_property"),
  *                    Number::New(isolate, 3));
  *
  *    v8::Local<v8::Function> function = t->GetFunction();

samples/hello-world.cc

@@ -37,9 +37,7 @@ int main(int argc, char* argv[]) {
     {
       // Create a string containing the JavaScript source code.
       v8::Local<v8::String> source =
-          v8::String::NewFromUtf8(isolate, "'Hello' + ', World!'",
-                                  v8::NewStringType::kNormal)
-              .ToLocalChecked();
+          v8::String::NewFromUtf8Literal(isolate, "'Hello' + ', World!'");
 
       // Compile the source code.
       v8::Local<v8::Script> script =
@@ -63,7 +61,7 @@ int main(int argc, char* argv[]) {
       //       get_local 1
       //       i32.add)
       //
-      const char* csource = R"(
+      const char csource[] = R"(
         let bytes = new Uint8Array([
           0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00, 0x01, 0x07, 0x01,
           0x60, 0x02, 0x7f, 0x7f, 0x01, 0x7f, 0x03, 0x02, 0x01, 0x00, 0x07,
@@ -77,8 +75,7 @@ int main(int argc, char* argv[]) {
 
       // Create a string containing the JavaScript source code.
       v8::Local<v8::String> source =
-          v8::String::NewFromUtf8(isolate, csource, v8::NewStringType::kNormal)
-              .ToLocalChecked();
+          v8::String::NewFromUtf8Literal(isolate, csource);
 
       // Compile the source code.
       v8::Local<v8::Script> script =

samples/process.cc

@@ -183,8 +183,7 @@ bool JsHttpRequestProcessor::Initialize(map<string, string>* opts,
   // Create a template for the global object where we set the
   // built-in global functions.
   Local<ObjectTemplate> global = ObjectTemplate::New(GetIsolate());
-  global->Set(String::NewFromUtf8(GetIsolate(), "log", NewStringType::kNormal)
-                  .ToLocalChecked(),
+  global->Set(String::NewFromUtf8Literal(GetIsolate(), "log"),
               FunctionTemplate::New(GetIsolate(), LogCallback));
 
   // Each processor gets its own context so different processors don't
@@ -210,8 +209,7 @@ bool JsHttpRequestProcessor::Initialize(map<string, string>* opts,
   // The script compiled and ran correctly.  Now we fetch out the
   // Process function from the global object.
   Local<String> process_name =
-      String::NewFromUtf8(GetIsolate(), "Process", NewStringType::kNormal)
-          .ToLocalChecked();
+      String::NewFromUtf8Literal(GetIsolate(), "Process");
   Local<Value> process_val;
   // If there is no Process function, or if it is not a function,
   // bail out
@@ -276,17 +274,13 @@ bool JsHttpRequestProcessor::InstallMaps(map<string, string>* opts,
 
   // Set the options object as a property on the global object.
   context->Global()
-      ->Set(context,
-            String::NewFromUtf8(GetIsolate(), "options", NewStringType::kNormal)
-                .ToLocalChecked(),
+      ->Set(context, String::NewFromUtf8Literal(GetIsolate(), "options"),
             opts_obj)
       .FromJust();
 
   Local<Object> output_obj = WrapMap(output);
   context->Global()
-      ->Set(context,
-            String::NewFromUtf8(GetIsolate(), "output", NewStringType::kNormal)
-                .ToLocalChecked(),
+      ->Set(context, String::NewFromUtf8Literal(GetIsolate(), "output"),
             output_obj)
       .FromJust();
 
@@ -563,21 +557,17 @@ Local<ObjectTemplate> JsHttpRequestProcessor::MakeRequestTemplate(
 
   // Add accessors for each of the fields of the request.
   result->SetAccessor(
-      String::NewFromUtf8(isolate, "path", NewStringType::kInternalized)
-          .ToLocalChecked(),
+      String::NewFromUtf8Literal(isolate, "path", NewStringType::kInternalized),
       GetPath);
+  result->SetAccessor(String::NewFromUtf8Literal(isolate, "referrer",
+                                                 NewStringType::kInternalized),
+                      GetReferrer);
   result->SetAccessor(
-      String::NewFromUtf8(isolate, "referrer", NewStringType::kInternalized)
-          .ToLocalChecked(),
-      GetReferrer);
-  result->SetAccessor(
-      String::NewFromUtf8(isolate, "host", NewStringType::kInternalized)
-          .ToLocalChecked(),
+      String::NewFromUtf8Literal(isolate, "host", NewStringType::kInternalized),
       GetHost);
-  result->SetAccessor(
-      String::NewFromUtf8(isolate, "userAgent", NewStringType::kInternalized)
-          .ToLocalChecked(),
-      GetUserAgent);
+  result->SetAccessor(String::NewFromUtf8Literal(isolate, "userAgent",
+                                                 NewStringType::kInternalized),
+                      GetUserAgent);
 
   // Again, return the result through the current handle scope.
   return handle_scope.Escape(result);

samples/shell.cc

@@ -108,27 +108,20 @@ v8::Local<v8::Context> CreateShellContext(v8::Isolate* isolate) {
   // Create a template for the global object.
   v8::Local<v8::ObjectTemplate> global = v8::ObjectTemplate::New(isolate);
   // Bind the global 'print' function to the C++ Print callback.
-  global->Set(
-      v8::String::NewFromUtf8(isolate, "print", v8::NewStringType::kNormal)
-          .ToLocalChecked(),
-      v8::FunctionTemplate::New(isolate, Print));
+  global->Set(v8::String::NewFromUtf8Literal(isolate, "print"),
+              v8::FunctionTemplate::New(isolate, Print));
   // Bind the global 'read' function to the C++ Read callback.
-  global->Set(v8::String::NewFromUtf8(
-                  isolate, "read", v8::NewStringType::kNormal).ToLocalChecked(),
+  global->Set(v8::String::NewFromUtf8Literal(isolate, "read"),
               v8::FunctionTemplate::New(isolate, Read));
   // Bind the global 'load' function to the C++ Load callback.
-  global->Set(v8::String::NewFromUtf8(
-                  isolate, "load", v8::NewStringType::kNormal).ToLocalChecked(),
+  global->Set(v8::String::NewFromUtf8Literal(isolate, "load"),
               v8::FunctionTemplate::New(isolate, Load));
   // Bind the 'quit' function
-  global->Set(v8::String::NewFromUtf8(
-                  isolate, "quit", v8::NewStringType::kNormal).ToLocalChecked(),
+  global->Set(v8::String::NewFromUtf8Literal(isolate, "quit"),
               v8::FunctionTemplate::New(isolate, Quit));
   // Bind the 'version' function
-  global->Set(
-      v8::String::NewFromUtf8(isolate, "version", v8::NewStringType::kNormal)
-          .ToLocalChecked(),
-      v8::FunctionTemplate::New(isolate, Version));
+  global->Set(v8::String::NewFromUtf8Literal(isolate, "version"),
+              v8::FunctionTemplate::New(isolate, Version));
 
   return v8::Context::New(isolate, NULL, global);
 }
@@ -161,22 +154,19 @@ void Print(const v8::FunctionCallbackInfo<v8::Value>& args) {
 void Read(const v8::FunctionCallbackInfo<v8::Value>& args) {
   if (args.Length() != 1) {
     args.GetIsolate()->ThrowException(
-        v8::String::NewFromUtf8(args.GetIsolate(), "Bad parameters",
-                                v8::NewStringType::kNormal).ToLocalChecked());
+        v8::String::NewFromUtf8Literal(args.GetIsolate(), "Bad parameters"));
     return;
   }
   v8::String::Utf8Value file(args.GetIsolate(), args[0]);
   if (*file == NULL) {
-    args.GetIsolate()->ThrowException(
-        v8::String::NewFromUtf8(args.GetIsolate(), "Error loading file",
-                                v8::NewStringType::kNormal).ToLocalChecked());
+    args.GetIsolate()->ThrowException(v8::String::NewFromUtf8Literal(
+        args.GetIsolate(), "Error loading file"));
     return;
   }
   v8::Local<v8::String> source;
   if (!ReadFile(args.GetIsolate(), *file).ToLocal(&source)) {
-    args.GetIsolate()->ThrowException(
-        v8::String::NewFromUtf8(args.GetIsolate(), "Error loading file",
-                                v8::NewStringType::kNormal).ToLocalChecked());
+    args.GetIsolate()->ThrowException(v8::String::NewFromUtf8Literal(
+        args.GetIsolate(), "Error loading file"));
     return;
   }
 
@@ -191,22 +181,19 @@ void Load(const v8::FunctionCallbackInfo<v8::Value>& args) {
     v8::HandleScope handle_scope(args.GetIsolate());
     v8::String::Utf8Value file(args.GetIsolate(), args[i]);
     if (*file == NULL) {
-      args.GetIsolate()->ThrowException(
-          v8::String::NewFromUtf8(args.GetIsolate(), "Error loading file",
-                                  v8::NewStringType::kNormal).ToLocalChecked());
+      args.GetIsolate()->ThrowException(v8::String::NewFromUtf8Literal(
+          args.GetIsolate(), "Error loading file"));
       return;
     }
     v8::Local<v8::String> source;
     if (!ReadFile(args.GetIsolate(), *file).ToLocal(&source)) {
-      args.GetIsolate()->ThrowException(
-          v8::String::NewFromUtf8(args.GetIsolate(), "Error loading file",
-                                  v8::NewStringType::kNormal).ToLocalChecked());
+      args.GetIsolate()->ThrowException(v8::String::NewFromUtf8Literal(
+          args.GetIsolate(), "Error loading file"));
       return;
     }
     if (!ExecuteString(args.GetIsolate(), source, args[i], false, false)) {
-      args.GetIsolate()->ThrowException(
-          v8::String::NewFromUtf8(args.GetIsolate(), "Error executing file",
-                                  v8::NewStringType::kNormal).ToLocalChecked());
+      args.GetIsolate()->ThrowException(v8::String::NewFromUtf8Literal(
+          args.GetIsolate(), "Error executing file"));
       return;
     }
   }
@@ -228,8 +215,8 @@ void Quit(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
 void Version(const v8::FunctionCallbackInfo<v8::Value>& args) {
   args.GetReturnValue().Set(
-      v8::String::NewFromUtf8(args.GetIsolate(), v8::V8::GetVersion(),
-                              v8::NewStringType::kNormal).ToLocalChecked());
+      v8::String::NewFromUtf8(args.GetIsolate(), v8::V8::GetVersion())
+          .ToLocalChecked());
 }
 
 
@@ -276,12 +263,9 @@ int RunMain(v8::Isolate* isolate, v8::Platform* platform, int argc,
     } else if (strcmp(str, "-e") == 0 && i + 1 < argc) {
       // Execute argument given to -e option directly.
       v8::Local<v8::String> file_name =
-          v8::String::NewFromUtf8(isolate, "unnamed",
-                                  v8::NewStringType::kNormal).ToLocalChecked();
+          v8::String::NewFromUtf8Literal(isolate, "unnamed");
       v8::Local<v8::String> source;
-      if (!v8::String::NewFromUtf8(isolate, argv[++i],
-                                   v8::NewStringType::kNormal)
-               .ToLocal(&source)) {
+      if (!v8::String::NewFromUtf8(isolate, argv[++i]).ToLocal(&source)) {
         return 1;
       }
       bool success = ExecuteString(isolate, source, file_name, false, true);
@@ -290,8 +274,7 @@ int RunMain(v8::Isolate* isolate, v8::Platform* platform, int argc,
     } else {
       // Use all other arguments as names of files to load and run.
       v8::Local<v8::String> file_name =
-          v8::String::NewFromUtf8(isolate, str, v8::NewStringType::kNormal)
-              .ToLocalChecked();
+          v8::String::NewFromUtf8(isolate, str).ToLocalChecked();
       v8::Local<v8::String> source;
       if (!ReadFile(isolate, str).ToLocal(&source)) {
         fprintf(stderr, "Error reading '%s'\n", str);
@@ -313,8 +296,7 @@ void RunShell(v8::Local<v8::Context> context, v8::Platform* platform) {
   // Enter the execution environment before evaluating any code.
   v8::Context::Scope context_scope(context);
   v8::Local<v8::String> name(
-      v8::String::NewFromUtf8(context->GetIsolate(), "(shell)",
-                              v8::NewStringType::kNormal).ToLocalChecked());
+      v8::String::NewFromUtf8Literal(context->GetIsolate(), "(shell)"));
   while (true) {
     char buffer[kBufferSize];
     fprintf(stderr, "> ");
@@ -323,8 +305,7 @@ void RunShell(v8::Local<v8::Context> context, v8::Platform* platform) {
     v8::HandleScope handle_scope(context->GetIsolate());
     ExecuteString(
         context->GetIsolate(),
-        v8::String::NewFromUtf8(context->GetIsolate(), str,
-                                v8::NewStringType::kNormal).ToLocalChecked(),
+        v8::String::NewFromUtf8(context->GetIsolate(), str).ToLocalChecked(),
         name, true, true);
     while (v8::platform::PumpMessageLoop(platform, context->GetIsolate()))
       continue;

src/api/api.cc

@@ -6371,6 +6371,19 @@ STATIC_ASSERT(v8::String::kMaxLength == i::String::kMaxLength);
     result = Utils::ToLocal(handle_result);                                \
   }
 
+Local<String> String::NewFromUtf8Literal(Isolate* isolate, const char* literal,
+                                         NewStringType type, int length) {
+  DCHECK_LE(length, i::String::kMaxLength);
+  i::Isolate* i_isolate = reinterpret_cast<internal::Isolate*>(isolate);
+  ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate);
+  LOG_API(i_isolate, String, NewFromUtf8Literal);
+  i::Handle<i::String> handle_result =
+      NewString(i_isolate->factory(), type,
+                i::Vector<const char>(literal, length))
+          .ToHandleChecked();
+  return Utils::ToLocal(handle_result);
+}
+
 MaybeLocal<String> String::NewFromUtf8(Isolate* isolate, const char* data,
                                        NewStringType type, int length) {
   NEW_STRING(isolate, String, NewFromUtf8, char, data, type, length);