[base] Fix {StaticCharVector} and add {StaticOneByteVector}

{StaticCharVector}, according to its name, should return a
{Vector<const char>}. For getting a {Vector<const uint8_t>}, the method
should be called {StaticOneByteVector}, analog to the
{OneByteVector} methods that already exist.

Also, {StaticCharVector} is constexpr, but {StaticOneByteVector} cannot
be, since it contains a {reinterpret_cast}. The same holds for
{Vector::cast} in general.

This CL
- changes the return type of {StaticCharVector} to be
  {Vector<const char>},
- introduces a new {StaticOneByteVector} which returns
  {Vector<const uint8_t>},
- fixes constexpr annotations at various methods returning {Vector}s,
- refactors users of {StaticCharVector} to either use
  {StaticOneByteVector} instead, or work on {char} if that makes more
  sense.

R=leszeks@chromium.org

Bug: v8:10426
Change-Id: I71e336097e41ad30f982aa6344ca3d67b3a01fe3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2154196
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67213}

src/heap/factory.h

@@ -234,7 +234,7 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
       const char (&str)[N],
       AllocationType allocation = AllocationType::kYoung) {
     DCHECK_EQ(N, strlen(str) + 1);
-    return NewStringFromOneByte(StaticCharVector(str), allocation)
+    return NewStringFromOneByte(StaticOneByteVector(str), allocation)
         .ToHandleChecked();
   }
 

src/parsing/scanner.cc

@@ -233,9 +233,9 @@ void Scanner::TryToParseSourceURLComment() {
   if (!name.is_one_byte()) return;
   Vector<const uint8_t> name_literal = name.one_byte_literal();
   LiteralBuffer* value;
-  if (name_literal == StaticCharVector("sourceURL")) {
+  if (name_literal == StaticOneByteVector("sourceURL")) {
     value = &source_url_;
-  } else if (name_literal == StaticCharVector("sourceMappingURL")) {
+  } else if (name_literal == StaticOneByteVector("sourceMappingURL")) {
     value = &source_mapping_url_;
   } else {
     return;

src/strings/uri.cc

@@ -420,7 +420,7 @@ static MaybeHandle<String> UnescapePrivate(Isolate* isolate,
   int index;
   {
     DisallowHeapAllocation no_allocation;
-    StringSearch<uint8_t, Char> search(isolate, StaticCharVector("%"));
+    StringSearch<uint8_t, Char> search(isolate, StaticOneByteVector("%"));
     index = search.Search(source->GetCharVector<Char>(no_allocation), 0);
     if (index < 0) return source;
   }

src/utils/vector.h

@@ -120,17 +120,15 @@ class Vector {
   }
 
   template <typename S>
-  static constexpr Vector<T> cast(Vector<S> input) {
+  static Vector<T> cast(Vector<S> input) {
     // Casting is potentially dangerous, so be really restrictive here. This
     // might be lifted once we have use cases for that.
     STATIC_ASSERT(std::is_pod<S>::value);
     STATIC_ASSERT(std::is_pod<T>::value);
-#if V8_HAS_CXX14_CONSTEXPR
-    DCHECK_EQ(0, (input.length() * sizeof(S)) % sizeof(T));
+    DCHECK_EQ(0, (input.size() * sizeof(S)) % sizeof(T));
     DCHECK_EQ(0, reinterpret_cast<uintptr_t>(input.begin()) % alignof(T));
-#endif
     return Vector<T>(reinterpret_cast<T*>(input.begin()),
-                     input.length() * sizeof(S) / sizeof(T));
+                     input.size() * sizeof(S) / sizeof(T));
   }
 
   bool operator==(const Vector<const T> other) const {
@@ -240,37 +238,48 @@ class OwnedVector {
   size_t length_ = 0;
 };
 
+// The vectors returned by {StaticCharVector}, {CStrVector}, or {OneByteVector}
+// do not contain a null-termination byte. If you want the null byte, use
+// {ArrayVector}.
+
+// Known length, constexpr.
 template <size_t N>
-constexpr Vector<const uint8_t> StaticCharVector(const char (&array)[N]) {
-  return Vector<const uint8_t>::cast(Vector<const char>(array, N - 1));
+constexpr Vector<const char> StaticCharVector(const char (&array)[N]) {
+  return {array, N - 1};
 }
 
-// The resulting vector does not contain a null-termination byte. If you want
-// the null byte, use ArrayVector("foo").
+// Unknown length, not constexpr.
 inline Vector<const char> CStrVector(const char* data) {
-  return Vector<const char>(data, strlen(data));
+  return {data, strlen(data)};
 }
 
+// OneByteVector is never constexpr because the data pointer is
+// {reinterpret_cast}ed.
 inline Vector<const uint8_t> OneByteVector(const char* data, size_t length) {
-  return Vector<const uint8_t>(reinterpret_cast<const uint8_t*>(data), length);
+  return {reinterpret_cast<const uint8_t*>(data), length};
 }
 
 inline Vector<const uint8_t> OneByteVector(const char* data) {
   return OneByteVector(data, strlen(data));
 }
 
+template <size_t N>
+Vector<const uint8_t> StaticOneByteVector(const char (&array)[N]) {
+  return OneByteVector(array, N - 1);
+}
+
 // For string literals, ArrayVector("foo") returns a vector ['f', 'o', 'o', \0]
 // with length 4 and null-termination.
 // If you want ['f', 'o', 'o'], use CStrVector("foo").
 template <typename T, size_t N>
 inline constexpr Vector<T> ArrayVector(T (&arr)[N]) {
-  return Vector<T>{arr, N};
+  return {arr, N};
 }
 
 // Construct a Vector from a start pointer and a size.
 template <typename T>
 inline constexpr Vector<T> VectorOf(T* start, size_t size) {
-  return Vector<T>(start, size);
+  return {start, size};
 }
 
 // Construct a Vector from anything providing a {data()} and {size()} accessor.

src/wasm/wasm-js.cc

@@ -331,14 +331,10 @@ class InstantiateBytesResultResolver
         isolate_->factory()->NewJSObject(isolate_->object_function());
 
     i::Handle<i::String> instance_name =
-        isolate_->factory()
-            ->NewStringFromOneByte(i::StaticCharVector("instance"))
-            .ToHandleChecked();
+        isolate_->factory()->NewStringFromStaticChars("instance");
 
     i::Handle<i::String> module_name =
-        isolate_->factory()
-            ->NewStringFromOneByte(i::StaticCharVector("module"))
-            .ToHandleChecked();
+        isolate_->factory()->NewStringFromStaticChars("module");
 
     i::JSObject::AddProperty(isolate_, result, instance_name, instance,
                              i::NONE);

test/cctest/compiler/value-helper.h

@@ -248,7 +248,7 @@ class ValueHelper {
     return ArrayVector(uint32_array);
   }
 
-  static constexpr Vector<const int32_t> int32_vector() {
+  static Vector<const int32_t> int32_vector() {
     return Vector<const int32_t>::cast(uint32_vector());
   }
 
@@ -282,7 +282,7 @@ class ValueHelper {
     return ArrayVector(uint64_array);
   }
 
-  static constexpr Vector<const int64_t> int64_vector() {
+  static Vector<const int64_t> int64_vector() {
     return Vector<const int64_t>::cast(uint64_vector());
   }
 
@@ -293,7 +293,7 @@ class ValueHelper {
     return ArrayVector(int16_array);
   }
 
-  static constexpr Vector<const uint16_t> uint16_vector() {
+  static Vector<const uint16_t> uint16_vector() {
     return Vector<const uint16_t>::cast(int16_vector());
   }
 
@@ -304,7 +304,7 @@ class ValueHelper {
     return ArrayVector(int8_array);
   }
 
-  static constexpr Vector<const uint8_t> uint8_vector() {
+  static Vector<const uint8_t> uint8_vector() {
     return Vector<const uint8_t>::cast(ArrayVector(int8_array));
   }
 
@@ -317,41 +317,41 @@ class ValueHelper {
   }
 
   template <typename T>
-  static constexpr Vector<const T> GetVector();
+  static inline Vector<const T> GetVector();
 };
 
 template <>
-constexpr Vector<const int8_t> ValueHelper::GetVector() {
+inline Vector<const int8_t> ValueHelper::GetVector() {
   return int8_vector();
 }
 
 template <>
-constexpr Vector<const uint8_t> ValueHelper::GetVector() {
+inline Vector<const uint8_t> ValueHelper::GetVector() {
   return uint8_vector();
 }
 
 template <>
-constexpr Vector<const int16_t> ValueHelper::GetVector() {
+inline Vector<const int16_t> ValueHelper::GetVector() {
   return int16_vector();
 }
 
 template <>
-constexpr Vector<const uint16_t> ValueHelper::GetVector() {
+inline Vector<const uint16_t> ValueHelper::GetVector() {
   return uint16_vector();
 }
 
 template <>
-constexpr Vector<const int32_t> ValueHelper::GetVector() {
+inline Vector<const int32_t> ValueHelper::GetVector() {
   return int32_vector();
 }
 
 template <>
-constexpr Vector<const uint32_t> ValueHelper::GetVector() {
+inline Vector<const uint32_t> ValueHelper::GetVector() {
   return uint32_vector();
 }
 
 template <>
-constexpr Vector<const int64_t> ValueHelper::GetVector() {
+inline Vector<const int64_t> ValueHelper::GetVector() {
   return int64_vector();
 }
 

test/cctest/test-serialize.cc

@@ -186,12 +186,11 @@ static StartupBlobs Serialize(v8::Isolate* isolate) {
           WritePayload(read_only_snapshot.RawData())};
 }
 
-
-Vector<const uint8_t> ConstructSource(Vector<const uint8_t> head,
-                                      Vector<const uint8_t> body,
-                                      Vector<const uint8_t> tail, int repeats) {
-  int source_length = head.length() + body.length() * repeats + tail.length();
-  uint8_t* source = NewArray<uint8_t>(static_cast<size_t>(source_length));
+Vector<const char> ConstructSource(Vector<const char> head,
+                                   Vector<const char> body,
+                                   Vector<const char> tail, int repeats) {
+  size_t source_length = head.size() + body.size() * repeats + tail.size();
+  char* source = NewArray<char>(source_length);
   CopyChars(source, head.begin(), head.length());
   for (int i = 0; i < repeats; i++) {
     CopyChars(source + head.length() + i * body.length(), body.begin(),
@@ -199,8 +198,7 @@ Vector<const uint8_t> ConstructSource(Vector<const uint8_t> head,
   }
   CopyChars(source + head.length() + repeats * body.length(), tail.begin(),
             tail.length());
-  return Vector<const uint8_t>(const_cast<const uint8_t*>(source),
-                               source_length);
+  return VectorOf(source, source_length);
 }
 
 static v8::Isolate* Deserialize(const StartupBlobs& blobs) {
@@ -511,12 +509,12 @@ static void PartiallySerializeCustomContext(
           "var p = 0;"
           "(async ()=>{ p = await 42; })();");
 
-      Vector<const uint8_t> source = ConstructSource(
+      Vector<const char> source = ConstructSource(
           StaticCharVector("function g() { return [,"), StaticCharVector("1,"),
           StaticCharVector("];} a = g(); b = g(); b.push(1);"), 100000);
-      v8::MaybeLocal<v8::String> source_str = v8::String::NewFromOneByte(
-          v8_isolate, source.begin(), v8::NewStringType::kNormal,
-          source.length());
+      v8::MaybeLocal<v8::String> source_str =
+          v8::String::NewFromUtf8(v8_isolate, source.begin(),
+                                  v8::NewStringType::kNormal, source.length());
       CompileRun(source_str.ToLocalChecked());
       source.Dispose();
     }
@@ -1496,14 +1494,13 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobImmortalImmovableRoots) {
   // Flood the startup snapshot with shared function infos. If they are
   // serialized before the immortal immovable root, the root will no longer end
   // up on the first page.
-  Vector<const uint8_t> source =
+  Vector<const char> source =
       ConstructSource(StaticCharVector("var a = [];"),
                       StaticCharVector("a.push(function() {return 7});"),
                       StaticCharVector("\0"), 10000);
 
   DisableEmbeddedBlobRefcounting();
-  v8::StartupData data =
-      CreateSnapshotDataBlob(reinterpret_cast<const char*>(source.begin()));
+  v8::StartupData data = CreateSnapshotDataBlob(source.begin());
 
   v8::Isolate::CreateParams params;
   params.snapshot_blob = &data;
@@ -1804,13 +1801,13 @@ TEST(CodeSerializerLargeCodeObject) {
   // code. Don't even bother generating optimized code to avoid timeouts.
   FLAG_always_opt = false;
 
-  Vector<const uint8_t> source = ConstructSource(
+  Vector<const char> source = ConstructSource(
       StaticCharVector("var j=1; if (j == 0) {"),
       StaticCharVector(
           "for (let i of Object.prototype) for (let k = 0; k < 0; ++k);"),
       StaticCharVector("} j=7; j"), 2000);
   Handle<String> source_str =
-      isolate->factory()->NewStringFromOneByte(source).ToHandleChecked();
+      isolate->factory()->NewStringFromUtf8(source).ToHandleChecked();
 
   Handle<JSObject> global(isolate->context().global_object(), isolate);
   ScriptData* cache = nullptr;
@@ -1860,12 +1857,12 @@ TEST(CodeSerializerLargeCodeObjectWithIncrementalMarking) {
 
   v8::HandleScope scope(CcTest::isolate());
 
-  Vector<const uint8_t> source = ConstructSource(
+  Vector<const char> source = ConstructSource(
       StaticCharVector("var j=1; if (j == 0) {"),
       StaticCharVector("for (var i = 0; i < Object.prototype; i++);"),
       StaticCharVector("} j=7; var s = 'happy_hippo'; j"), 20000);
   Handle<String> source_str =
-      isolate->factory()->NewStringFromOneByte(source).ToHandleChecked();
+      isolate->factory()->NewStringFromUtf8(source).ToHandleChecked();
 
   // Create a string on an evacuation candidate in old space.
   Handle<String> moving_object;
@@ -1922,6 +1919,7 @@ TEST(CodeSerializerLargeCodeObjectWithIncrementalMarking) {
   delete cache;
   source.Dispose();
 }
+
 TEST(CodeSerializerLargeStrings) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();
@@ -1931,15 +1929,15 @@ TEST(CodeSerializerLargeStrings) {
 
   v8::HandleScope scope(CcTest::isolate());
 
-  Vector<const uint8_t> source_s = ConstructSource(
+  Vector<const char> source_s = ConstructSource(
       StaticCharVector("var s = \""), StaticCharVector("abcdef"),
       StaticCharVector("\";"), 1000000);
-  Vector<const uint8_t> source_t = ConstructSource(
+  Vector<const char> source_t = ConstructSource(
       StaticCharVector("var t = \""), StaticCharVector("uvwxyz"),
       StaticCharVector("\"; s + t"), 999999);
   Handle<String> source_str =
-      f->NewConsString(f->NewStringFromOneByte(source_s).ToHandleChecked(),
-                       f->NewStringFromOneByte(source_t).ToHandleChecked())
+      f->NewConsString(f->NewStringFromUtf8(source_s).ToHandleChecked(),
+                       f->NewStringFromUtf8(source_t).ToHandleChecked())
           .ToHandleChecked();
 
   Handle<JSObject> global(isolate->context().global_object(), isolate);
@@ -1992,23 +1990,23 @@ TEST(CodeSerializerThreeBigStrings) {
   const int32_t length_of_b = kMaxRegularHeapObjectSize / 2;
   const int32_t length_of_c = kMaxRegularHeapObjectSize / 2;
 
-  Vector<const uint8_t> source_a =
+  Vector<const char> source_a =
       ConstructSource(StaticCharVector("var a = \""), StaticCharVector("a"),
                       StaticCharVector("\";"), length_of_a);
   Handle<String> source_a_str =
-      f->NewStringFromOneByte(source_a).ToHandleChecked();
+      f->NewStringFromUtf8(source_a).ToHandleChecked();
 
-  Vector<const uint8_t> source_b =
+  Vector<const char> source_b =
       ConstructSource(StaticCharVector("var b = \""), StaticCharVector("b"),
                       StaticCharVector("\";"), length_of_b);
   Handle<String> source_b_str =
-      f->NewStringFromOneByte(source_b).ToHandleChecked();
+      f->NewStringFromUtf8(source_b).ToHandleChecked();
 
-  Vector<const uint8_t> source_c =
+  Vector<const char> source_c =
       ConstructSource(StaticCharVector("var c = \""), StaticCharVector("c"),
                       StaticCharVector("\";"), length_of_c);
   Handle<String> source_c_str =
-      f->NewStringFromOneByte(source_c).ToHandleChecked();
+      f->NewStringFromUtf8(source_c).ToHandleChecked();
 
   Handle<String> source_str =
       f->NewConsString(
@@ -2178,10 +2176,10 @@ TEST(CodeSerializerLargeExternalString) {
   v8::HandleScope scope(CcTest::isolate());
 
   // Create a huge external internalized string to use as variable name.
-  Vector<const uint8_t> string =
+  Vector<const char> string =
       ConstructSource(StaticCharVector(""), StaticCharVector("abcdef"),
                       StaticCharVector(""), 999999);
-  Handle<String> name = f->NewStringFromOneByte(string).ToHandleChecked();
+  Handle<String> name = f->NewStringFromUtf8(string).ToHandleChecked();
   SerializerOneByteResource one_byte_resource(
       reinterpret_cast<const char*>(string.begin()), string.length());
   name = f->InternalizeString(name);

test/unittests/heap/off-thread-factory-unittest.cc

@@ -123,7 +123,7 @@ class OffThreadFactoryTest : public TestWithIsolateAndZone {
 };
 
 TEST_F(OffThreadFactoryTest, OneByteInternalizedString_IsAddedToStringTable) {
-  Vector<const uint8_t> string_vector = StaticCharVector("foo");
+  Vector<const uint8_t> string_vector = StaticOneByteVector("foo");
   uint32_t hash_field = StringHasher::HashSequentialString<uint8_t>(
       string_vector.begin(), string_vector.length(), HashSeed(isolate()));
 
@@ -162,7 +162,7 @@ TEST_F(OffThreadFactoryTest, OneByteInternalizedString_IsAddedToStringTable) {
 
 TEST_F(OffThreadFactoryTest,
        OneByteInternalizedString_DuplicateIsDeduplicated) {
-  Vector<const uint8_t> string_vector = StaticCharVector("foo");
+  Vector<const uint8_t> string_vector = StaticOneByteVector("foo");
   uint32_t hash_field = StringHasher::HashSequentialString<uint8_t>(
       string_vector.begin(), string_vector.length(), HashSeed(isolate()));
 

test/unittests/utils/vector-unittest.cc

@@ -87,10 +87,8 @@ TEST(VectorTest, ConstexprFactories) {
   STATIC_ASSERT(kVec2.size() == 2);
   EXPECT_THAT(kVec2, testing::ElementsAre(4, 11));
 
-  // TODO(clemensb): Use StaticCharVector.
   static constexpr const char kInit3[] = "foobar";
-  static constexpr auto kVec3 =
-      VectorOf(kInit3, 6);  // StaticCharVector(kInit3);
+  static constexpr auto kVec3 = StaticCharVector(kInit3);
   STATIC_ASSERT(kVec3.size() == 6);
   EXPECT_THAT(kVec3, testing::ElementsAreArray(kInit3, kInit3 + 6));
 }