[api] Implement [[HostDefined]] for Synthetic Module Records [v8/v8 : main]
0 views
Skip to first unread message
Olivier Flückiger (Gerrit)
Jun 9, 2026, 10:12:34 AM (2 days ago) Jun 9
to Camillo Bruni, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Camillo Bruni
New activity on the change![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Camillo Bruni
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olivier Flückiger (Gerrit)
Jun 9, 2026, 10:13:28 AM (2 days ago) Jun 9
to Camillo Bruni, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Camillo Bruni
Olivier Flückiger added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 5 (Latest):
Olivier Flückiger . resolved
btw. here is the related chrome change:
https://chromium-review.googlesource.com/c/chromium/src/+/7913167
Related details
Attention is currently required from:
- Camillo Bruni
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Camillo Bruni (Gerrit)
Jun 10, 2026, 10:04:07 AM (19 hours ago) Jun 10
to Olivier Flückiger, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Olivier Flückiger
Camillo Bruni voted and added 2 comments![]( "Open in Gerrit")
Votes added by Camillo Bruni
| Code-Review | +1 |
2 comments
File include/v8-script.h
Line 371, Patchset 5 (Latest):
* the host defined options from the ScriptOrigin for SourceTextModules.Camillo Bruni . resolved
if I remember ScriptOrigin is the wrong source for this (since it should be context allocated for scripts / adn probably module record allocated for )SourceTextModules. but fine as it the status quo.
File src/api/api.cc
Line 2523, Patchset 5 (Latest):
i::Cast<i::SourceTextModule>(self)->GetScript()->host_defined_options(),Camillo Bruni . unresolved
nit: maybe this could be a getter on SourceTextModule for now, so we don't have a explicit dependency on i::Script here
Related details
Attention is currently required from:
- Olivier Flückiger
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Camillo Bruni (Gerrit)
Jun 10, 2026, 10:04:07 AM (19 hours ago) Jun 10
to Olivier Flückiger, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Olivier Flückiger
Camillo Bruni voted Code-Review+0![]( "Open in Gerrit")
| Code-Review | +0 |
Related details
Attention is currently required from:
- Olivier Flückiger
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olivier Flückiger (Gerrit)
Jun 10, 2026, 10:35:33 AM (19 hours ago) Jun 10
to Camillo Bruni, v8-s...@luci-project-accounts.iam.gserviceaccount.com, Hannes Payer, cbruni...@chromium.org, mlippau...@chromium.org, v8-re...@googlegroups.com
Attention needed from Camillo Bruni
Olivier Flückiger voted and added 3 comments![]( "Open in Gerrit")
Votes added by Olivier Flückiger
| Auto-Submit | +1 |
| Commit-Queue | +1 |
3 comments
Patchset-level comments
File-level comment, Patchset 6 (Latest):
Olivier Flückiger . resolved
as discussed offline, now only for synthetic modules to avoid future issues.
ptal again.
File include/v8-script.h
Line 371, Patchset 5:
* the host defined options from the ScriptOrigin for SourceTextModules.Camillo Bruni . resolved
if I remember ScriptOrigin is the wrong source for this (since it should be context allocated for scripts / adn probably module record allocated for )SourceTextModules. but fine as it the status quo.
Olivier Flückiger
Ok, let's entirely avoid this future problem by only exposing the host defined options for synthetic modules.
File src/api/api.cc
Line 2523, Patchset 5:
i::Cast<i::SourceTextModule>(self)->GetScript()->host_defined_options(),Camillo Bruni . resolved
nit: maybe this could be a getter on SourceTextModule for now, so we don't have a explicit dependency on i::Script here
Attention is currently required from:
- Camillo Bruni
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages