Skip to content

src: update std::vector<v8::Local<T>> to use v8::LocalVector<T> - #57578

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Aditi-1400:replace-vector
Apr 16, 2025
Merged

src: update std::vector<v8::Local<T>> to use v8::LocalVector<T>#57578
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
Aditi-1400:replace-vector

Conversation

@Aditi-1400

Copy link
Copy Markdown
Contributor

According to V8's public API documentation, local handles (i.e., objects of type v8::Local) "should never be allocated on the heap". This replaces the usage of heap-allocated data structures containing instances of v8::Local, specifically the std::vector<v8::Local<v8::String>> with recently introduced v8::LocalVector<T>.

This is first of the series of commits to replace all std::vector<v8::Local<T>> to use v8::LocalVector<T>.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Mar 21, 2025
Comment threadsrc/node_builtins.cc Outdated
@anonrig
anonrig requested a review from jasnellMarch 21, 2025 15:33
@codecov

codecovBot commented Mar 21, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.22%. Comparing base (922ce9d) to head (c8f2584).
Report is 183 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #57578 +/- ##
==========================================
- Coverage 90.23% 90.22% -0.02% 
==========================================
Files 629 630 +1 Lines 184939 185054 +115 Branches 36232 36249 +17 ==========================================
+ Hits 166885 166961 +76 - Misses 11011 11036 +25 - Partials 7043 7057 +14 
Files with missing linesCoverage Δ
src/node_builtins.cc79.12% <100.00%> (+1.13%)⬆️
src/node_builtins.h100.00% <ø> (ø)

... and 57 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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 replaces the usage of heap-allocated data
structures containing instances of `v8::Local`, specifically the
`std::vector<v8::Local<v8::String>>` with recently
introduced `v8::LocalVector<T>`.
This is first of the series of commits to replace all
`std::vector<v8::Local<T>>` to use `v8::LocalVector<T>`.
Aditi-1400 added a commit to Aditi-1400/node that referenced this pull request Mar 27, 2025
Aditi-1400 added a commit to Aditi-1400/node that referenced this pull request Mar 27, 2025
Aditi-1400 added a commit to Aditi-1400/node that referenced this pull request Apr 2, 2025
A follow up of nodejs#57578 to
replace all std::vector<v8::Local<T>> to use v8::LocalVector<T>
nodejs-github-bot pushed a commit that referenced this pull request Apr 5, 2025
Refs: #57578
PR-URL: #57733
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
@joyeecheungjoyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 10, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 10, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

RafaelGSS pushed a commit that referenced this pull request May 14, 2025
Refs: #57578
PR-URL: #57646
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request May 16, 2025
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 replaces the usage of heap-allocated data
structures containing instances of `v8::Local`, specifically the
`std::vector<v8::Local<v8::String>>` with recently
introduced `v8::LocalVector<T>`.
This is first of the series of commits to replace all
`std::vector<v8::Local<T>>` to use `v8::LocalVector<T>`.
PR-URL: #57578
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request May 16, 2025
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>
aduh95 pushed a commit that referenced this pull request May 16, 2025
Refs: #57578
PR-URL: #57646
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
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 replaces the usage of heap-allocated data
structures containing instances of `v8::Local`, specifically the
`std::vector<v8::Local<v8::String>>` with recently
introduced `v8::LocalVector<T>`.
This is first of the series of commits to replace all
`std::vector<v8::Local<T>>` to use `v8::LocalVector<T>`.
PR-URL: #57578
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
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>
aduh95 pushed a commit that referenced this pull request May 17, 2025
Refs: #57578
PR-URL: #57646
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request May 18, 2025
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 replaces the usage of heap-allocated data
structures containing instances of `v8::Local`, specifically the
`std::vector<v8::Local<v8::String>>` with recently
introduced `v8::LocalVector<T>`.
This is first of the series of commits to replace all
`std::vector<v8::Local<T>>` to use `v8::LocalVector<T>`.
PR-URL: #57578
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request May 18, 2025
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>
aduh95 pushed a commit that referenced this pull request May 18, 2025
Refs: #57578
PR-URL: #57646
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
aduh95 pushed a commit that referenced this pull request May 19, 2025
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 replaces the usage of heap-allocated data
structures containing instances of `v8::Local`, specifically the
`std::vector<v8::Local<v8::String>>` with recently
introduced `v8::LocalVector<T>`.
This is first of the series of commits to replace all
`std::vector<v8::Local<T>>` to use `v8::LocalVector<T>`.
PR-URL: #57578
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request May 19, 2025
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>
aduh95 pushed a commit that referenced this pull request May 19, 2025
Refs: #57578
PR-URL: #57646
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request May 23, 2025
codebytere added a commit to electron/electron that referenced this pull request May 26, 2025
codebytere added a commit to electron/electron that referenced this pull request May 29, 2025
codebytere added a commit to electron/electron that referenced this pull request Jun 2, 2025
jkleinsc pushed a commit to electron/electron that referenced this pull request Jun 2, 2025
* chore: bump node in DEPS to v22.16.0
* crypto: remove BoringSSL dh-primes addition
nodejs/node#57023
* tools: enable linter in test/fixtures/test\-runner/output
nodejs/node#57698
* src: improve thread safety of TaskQueue
nodejs/node#57910
* buffer: define global v8::CFunction objects as const
nodejs/node#57676
* src: disable abseil deadlock detection
nodejs/node#57582
* zlib: fix pointer alignment
nodejs/node#57727
* chore: fixup patch indices
* src: set default config as node.config.json
nodejs/node#57171
* src: update std::vector<v8::Local<T>> to use v8::LocalVector<T>
nodejs/node#57578
* test: disable chmod tests failing in Docker
nodejs/node#58326
---------
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
@ghostghost mentioned this pull request Jun 8, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@Aditi-1400@nodejs-github-bot@jasnell@anonrig@legendecas@RaisinTen@joyeecheung