Skip to content

regex: omit unused test and empty-replacement results - #2270

Open
leventov wants to merge 2 commits into
meta:static_hfrom
convex-in-prod:upstream-regexp-unused-results
Open

leventov wants to merge 2 commits into
meta:static_hfrom
convex-in-prod:upstream-regexp-unused-results

Conversation

@leventov

Copy link
Copy Markdown

Summary

Builtin Boolean regex tests currently construct a full JavaScript match result and immediately discard it. Global replacement with an empty string also constructs match arrays and intermediate match strings that are not observable when dispatch stays on the builtin path.

Separate matching and observable state updates from JavaScript result construction. Boolean consumers retain native capture ranges, legacy match state and lastIndex updates without building the match array, capture substrings, groups object or indices object. Eligible empty global replacements append unmatched spans directly.

Custom dispatch still follows RegExpExec. The no-lookup shortcut requires an ordinary data property resolving to the actual builtin exec; accessors and proxy/host prototype chains retain normal property access. Replacement takes the shortcut only after input/replacement coercion, global/unicode property reads and the initial lastIndex write, with both observable and internal global flags enabled.

This builds on #2269; the first commit is that correctness fix and the second is the allocation change. Generated matchers, shared literal bytecode, compiler fusion and changes to the general regex matching algorithm are outside this PR. Failed matches have fewer allocations to remove, so the benefit depends on the workload.

Test Plan

Built Hermes and Static Hermes from the extracted branch. Both the new regression and the dispatch regression pass in normal Hermes execution, -O, emitted-bytecode execution, and shermes -exec -O. The new differential assertions also pass in Node.

Regressions compare builtin operations with an overridden exec forwarding to the original method, including captures, named groups/indices, legacy state, global/sticky behavior, zero-width Unicode advancement, failed matches, getter/coercion order and nonwritable lastIndex errors.

RegExp.prototype.test must use RegExpExec after coercing its argument to
a string. Calling the builtin implementation directly bypasses overridden
exec methods and rejects otherwise valid generic object receivers.

Use the existing RegExpExec implementation, including its object-or-null
return check. Inline the now single-use exec helper into its entrypoint.
Cover dispatch, coercion order, getter effects, invalid returns, exceptions
and noncallable exec fallback on real RegExp objects.
Separate builtin matching and observable state updates from JavaScript
match-result construction. Boolean tests retain capture ranges and legacy
match state without building result arrays, substrings, groups or indices.

Guard the builtin path against overridden exec methods and effectful
property access. For global empty-string replacement, append unmatched
spans directly after the required coercions, flag reads and lastIndex
write. Retain zero-width Unicode advancement and ordinary error behavior.

Compare the optimized operations with a custom exec forwarding to the
builtin, covering captures, legacy state, flags, getters and write errors.
@meta-cla meta-cla Bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Oct 23, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant