Repository navigation
Conversation
A lookbehind's nodes are reversed after nested AlternationNode and LoopNode instances have cached MatchConstraintAnchoredAtStart. The executor then rejected those alternatives and loopees, because a backwards match starts after the anchored text, not at the start of the input. For example, /a(?<=x|^a)/ failed to match "a". Clear the stale constraint when these nodes reverse their children, as LookaroundNode already does for the top level of a lookbehind. Fixes meta#2233 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Hi @idhanth! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
Summary
Fixes #2233.
/a(?<=x|^a)/.test("a")returnsfalseinstead oftrue.Regex::pushLookaroundreverses a lookbehind's node list after its nested nodes were constructed.AlternationNodeandLoopNodecache their children's match constraints at construction, so a^inside them leavesMatchConstraintAnchoredAtStartset. That flag reaches the executor as the alternation'sprimaryConstraints/secondaryConstraintsand the loop'sloopeeConstraints.satisfiesConstraintsthen rejects the^aalternative, because the backwards match starts at position 1 (after thea), not at the start of the input.LookaroundNodealready clearsMatchConstraintAnchoredAtStartfor the top level of a lookbehind. This change clears it inAlternationNode::reverseChildrenandLoopNode::reverseChildrenas well. The constraint is only an early-reject optimization: the^itself is still checked when itsLeftAnchorinstruction runs, so clearing it can't make a regex match where it shouldn't.The same bug affected loops:
/a(?<=(?:^a)+)/didn't match"a"either.Not changed:
MarkedSubexpressionNodecaches constraints too, but only its compile-timematchConstraints()reads them, before reversal, so they never reach the executor.LookaroundNodedoesn't overridereverseChildren), so its constraints stay correct.Test Plan
Added 8 cases to
test/hermes/regexp.js. They cover the issue's pattern, a^in a later alternative, a capture group, unbounded and bounded loops, a loop around an alternation, and two cases where^must still reject. Every expected result matches V8 (Node 22).Built on Ubuntu 24.04 (clang 18, Release, linux/arm64 in Docker) from
static_h:nullwhere V8 returns a match. With it, all 8 match V8.test/hermes/regexp.jsandtest/hermes/string-functions.jspass when run through theirRUNlines with the builthermesandFileCheck.LIT_FILTER="(?i)regex|regexp" ninja check-hermes: 36 of 4321 tests selected, 35 passed, 1 unsupported, 0 failed.clang-format --dry-run --Werror include/hermes/Regex/RegexNode.h(clang-format 18, repo.clang-format): clean.The full
check-hermessuite was not run.🤖 Generated with Claude Code