Restrict JSG_STRUCT from having v8 local members#4916
Merged
Conversation
jasnell
commented
Aug 27, 2025
|
The generated output of |
70963b2 to
db35d17
Compare
jasnell
commented
Aug 27, 2025
8c760ff to
89f0792
Compare
kentonv
reviewed
Aug 27, 2025
89f0792 to
60bef82
Compare
harrishancock
approved these changes
Aug 29, 2025
d52b02a to
6dfea8c
Compare
Make it a compile time error for a JSG_STRUCT to have members that are v8 locals (includig JsValues). It's not entirely safe for a JSG_STRUCT to use v8 locals because the struct may outlive the v8 HandleScope in which the locals were created. Let's enforce it at compile time. Update src/workerd/jsg/struct-test.c++
6dfea8c to
8514ca1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make it a compile time error for a JSG_STRUCT to have members that are v8 locals (including JsValues).
It's not entirely safe for a JSG_STRUCT to use v8 locals because the struct may outlive the v8 HandleScope in which the locals were created. Let's enforce it at compile time.
Refs: #4834 (comment)