Repository navigation
fix: rich-API (findAll/replaceAll/split) on annotation-processor matchers - #136
Conversation
The APT dispatcher emitted findMatchFrom/findBoundsFrom for the DFA-unrolled and generic-NFA strategies whose bodies call the findLongestMatchEnd helper without emitting it - every rich-API use threw NoSuchMethodError (RuntimeCompiler already emits the helper for the same strategies). Also: shared NFA findBoundsFrom no longer uses findLongestMatchEnd for lookaround patterns - the greedy helper never evaluates assertions and reported no-match, silently making replaceAll/split no-ops on both paths; bounds now derive from the assertion-correct findMatchFrom instead.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #136 +/- ##
=========================================
- Coverage 85.4% 84.9% -0.6%
Complexity 1 1
=========================================
Files 163 163
Lines 48759 48850 +91
Branches 6970 6972 +2
=========================================
- Hits 41645 41476 -169
- Misses 5105 5368 +263
+ Partials 2009 2006 -3
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee3ea3979f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
The DFA-unrolled findBoundsFrom inline greedy scan aborts the whole scan on
the first failed lookahead instead of skipping the accepting-position record
and continuing (a+(?=b) on "aab": replaceAll("_") returned the input
unchanged while findAll was correct), and it ignores acceptance anchor
conditions. Route assertion-bearing DFAs to findLongestMatchEnd, whose greedy
state code gates the record on lookaheads (skipRecord) and anchors - the same
end search findMatchFrom uses - so replaceAll/split bounds match findAll.
Also document that the NFA bounds delegation inherits findMatchFrom's
longest-end search semantics: leftmost-first alternation patterns diverge on
the rich API exactly like findAll does (pre-existing, needs priority-aware
matching or an analyzer-level refusal).
2 participants
What does this PR do?
Two fixes to the rich match-result API (
findAll/replaceAll/split) on generated matchers:APT dispatcher emits the
findLongestMatchEndhelper where it is called. The annotation-processor dispatcher emittedfindMatchFrom/findBoundsFromfor the DFA-unrolled and generic-NFA strategies whose bodies call thefindLongestMatchEndhelper — without also emitting that helper. Every rich-API use on such matchers threwNoSuchMethodErrorat runtime. The dispatcher now mirrorsRuntimeCompiler, which already emits the helper for the same strategies (dual-path rule).Shared NFA
findBoundsFromno longer usesfindLongestMatchEndfor lookaround patterns. The greedy helper never evaluates lookaround assertions, so for assertion-bearing patterns it never reached an accepting state and returned -1 —replaceAll/splitsilently returned the input unchanged (a wrong-result bug, not a crash, and it affected the runtime path too, not just APT). For assertion-bearing NFAs the bounds now derive fromfindMatchFrom, which evaluates assertions correctly. Assertion-free patterns keep the zero-allocation O(n) greedy path unchanged.Motivation
Found by the differential JDK-parity gate of the profiling-backend regex adoption wave 2 (capture/replace cohort): 8 of 21 pilot sites crashed or silently no-op'd on
replaceAll/splitwhen compiled through the annotation processor.Related Issue(s)
None filed yet (regression present in 0.5.0; discovered during backend adoption).
Change Type
Checklist
./gradlew build)Performance Impact
Assertion-free patterns: unchanged (identical bytecode paths; the new emitter only engages when the NFA contains lookaround assertions). Lookaround-bearing patterns on the rich API: the bounds path now pays a per-occurrence
findMatchFromscan (O(n)matchBoundedattempts, oneMatchResulteach) instead of the broken greedy helper — correctness over speed for a rare pattern class; noted in code comments.Note: the helper emission was deliberately removed from the hybrid and
OPTIMIZED_NFA_WITH_LOOKAHEADarms (assertion-bearing by definition — the helper would be unreferenced dead bytecode); it is kept in theOPTIMIZED_NFAarm as defensive runtime parity (unreachable through APT refusal guards today, documented inline).Additional Notes
Differential test (
RichApiDifferentialTest): 8 patterns x 11 inputs, JDK parity forreplaceAll/split/findAllacross the reachable APT strategies.^(.+?)\.([^/]+)is intentionally excluded — it routes to the PIKEVM_CAPTURE fallback whose lazy-group divergence is fixed separately onfix/pikevm-lazy-group-priority; it is added back there.An external model review (gpt-5.6-terra) of this diff returned approve-with-comments; all medium findings were addressed (test oracle, dead emissions, cost documentation, allocator discipline).