Skip to content

Commit a054817

Browse files
Aditi-1400aduh95
authored andcommitted
src: update std::vector<v8::Local<T>> to use v8::LocalVector<T>
A follow up of #57578 to replace all std::vector<v8::Local<T>> to use v8::LocalVector<T> PR-URL: #57642 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent d54ff41 commit a054817

File tree

5 files changed

+57
-29
lines changed

5 files changed

+57
-29
lines changed

src/README.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,25 @@ is done executing. `Local` handles can only be allocated on the C++ stack.
151151
Most of the V8 API uses `Local` handles to work with JavaScript values or return
152152
them from functions.
153153

154+
Additionally, according to [V8 public API documentation][`v8::Local<T>`], local handles
155+
(`v8::Local<T>`) should **never** be allocated on the heap.
156+
157+
This disallows heap-allocated data structures containing instances of `v8::Local`
158+
159+
For example:
160+
161+
```cpp
162+
// Don't do this
163+
std::vector<v8::Local<v8::Value>> v1;
164+
```
165+
166+
Instead, it is recommended to use `v8::LocalVector<T>` provided by V8
167+
for such scenarios:
168+
169+
```cpp
170+
v8::LocalVector<v8::Value> v1(isolate);
171+
```
172+
154173
Whenever a `Local` handle is created, a `v8::HandleScope` or
155174
`v8::EscapableHandleScope` object must exist on the stack. The `Local` is then
156175
added to that scope and deleted along with it.
@@ -1176,6 +1195,7 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
11761195
[`v8.h` in Code Search]: https://cs.chromium.org/chromium/src/v8/include/v8.h
11771196
[`v8.h` in Node.js]: https://github.com/nodejs/node/blob/HEAD/deps/v8/include/v8.h
11781197
[`v8.h` in V8]: https://github.com/v8/v8/blob/HEAD/include/v8.h
1198+
[`v8::Local<T>`]: https://v8.github.io/api/head/classv8_1_1Local.html
11791199
[`vm` module]: https://nodejs.org/api/vm.html
11801200
[binding function]: #binding-functions
11811201
[cleanup hooks]: #cleanup-hooks

src/node_contextify.cc

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,7 +1165,7 @@ Maybe<void> StoreCodeCacheResult(
11651165
MaybeLocal<Function> CompileFunction(Local<Context> context,
11661166
Local<String> filename,
11671167
Local<String> content,
1168-
std::vector<Local<String>>* parameters) {
1168+
LocalVector<String>* parameters) {
11691169
ScriptOrigin script_origin(filename, 0, 0, true);
11701170
ScriptCompiler::Source script_source(content, script_origin);
11711171

@@ -1474,7 +1474,7 @@ void ContextifyContext::CompileFunction(
14741474
Context::Scope scope(parsing_context);
14751475

14761476
// Read context extensions from buffer
1477-
std::vector<Local<Object>> context_extensions;
1477+
LocalVector<Object> context_extensions(isolate);
14781478
if (!context_extensions_buf.IsEmpty()) {
14791479
for (uint32_t n = 0; n < context_extensions_buf->Length(); n++) {
14801480
Local<Value> val;
@@ -1485,7 +1485,7 @@ void ContextifyContext::CompileFunction(
14851485
}
14861486

14871487
// Read params from params buffer
1488-
std::vector<Local<String>> params;
1488+
LocalVector<String> params(isolate);
14891489
if (!params_buf.IsEmpty()) {
14901490
for (uint32_t n = 0; n < params_buf->Length(); n++) {
14911491
Local<Value> val;
@@ -1517,22 +1517,24 @@ void ContextifyContext::CompileFunction(
15171517
args.GetReturnValue().Set(result);
15181518
}
15191519

1520-
static std::vector<Local<String>> GetCJSParameters(IsolateData* data) {
1521-
return {
1522-
data->exports_string(),
1523-
data->require_string(),
1524-
data->module_string(),
1525-
data->__filename_string(),
1526-
data->__dirname_string(),
1527-
};
1520+
static LocalVector<String> GetCJSParameters(IsolateData* data) {
1521+
LocalVector<String> result(data->isolate(),
1522+
{
1523+
data->exports_string(),
1524+
data->require_string(),
1525+
data->module_string(),
1526+
data->__filename_string(),
1527+
data->__dirname_string(),
1528+
});
1529+
return result;
15281530
}
15291531

15301532
Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
15311533
Environment* env,
15321534
Local<Context> parsing_context,
15331535
ScriptCompiler::Source* source,
1534-
std::vector<Local<String>> params,
1535-
std::vector<Local<Object>> context_extensions,
1536+
LocalVector<String> params,
1537+
LocalVector<Object> context_extensions,
15361538
ScriptCompiler::CompileOptions options,
15371539
bool produce_cached_data,
15381540
Local<Symbol> id_symbol,
@@ -1668,7 +1670,7 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(
16681670
options = ScriptCompiler::kConsumeCodeCache;
16691671
}
16701672

1671-
std::vector<Local<String>> params;
1673+
LocalVector<String> params(isolate);
16721674
if (is_cjs_scope) {
16731675
params = GetCJSParameters(env->isolate_data());
16741676
}

src/node_contextify.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ class ContextifyContext : public BaseObject {
9292
Environment* env,
9393
v8::Local<v8::Context> parsing_context,
9494
v8::ScriptCompiler::Source* source,
95-
std::vector<v8::Local<v8::String>> params,
96-
std::vector<v8::Local<v8::Object>> context_extensions,
95+
v8::LocalVector<v8::String> params,
96+
v8::LocalVector<v8::Object> context_extensions,
9797
v8::ScriptCompiler::CompileOptions options,
9898
bool produce_cached_data,
9999
v8::Local<v8::Symbol> id_symbol,
@@ -189,7 +189,7 @@ v8::MaybeLocal<v8::Function> CompileFunction(
189189
v8::Local<v8::Context> context,
190190
v8::Local<v8::String> filename,
191191
v8::Local<v8::String> content,
192-
std::vector<v8::Local<v8::String>>* parameters);
192+
v8::LocalVector<v8::String>* parameters);
193193

194194
} // namespace contextify
195195
} // namespace node

src/node_sea.cc

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ using v8::FunctionCallbackInfo;
3636
using v8::HandleScope;
3737
using v8::Isolate;
3838
using v8::Local;
39+
using v8::LocalVector;
3940
using v8::MaybeLocal;
4041
using v8::NewStringType;
4142
using v8::Object;
@@ -450,13 +451,15 @@ std::optional<std::string> GenerateCodeCache(std::string_view main_path,
450451
return std::nullopt;
451452
}
452453

453-
std::vector<Local<String>> parameters = {
454-
FIXED_ONE_BYTE_STRING(isolate, "exports"),
455-
FIXED_ONE_BYTE_STRING(isolate, "require"),
456-
FIXED_ONE_BYTE_STRING(isolate, "module"),
457-
FIXED_ONE_BYTE_STRING(isolate, "__filename"),
458-
FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
459-
};
454+
LocalVector<String> parameters(
455+
isolate,
456+
{
457+
FIXED_ONE_BYTE_STRING(isolate, "exports"),
458+
FIXED_ONE_BYTE_STRING(isolate, "require"),
459+
FIXED_ONE_BYTE_STRING(isolate, "module"),
460+
FIXED_ONE_BYTE_STRING(isolate, "__filename"),
461+
FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
462+
});
460463

461464
// TODO(RaisinTen): Using the V8 code cache prevents us from using `import()`
462465
// in the SEA code. Support it.

src/node_snapshotable.cc

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ using v8::FunctionCallbackInfo;
4141
using v8::HandleScope;
4242
using v8::Isolate;
4343
using v8::Local;
44+
using v8::LocalVector;
4445
using v8::Object;
4546
using v8::ObjectTemplate;
4647
using v8::SnapshotCreator;
@@ -1479,11 +1480,13 @@ void CompileSerializeMain(const FunctionCallbackInfo<Value>& args) {
14791480
Local<Context> context = isolate->GetCurrentContext();
14801481
// TODO(joyeecheung): do we need all of these? Maybe we would want a less
14811482
// internal version of them.
1482-
std::vector<Local<String>> parameters = {
1483-
FIXED_ONE_BYTE_STRING(isolate, "require"),
1484-
FIXED_ONE_BYTE_STRING(isolate, "__filename"),
1485-
FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
1486-
};
1483+
LocalVector<String> parameters(
1484+
isolate,
1485+
{
1486+
FIXED_ONE_BYTE_STRING(isolate, "require"),
1487+
FIXED_ONE_BYTE_STRING(isolate, "__filename"),
1488+
FIXED_ONE_BYTE_STRING(isolate, "__dirname"),
1489+
});
14871490
Local<Function> fn;
14881491
if (contextify::CompileFunction(context, filename, source, &parameters)
14891492
.ToLocal(&fn)) {

0 commit comments

Comments
 (0)