Skip to content

Commit 8ff02b7

Browse files
nickieV8 LUCI CQ
nickie
authored and
V8 LUCI CQ
committed
[api] Deprecate vector<v8::Local>, part 1
According to V8's public API documentation, local handles (i.e., objects of type v8::Local<T>) "should never be allocated on the heap". This disallows heap-allocated data structures containing instances of v8::Local, like std::vector<v8::Local<v8::String>>. It is unfortunate that the V8 API itself requires the usage of such data structures. This CL starts a deprecation of such data structures from V8's public API. It replaces the std::vector<Local<String>>, used as parameter to Module::CreateSyntheticModule, with a MemorySpan<const Local<String>>. This stands for a (contiguously allocated) array of local handles to strings, which will not be modified. The size of the array is known, as part of the MemorySpan object. The CL also replaces some vectors of locals with arrays of locals in API tests. Moreover, the CL adds some functionality to the existing v8::MemorySpan class, so that it behaves more like the upcoming (in C++20) std::span<T, std::dynamic_extent>. In particular, it makes spans subscriptable, iterable, and allows implicit conversion from arrays. It also introduces a helper function template v8::to_array<T> (the upcoming std::to_array<T>, in C++20), to better support automatic deduction of array size without omitting the element type. Bug: chromium:1454114 Change-Id: Iaa36da8199428a9120259aa2a3a4543e261fbc60 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4866222 Commit-Queue: Nikolaos Papaspyrou <[email protected]> Reviewed-by: Anton Bikineev <[email protected]> Reviewed-by: Michael Lippautz <[email protected]> Cr-Commit-Position: refs/heads/main@{#90056}
1 parent a0c17a9 commit 8ff02b7

5 files changed

Lines changed: 185 additions & 20 deletions

File tree

‎include/v8-memory-span.h‎

Lines changed: 152 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,16 @@
77

88
#include <stddef.h>
99

10+
#include <array>
11+
#include <iterator>
12+
#include <type_traits>
13+
1014
#include "v8config.h" // NOLINT(build/include_directory)
1115

1216
namespace v8 {
1317

1418
/**
15-
* Points to an unowned continous buffer holding a known number of elements.
19+
* Points to an unowned contiguous buffer holding a known number of elements.
1620
*
1721
* This is similar to std::span (under consideration for C++20), but does not
1822
* require advanced C++ support. In the (far) future, this may be replaced with
@@ -23,21 +27,167 @@ namespace v8 {
2327
*/
2428
template <typename T>
2529
class V8_EXPORT MemorySpan {
30+
private:
31+
/** Some C++ machinery, brought from the future. */
32+
template <typename From, typename To>
33+
using is_array_convertible = std::is_convertible<From (*)[], To (*)[]>;
34+
template <typename From, typename To>
35+
static constexpr bool is_array_convertible_v =
36+
is_array_convertible<From, To>::value;
37+
38+
template <typename It>
39+
using iter_reference_t = decltype(*std::declval<It&>());
40+
41+
template <typename It, typename = void>
42+
struct is_compatible_iterator : std::false_type {};
43+
template <typename It>
44+
struct is_compatible_iterator<
45+
It,
46+
std::void_t<
47+
std::is_base_of<std::random_access_iterator_tag,
48+
typename std::iterator_traits<It>::iterator_category>,
49+
is_array_convertible<std::remove_reference_t<iter_reference_t<It>>,
50+
T>>> : std::true_type {};
51+
template <typename It>
52+
static constexpr bool is_compatible_iterator_v =
53+
is_compatible_iterator<It>::value;
54+
55+
template <typename U>
56+
static constexpr U* to_address(U* p) noexcept {
57+
return p;
58+
}
59+
60+
template <typename It,
61+
typename = std::void_t<decltype(std::declval<It&>().operator->())>>
62+
static constexpr auto to_address(It it) noexcept {
63+
return it.operator->();
64+
}
65+
2666
public:
2767
/** The default constructor creates an empty span. */
2868
constexpr MemorySpan() = default;
2969

30-
constexpr MemorySpan(T* data, size_t size) : data_(data), size_(size) {}
70+
/** Constructor from nullptr and count, for backwards compatibility.
71+
* This is not compatible with C++20 std::span.
72+
*/
73+
constexpr MemorySpan(std::nullptr_t, size_t) {}
74+
75+
/** Constructor from "iterator" and count. */
76+
template <typename Iterator,
77+
std::enable_if_t<is_compatible_iterator_v<Iterator>, bool> = true>
78+
constexpr MemorySpan(Iterator first,
79+
size_t count) // NOLINT(runtime/explicit)
80+
: data_(to_address(first)), size_(count) {}
81+
82+
/** Constructor from two "iterators". */
83+
template <typename Iterator,
84+
std::enable_if_t<is_compatible_iterator_v<Iterator> &&
85+
!std::is_convertible_v<Iterator, size_t>,
86+
bool> = true>
87+
constexpr MemorySpan(Iterator first,
88+
Iterator last) // NOLINT(runtime/explicit)
89+
: data_(to_address(first)), size_(last - first) {}
90+
91+
/** Implicit conversion from C-style array. */
92+
template <size_t N>
93+
constexpr MemorySpan(T (&a)[N]) noexcept // NOLINT(runtime/explicit)
94+
: data_(a), size_(N) {}
95+
96+
/** Implicit conversion from std::array. */
97+
template <typename U, size_t N,
98+
std::enable_if_t<is_array_convertible_v<U, T>, bool> = true>
99+
constexpr MemorySpan(
100+
std::array<U, N>& a) noexcept // NOLINT(runtime/explicit)
101+
: data_(a.data()), size_{N} {}
102+
103+
/** Implicit conversion from const std::array. */
104+
template <typename U, size_t N,
105+
std::enable_if_t<is_array_convertible_v<const U, T>, bool> = true>
106+
constexpr MemorySpan(
107+
const std::array<U, N>& a) noexcept // NOLINT(runtime/explicit)
108+
: data_(a.data()), size_{N} {}
31109

32110
/** Returns a pointer to the beginning of the buffer. */
33111
constexpr T* data() const { return data_; }
34112
/** Returns the number of elements that the buffer holds. */
35113
constexpr size_t size() const { return size_; }
36114

115+
constexpr T& operator[](size_t i) const { return data_[i]; }
116+
117+
class Iterator {
118+
public:
119+
using iterator_category = std::forward_iterator_tag;
120+
using value_type = T;
121+
using difference_type = std::ptrdiff_t;
122+
using pointer = value_type*;
123+
using reference = value_type&;
124+
125+
T& operator*() const { return *ptr_; }
126+
T* operator->() const { return ptr_; }
127+
128+
bool operator==(Iterator other) const { return ptr_ == other.ptr_; }
129+
bool operator!=(Iterator other) const { return !(*this == other); }
130+
131+
Iterator& operator++() {
132+
++ptr_;
133+
return *this;
134+
}
135+
136+
Iterator operator++(int) {
137+
Iterator temp(*this);
138+
++(*this);
139+
return temp;
140+
}
141+
142+
private:
143+
explicit Iterator(T* ptr) : ptr_(ptr) {}
144+
145+
T* ptr_ = nullptr;
146+
};
147+
148+
Iterator begin() const { return Iterator(data_); }
149+
Iterator end() const { return Iterator(data_ + size_); }
150+
37151
private:
38152
T* data_ = nullptr;
39153
size_t size_ = 0;
40154
};
41155

156+
/**
157+
* Helper function template to create an array of fixed length, initialized by
158+
* the provided initializer list, without explicitly specifying the array size,
159+
* e.g.
160+
*
161+
* auto arr = v8::to_array<Local<String>>({v8_str("one"), v8_str("two")});
162+
*
163+
* In the future, this may be replaced with or aliased to std::to_array (under
164+
* consideration for C++20).
165+
*/
166+
167+
namespace detail {
168+
template <class T, std::size_t N, std::size_t... I>
169+
constexpr std::array<std::remove_cv_t<T>, N> to_array_lvalue_impl(
170+
T (&a)[N], std::index_sequence<I...>) {
171+
return {{a[I]...}};
172+
}
173+
174+
template <class T, std::size_t N, std::size_t... I>
175+
constexpr std::array<std::remove_cv_t<T>, N> to_array_rvalue_impl(
176+
T (&&a)[N], std::index_sequence<I...>) {
177+
return {{std::move(a[I])...}};
178+
}
179+
} // namespace detail
180+
181+
template <class T, std::size_t N>
182+
constexpr std::array<std::remove_cv_t<T>, N> to_array(T (&a)[N]) {
183+
return detail::to_array_lvalue_impl(a, std::make_index_sequence<N>{});
184+
}
185+
186+
template <class T, std::size_t N>
187+
constexpr std::array<std::remove_cv_t<T>, N> to_array(T (&&a)[N]) {
188+
return detail::to_array_rvalue_impl(std::move(a),
189+
std::make_index_sequence<N>{});
190+
}
191+
42192
} // namespace v8
43193
#endif // INCLUDE_V8_MEMORY_SPAN_H_

‎include/v8-script.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "v8-data.h" // NOLINT(build/include_directory)
1717
#include "v8-local-handle.h" // NOLINT(build/include_directory)
1818
#include "v8-maybe.h" // NOLINT(build/include_directory)
19+
#include "v8-memory-span.h" // NOLINT(build/include_directory)
1920
#include "v8-message.h" // NOLINT(build/include_directory)
2021
#include "v8config.h" // NOLINT(build/include_directory)
2122

@@ -285,10 +286,15 @@ class V8_EXPORT Module : public Data {
285286
* module_name is used solely for logging/debugging and doesn't affect module
286287
* behavior.
287288
*/
289+
V8_DEPRECATE_SOON("Please use the version that takes a MemorySpan")
288290
static Local<Module> CreateSyntheticModule(
289291
Isolate* isolate, Local<String> module_name,
290292
const std::vector<Local<String>>& export_names,
291293
SyntheticModuleEvaluationSteps evaluation_steps);
294+
static Local<Module> CreateSyntheticModule(
295+
Isolate* isolate, Local<String> module_name,
296+
const MemorySpan<const Local<String>>& export_names,
297+
SyntheticModuleEvaluationSteps evaluation_steps);
292298

293299
/**
294300
* Set this module's exported value for the name export_name to the specified

‎src/api/api.cc‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2444,7 +2444,17 @@ MaybeLocal<Value> Module::Evaluate(Local<Context> context) {
24442444

24452445
Local<Module> Module::CreateSyntheticModule(
24462446
Isolate* v8_isolate, Local<String> module_name,
2447-
const std::vector<Local<v8::String>>& export_names,
2447+
const std::vector<Local<String>>& export_names,
2448+
v8::Module::SyntheticModuleEvaluationSteps evaluation_steps) {
2449+
return CreateSyntheticModule(
2450+
v8_isolate, module_name,
2451+
MemorySpan<const Local<String>>(export_names.begin(), export_names.end()),
2452+
evaluation_steps);
2453+
}
2454+
2455+
Local<Module> Module::CreateSyntheticModule(
2456+
Isolate* v8_isolate, Local<String> module_name,
2457+
const MemorySpan<const Local<String>>& export_names,
24482458
v8::Module::SyntheticModuleEvaluationSteps evaluation_steps) {
24492459
auto i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
24502460
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate);

‎src/d8/d8.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,8 +1141,8 @@ MaybeLocal<Module> Shell::FetchModuleTree(Local<Module> referrer,
11411141
return MaybeLocal<Module>();
11421142
}
11431143

1144-
std::vector<Local<String>> export_names{
1145-
String::NewFromUtf8(isolate, "default").ToLocalChecked()};
1144+
auto export_names = v8::to_array<Local<String>>(
1145+
{String::NewFromUtf8(isolate, "default").ToLocalChecked()});
11461146

11471147
module = v8::Module::CreateSyntheticModule(
11481148
isolate,

‎test/cctest/test-api.cc‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5121,9 +5121,9 @@ THREADED_TEST(Array) {
51215121
array = v8::Array::New(context->GetIsolate(), -27);
51225122
CHECK_EQ(0u, array->Length());
51235123

5124-
std::vector<Local<Value>> vector = {v8_num(1), v8_num(2), v8_num(3)};
5125-
array = v8::Array::New(context->GetIsolate(), vector.data(), vector.size());
5126-
CHECK_EQ(vector.size(), array->Length());
5124+
auto numbers = v8::to_array<Local<Value>>({v8_num(1), v8_num(2), v8_num(3)});
5125+
array = v8::Array::New(context->GetIsolate(), numbers.data(), numbers.size());
5126+
CHECK_EQ(numbers.size(), array->Length());
51275127
CHECK_EQ(1, arr->Get(context.local(), 0)
51285128
.ToLocalChecked()
51295129
->Int32Value(context.local())
@@ -23781,7 +23781,7 @@ Local<Module> CompileAndInstantiateModule(v8::Isolate* isolate,
2378123781

2378223782
Local<Module> CreateAndInstantiateSyntheticModule(
2378323783
v8::Isolate* isolate, Local<String> module_name, Local<Context> context,
23784-
std::vector<v8::Local<v8::String>> export_names,
23784+
const v8::MemorySpan<const v8::Local<v8::String>>& export_names,
2378523785
v8::Module::SyntheticModuleEvaluationSteps evaluation_steps) {
2378623786
Local<Module> module = v8::Module::CreateSyntheticModule(
2378723787
isolate, module_name, export_names, evaluation_steps);
@@ -23815,7 +23815,7 @@ Local<Module> CompileAndInstantiateModuleFromCache(
2381523815
v8::MaybeLocal<Module> SyntheticModuleResolveCallback(
2381623816
Local<Context> context, Local<String> specifier,
2381723817
Local<FixedArray> import_assertions, Local<Module> referrer) {
23818-
std::vector<v8::Local<v8::String>> export_names{v8_str("test_export")};
23818+
auto export_names = v8::to_array<Local<v8::String>>({v8_str("test_export")});
2381923819
Local<Module> module = CreateAndInstantiateSyntheticModule(
2382023820
context->GetIsolate(),
2382123821
v8_str("SyntheticModuleResolveCallback-TestSyntheticModule"), context,
@@ -23826,7 +23826,7 @@ v8::MaybeLocal<Module> SyntheticModuleResolveCallback(
2382623826
v8::MaybeLocal<Module> SyntheticModuleThatThrowsDuringEvaluateResolveCallback(
2382723827
Local<Context> context, Local<String> specifier,
2382823828
Local<FixedArray> import_assertions, Local<Module> referrer) {
23829-
std::vector<v8::Local<v8::String>> export_names{v8_str("test_export")};
23829+
auto export_names = v8::to_array<Local<v8::String>>({v8_str("test_export")});
2383023830
Local<Module> module = CreateAndInstantiateSyntheticModule(
2383123831
context->GetIsolate(),
2383223832
v8_str("SyntheticModuleThatThrowsDuringEvaluateResolveCallback-"
@@ -23923,7 +23923,7 @@ TEST(CreateSyntheticModule) {
2392323923
v8::Local<v8::Context> context = v8::Context::New(isolate);
2392423924
v8::Context::Scope cscope(context);
2392523925

23926-
std::vector<v8::Local<v8::String>> export_names{v8_str("default")};
23926+
auto export_names = v8::to_array<Local<v8::String>>({v8_str("default")});
2392723927

2392823928
Local<Module> module = CreateAndInstantiateSyntheticModule(
2392923929
isolate, v8_str("CreateSyntheticModule-TestSyntheticModule"), context,
@@ -23964,7 +23964,7 @@ TEST(CreateSyntheticModuleGC) {
2396423964
v8::Local<v8::Context> context = v8::Context::New(isolate);
2396523965
v8::Context::Scope cscope(context);
2396623966

23967-
std::vector<v8::Local<v8::String>> export_names{v8_str("default")};
23967+
auto export_names = v8::to_array<Local<v8::String>>({v8_str("default")});
2396823968
v8::Local<v8::String> module_name =
2396923969
v8_str("CreateSyntheticModule-TestSyntheticModuleGC");
2397023970

@@ -23988,7 +23988,7 @@ TEST(CreateSyntheticModuleGCName) {
2398823988

2398923989
{
2399023990
v8::EscapableHandleScope inner_scope(isolate);
23991-
std::vector<v8::Local<v8::String>> export_names{v8_str("default")};
23991+
auto export_names = v8::to_array<Local<v8::String>>({v8_str("default")});
2399223992
v8::Local<v8::String> module_name =
2399323993
v8_str("CreateSyntheticModuleGCName-TestSyntheticModule");
2399423994
module = inner_scope.Escape(v8::Module::CreateSyntheticModule(
@@ -24015,7 +24015,7 @@ TEST(SyntheticModuleSetExports) {
2401524015

2401624016
Local<String> foo_string = v8_str("foo");
2401724017
Local<String> bar_string = v8_str("bar");
24018-
std::vector<v8::Local<v8::String>> export_names{foo_string};
24018+
auto export_names = v8::to_array<Local<v8::String>>({foo_string});
2401924019

2402024020
Local<Module> module = CreateAndInstantiateSyntheticModule(
2402124021
isolate, v8_str("SyntheticModuleSetExports-TestSyntheticModule"), context,
@@ -24059,8 +24059,7 @@ TEST(SyntheticModuleSetMissingExport) {
2405924059

2406024060
Local<Module> module = CreateAndInstantiateSyntheticModule(
2406124061
isolate, v8_str("SyntheticModuleSetExports-TestSyntheticModule"), context,
24062-
std::vector<v8::Local<v8::String>>(),
24063-
UnexpectedSyntheticModuleEvaluationStepsCallback);
24062+
{}, UnexpectedSyntheticModuleEvaluationStepsCallback);
2406424063

2406524064
i::Handle<i::SyntheticModule> i_module =
2406624065
i::Handle<i::SyntheticModule>::cast(v8::Utils::OpenHandle(*module));
@@ -24082,7 +24081,7 @@ TEST(SyntheticModuleEvaluationStepsNoThrow) {
2408224081
v8::Local<v8::Context> context = v8::Context::New(isolate);
2408324082
v8::Context::Scope cscope(context);
2408424083

24085-
std::vector<v8::Local<v8::String>> export_names{v8_str("default")};
24084+
auto export_names = v8::to_array<Local<v8::String>>({v8_str("default")});
2408624085

2408724086
Local<Module> module = CreateAndInstantiateSyntheticModule(
2408824087
isolate,
@@ -24104,7 +24103,7 @@ TEST(SyntheticModuleEvaluationStepsThrow) {
2410424103
v8::Local<v8::Context> context = CcTest::isolate()->GetCurrentContext();
2410524104
v8::Context::Scope cscope(context);
2410624105

24107-
std::vector<v8::Local<v8::String>> export_names{v8_str("default")};
24106+
auto export_names = v8::to_array<Local<v8::String>>({v8_str("default")});
2410824107

2410924108
Local<Module> module = CreateAndInstantiateSyntheticModule(
2411024109
isolate,
@@ -24130,7 +24129,7 @@ TEST(SyntheticModuleEvaluationStepsSetExport) {
2413024129
v8::Context::Scope cscope(context);
2413124130

2413224131
Local<String> test_export_string = v8_str("test_export");
24133-
std::vector<v8::Local<v8::String>> export_names{test_export_string};
24132+
auto export_names = v8::to_array<Local<v8::String>>({test_export_string});
2413424133

2413524134
Local<Module> module = CreateAndInstantiateSyntheticModule(
2413624135
isolate,

0 commit comments

Comments
 (0)