Marco Vitale (Gerrit)

unread,

Mar 13, 2026, 10:13:53 AM (3 days ago) Mar 13

to Michael Lippautz, V8 LUCI CQ, chrom...@appspot.gserviceaccount.com, marja...@chromium.org, v8-re...@googlegroups.com

Attention needed from Michael Lippautz

New activity on the change

Open in Gerrit

Related details

Attention is currently required from:

Michael Lippautz

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Michael Lippautz (Gerrit)

unread,

Mar 13, 2026, 10:25:03 AM (3 days ago) Mar 13

to Marco Vitale, Toon Verwaest, Jakob Linke, V8 LUCI CQ, chrom...@appspot.gserviceaccount.com, marja...@chromium.org, v8-re...@googlegroups.com

Attention needed from Marco Vitale and Toon Verwaest

Michael Lippautz added 2 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Michael Lippautz . resolved

It's best to pick a more local owner.

Commit Message

Line 18, Patchset 4 (Latest):

Michael Lippautz . unresolved

As always: Tracking bug.

Open in Gerrit

Related details

Attention is currently required from:

Marco Vitale
Toon Verwaest

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.

Gerrit

unsatisfied_requirement

open

diffy

Jakob Linke (Gerrit)

unread,

12:17 AM (19 hours ago) 12:17 AM

to Marco Vitale, Toon Verwaest, V8 LUCI CQ, chrom...@appspot.gserviceaccount.com, marja...@chromium.org, v8-re...@googlegroups.com

Attention needed from Marco Vitale and Toon Verwaest

Jakob Linke voted and added 3 comments

Votes added by Jakob Linke

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Jakob Linke . resolved

Nice find, lgtm with comments (not an owner).

File src/parsing/parser-base.h

Line 2065, Patchset 4 (Latest): if (this->flags().is_reparse()) {

Jakob Linke . unresolved

Please add a brief comment. (And consider renaming the flags parameter to regexp_error).

Line 2065, Patchset 4 (Parent): // TODO(jgruber): If already validated in the preparser, skip validation in

Jakob Linke . unresolved

Is this comment addressed? Note it's about the preparser, not reparsing.

Open in Gerrit

Related details

Attention is currently required from:

Marco Vitale
Toon Verwaest

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.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Toon Verwaest (Gerrit)

unread,

9:44 AM (10 hours ago) 9:44 AM

to Marco Vitale, Jakob Linke, V8 LUCI CQ, chrom...@appspot.gserviceaccount.com, marja...@chromium.org, v8-re...@googlegroups.com

Attention needed from Marco Vitale

Toon Verwaest voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Marco Vitale

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Toon Verwaest (Gerrit)

unread,

9:53 AM (10 hours ago) 9:53 AM

to Marco Vitale, Jakob Linke, V8 LUCI CQ, chrom...@appspot.gserviceaccount.com, marja...@chromium.org, v8-re...@googlegroups.com

Attention needed from Marco Vitale

Toon Verwaest added 1 comment

File src/parsing/parser-base.h

Line 2065, Patchset 4 (Parent): // TODO(jgruber): If already validated in the preparser, skip validation in

Jakob Linke . unresolved

Is this comment addressed? Note it's about the preparser, not reparsing.

Toon Verwaest

Actually you're right; this should probably be is_lazy_compile instead.

I presume this isn't the only place where regexp is checked for syntax correctness wrt v8 sandboxing?

Open in Gerrit

Related details

Attention is currently required from:

Marco Vitale

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Toon Verwaest (Gerrit)

unread,

9:55 AM (10 hours ago) 9:55 AM

to Marco Vitale, Jakob Linke, V8 LUCI CQ, chrom...@appspot.gserviceaccount.com, marja...@chromium.org, v8-re...@googlegroups.com

Attention needed from Marco Vitale

Toon Verwaest voted Code-Review+0

Code-Review

+0

Open in Gerrit

Related details

Attention is currently required from:

Marco Vitale

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.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Toon Verwaest (Gerrit)

unread,

10:06 AM (9 hours ago) 10:06 AM

to Marco Vitale, Jakob Linke, V8 LUCI CQ, chrom...@appspot.gserviceaccount.com, marja...@chromium.org, v8-re...@googlegroups.com

Attention needed from Marco Vitale

Toon Verwaest added 1 comment

File src/parsing/parser-base.h

Line 2065, Patchset 4 (Parent): // TODO(jgruber): If already validated in the preparser, skip validation in

Jakob Linke . unresolved

Is this comment addressed? Note it's about the preparser, not reparsing.

Toon Verwaest

Actually you're right; this should probably be is_lazy_compile instead.
I presume this isn't the only place where regexp is checked for syntax correctness wrt v8 sandboxing?

Toon Verwaest

I suppose in your case you _do_ want is_reparse since that's what we use for lazy source positions. But we should probably extend that flag to include what's currently covered by is_lazy_compile so we inlude all forms of reparses.

Open in Gerrit

Related details

Attention is currently required from:

Marco Vitale

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.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Marco Vitale (Gerrit)

unread,

10:15 AM (9 hours ago) 10:15 AM

to Toon Verwaest, Jakob Linke, V8 LUCI CQ, chrom...@appspot.gserviceaccount.com, marja...@chromium.org, v8-re...@googlegroups.com

Attention needed from Toon Verwaest

Marco Vitale added 1 comment

File src/parsing/parser-base.h

Line 2065, Patchset 4 (Parent): // TODO(jgruber): If already validated in the preparser, skip validation in

Jakob Linke . unresolved

Is this comment addressed? Note it's about the preparser, not reparsing.

Toon Verwaest

Actually you're right; this should probably be is_lazy_compile instead.
I presume this isn't the only place where regexp is checked for syntax correctness wrt v8 sandboxing?

Marco Vitale

I think that we can bail out in both cases. I’ve verified that `is_lazy_compile` implies a pre-parsed RegExp (https://chromium.googlesource.com/c/v8/v8/+/7665887), while `is_reparse` correctly identifies the re-parse phase triggered source position collection.

Open in Gerrit

Related details

Attention is currently required from:

Toon Verwaest

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.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Toon Verwaest (Gerrit)

unread,

12:01 PM (8 hours ago) 12:01 PM

to Marco Vitale, Jakob Linke, V8 LUCI CQ, chrom...@appspot.gserviceaccount.com, marja...@chromium.org, v8-re...@googlegroups.com

Attention needed from Marco Vitale

Toon Verwaest added 1 comment

File src/parsing/parser-base.h

Line 2065, Patchset 4 (Parent): // TODO(jgruber): If already validated in the preparser, skip validation in

Jakob Linke . unresolved

Is this comment addressed? Note it's about the preparser, not reparsing.

Toon Verwaest

Actually you're right; this should probably be is_lazy_compile instead.
I presume this isn't the only place where regexp is checked for syntax correctness wrt v8 sandboxing?

Marco Vitale

I think that we can bail out in both cases. I’ve verified that `is_lazy_compile` implies a pre-parsed RegExp (https://chromium.googlesource.com/c/v8/v8/+/7665887), while `is_reparse` correctly identifies the re-parse phase triggered source position collection.

Toon Verwaest

I think so, what I wanted to say was that a is_lazy_compile (should) imply a reparse? So we can still use is_reparse if we also correctly set the flag where we set is_lazy_compile

Open in Gerrit

Related details

Attention is currently required from:

Marco Vitale

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.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[parser] Skip redundant RegExp validation during re-parse [v8/v8 : main]

Marco Vitale (Gerrit)

New activity on the change

Related details

Michael Lippautz (Gerrit)

Michael Lippautz added 2 comments

Related details

Jakob Linke (Gerrit)

Jakob Linke voted and added 3 comments

Votes added by Jakob Linke

3 comments

Related details

Toon Verwaest (Gerrit)

Toon Verwaest voted Code-Review+1

Related details

Toon Verwaest (Gerrit)

Toon Verwaest added 1 comment

Related details

Toon Verwaest (Gerrit)

Toon Verwaest voted Code-Review+0

Related details

Toon Verwaest (Gerrit)

Toon Verwaest added 1 comment

Related details

Marco Vitale (Gerrit)

Marco Vitale added 1 comment

Related details

Toon Verwaest (Gerrit)

Toon Verwaest added 1 comment

Related details