mruby-regexp: ask the engine's byte questions of a byte-indexed subject too - #7115
Merged
Merged
Conversation
…ct too The tests that ask what the engine does with a byte standing for no character ask it through a UTF-8 subject carrying that byte. What each of them pins belongs to the pattern: which byte a quantifier binds to, whether `/i` folds a byte above 127, whether a class holds a byte or the character whose spelling ends in it, and where a match may start. A byte-indexed subject puts the same question to the engine, and none of them asked it there. That is a branch of its own rather than another wording of the same one. `mrb_re_exec()` takes a flag for a byte-indexed subject and the steps it drives turn on that flag: `mrb_re_charlen()` and `mrb_re_decode_char()` advance and decode a byte at a time, `memcmp_ci()` folds a byte at a time, `lookbehind_start()` rewinds by bytes, and the seeding guard skips `mrb_re_utf8_interior_p()` outright, so every byte is a match position. What the answers above are on that side went unasserted. So ask each of them there as well, next to the case it answers, leaving the UTF-8 subject each already asks. A byte-indexed subject reports every position in bytes, which its own indexing agrees with, so `#begin` states directly what `#pre_match` had to state indirectly.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds UTF-8 regexp tests for byte-indexed subjects. The tests cover malformed bytes, ignore-case matching, quantifiers, match positions, ChangesByte-oriented regexp behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #7110, which asked for one review of a behaviour change and a
large test rewrite at once. This is the first of four pieces and changes no
behaviour.
The tests that ask what the engine does with a byte standing for no character
ask it through a UTF-8 subject carrying that byte. What each of them pins
belongs to the pattern: which byte a quantifier binds to, whether
/ifolds abyte above 127, whether a class holds a byte or the character whose spelling
ends in it, and where a match may start. A byte-indexed subject puts the same
question to the engine, and none of them asked it there.
Why that is a different question to ask
mrb_re_exec()takes a flag for a byte-indexed subject, and the steps it drivesturn on that flag:
mrb_re_charlen()andmrb_re_decode_char()advance and decode a byte at atime
memcmp_ci()folds a byte at a timelookbehind_start()rewinds by bytesmrb_re_utf8_interior_p()outright, so every byte isa match position
So the answers these tests pin went unasserted on that side.
What this does
Asks each of them there as well, next to the case it answers, leaving the UTF-8
subject each already asks. A byte-indexed subject reports every position in
bytes, which its own indexing agrees with, so
#beginstates directly what#pre_matchhad to state indirectly.One assertion is new rather than a counterpart: a class that holds the character
µstill does not hold a lone0xB5on a byte-indexed subject either, which isthe contrast to the class that holds the byte on the line above it.
Verified
rake teston a full-core build (MRB_UTF8_STRINGthrough mruby-encoding):2249 tests, all green
rake teston the default gembox (noMRB_UTF8_STRING): 2058 tests, allgreen
prek run --all-filespasses, except thatmarkdownlintcould not installlocally (npm engine mismatch); no Markdown is touched here
Summary by CodeRabbit