Skip to content

Commit 54ff50f

Browse files
Aditi-1400Aditi-1400
committed
src: update std::vector<v8::Local<T>> to use v8::LocalVector<T>
A follow up of nodejs#57578 to replace all std::vector<v8::Local<T>> to use v8::LocalVector<T>
1 parent 4868ca4 commit 54ff50f

5 files changed

Lines changed: 57 additions & 29 deletions

File tree

‎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.
@@ -1409,6 +1428,7 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
14091428
[`v8.h` in Code Search]: https://cs.chromium.org/chromium/src/v8/include/v8.h
14101429
[`v8.h` in Node.js]: https://github.com/nodejs/node/blob/HEAD/deps/v8/include/v8.h
14111430
[`v8.h` in V8]: https://github.com/v8/v8/blob/HEAD/include/v8.h
1431+
[`v8::Local<T>`]: https://v8.github.io/api/head/classv8_1_1Local.html
14121432
[`vm` module]: https://nodejs.org/api/vm.html
14131433
[binding function]: #binding-functions
14141434
[cleanup hooks]: #cleanup-hooks

‎src/node_contextify.cc‎

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

@@ -1483,7 +1483,7 @@ void ContextifyContext::CompileFunction(
14831483
Context::Scope scope(parsing_context);
14841484

14851485
// Read context extensions from buffer
1486-
std::vector<Local<Object>> context_extensions;
1486+
LocalVector<Object> context_extensions(isolate);
14871487
if (!context_extensions_buf.IsEmpty()) {
14881488
for (uint32_t n = 0; n < context_extensions_buf->Length(); n++) {
14891489
Local<Value> val;
@@ -1494,7 +1494,7 @@ void ContextifyContext::CompileFunction(
14941494
}
14951495

14961496
// Read params from params buffer
1497-
std::vector<Local<String>> params;
1497+
LocalVector<String> params(isolate);
14981498
if (!params_buf.IsEmpty()) {
14991499
for (uint32_t n = 0; n < params_buf->Length(); n++) {
15001500
Local<Value> val;
@@ -1526,22 +1526,24 @@ void ContextifyContext::CompileFunction(
15261526
args.GetReturnValue().Set(result);
15271527
}
15281528

1529-
static std::vector<Local<String>> GetCJSParameters(IsolateData* data) {
1530-
return {
1531-
data->exports_string(),
1532-
data->require_string(),
1533-
data->module_string(),
1534-
data->__filename_string(),
1535-
data->__dirname_string(),
1536-
};
1529+
static LocalVector<String> GetCJSParameters(IsolateData* data) {
1530+
LocalVector<String> result(data->isolate(),
1531+
{
1532+
data->exports_string(),
1533+
data->require_string(),
1534+
data->module_string(),
1535+
data->__filename_string(),
1536+
data->__dirname_string(),
1537+
});
1538+
return result;
15371539
}
15381540

15391541
Local<Object> ContextifyContext::CompileFunctionAndCacheResult(
15401542
Environment* env,
15411543
Local<Context> parsing_context,
15421544
ScriptCompiler::Source* source,
1543-
std::vector<Local<String>> params,
1544-
std::vector<Local<Object>> context_extensions,
1545+
LocalVector<String> params,
1546+
LocalVector<Object> context_extensions,
15451547
ScriptCompiler::CompileOptions options,
15461548
bool produce_cached_data,
15471549
Local<Symbol> id_symbol,
@@ -1677,7 +1679,7 @@ static MaybeLocal<Function> CompileFunctionForCJSLoader(
16771679
options = ScriptCompiler::kConsumeCodeCache;
16781680
}
16791681

1680-
std::vector<Local<String>> params;
1682+
LocalVector<String> params(isolate);
16811683
if (is_cjs_scope) {
16821684
params = GetCJSParameters(env->isolate_data());
16831685
}

‎src/node_contextify.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ class ContextifyContext final : CPPGC_MIXIN(ContextifyContext) {
149149
Environment* env,
150150
v8::Local<v8::Context> parsing_context,
151151
v8::ScriptCompiler::Source* source,
152-
std::vector<v8::Local<v8::String>> params,
153-
std::vector<v8::Local<v8::Object>> context_extensions,
152+
v8::LocalVector<v8::String> params,
153+
v8::LocalVector<v8::Object> context_extensions,
154154
v8::ScriptCompiler::CompileOptions options,
155155
bool produce_cached_data,
156156
v8::Local<v8::Symbol> id_symbol,
@@ -244,7 +244,7 @@ v8::MaybeLocal<v8::Function> CompileFunction(
244244
v8::Local<v8::Context> context,
245245
v8::Local<v8::String> filename,
246246
v8::Local<v8::String> content,
247-
std::vector<v8::Local<v8::String>>* parameters);
247+
v8::LocalVector<v8::String>* parameters);
248248

249249
} // namespace contextify
250250
} // 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)