mruby-regexp: ask the byte cases of a byte-indexed subject alone - #7120
Conversation
Each of these cases asks what the engine does with a byte that stands for no character, and asks it twice: once through a subject read as UTF-8 that carries the byte, and once through a byte-indexed subject, which the two commits before this one added. Only one of the two subjects can answer. Read as UTF-8 the subject spells no character where that byte is, so what a quantifier binds to there, whether a class holds the byte, and where a match may start are answers about a decoder walking input it was never given a rule for. Byte-indexed the same byte is a byte from end to end, every position reported is a byte offset the subject's own indexing agrees with, and the engine reaches it through the branch it keeps for exactly that: `mrb_re_charlen()` and `mrb_re_decode_char()` advance a byte at a time, and the seeding guard skips `mrb_re_utf8_interior_p()`, so no position is the interior of anything. So drop the UTF-8 subject from those cases and keep the byte-indexed one, with the comment that explained the case moving to the assertion that still makes it. Nothing that a whole UTF-8 subject can ask is touched: a match position inside a character, a class that holds a character rather than a byte, and the pattern side of an overlong sequence all keep the subjects they had.
|
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 UTF-8 regexp tests now use byte-indexed subjects for malformed and truncated sequences. They add coverage for byte match positions, quantifier boundaries, invalid UTF-8 decoding, byte-pattern classes, and extraction offsets. ChangesUTF-8 regexp test coverage
Estimated code review effort: 3 (Moderate) | ~20 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 |
|
Closing in favour of #7121. This diff showed 21 added assertion lines, which are the same assertions moved rather than new ones, and a diff that deletes tests should not need the reader to work that out. #7121 does the same deletion with no reordering, so its only added lines are comments ( |
Third of the pieces #7110 was split into, on top of #7115 and #7116, and test
only. Deleting assertions deserves its own pull request rather than a paragraph
inside one that changes behaviour, so here it is on its own.
Each of these cases asks what the engine does with a byte that stands for no
character, and since #7115 and #7116 it asks twice: once through a subject read
as UTF-8 that carries the byte, and once through a byte-indexed subject.
Only one of the two subjects can answer
Read as UTF-8, the subject spells no character where that byte is. What a
quantifier binds to there, whether a class holds the byte, and where a match may
start are then answers about a decoder walking input no rule covers.
Byte-indexed, the same byte is a byte from end to end, every position reported is
a byte offset the subject's own indexing agrees with, and the engine reaches it
through the branch it keeps for exactly that:
mrb_re_charlen()andmrb_re_decode_char()advance a byte at a time, and the seeding guard skipsmrb_re_utf8_interior_p(), so no position is the interior of anything.What this does
Drops the UTF-8 subject from those cases and keeps the byte-indexed one. The
comment that explained each case moves to the assertion that still makes it,
which is most of what the diff shows as added.
Nothing a whole UTF-8 subject can ask is touched:
"あ".match(Regexp.new("\x81"))and the"ĵ"cases) keeps its subjectmu =~ /[\xB5]/and the\u{B5}spellings) keeps its subject(
Regexp.new("[\xC0\xBC]").match?("<")) keeps its subjectRegexp - match positions on malformed UTF-8 agree with string indexingisleft exactly as it is: what it pins is what mruby knows about the string
rather than what the engine does with the bytes, so it belongs with the piece
that changes its answer
The fuzz-derived buffer cases (
Regexp - invalid UTF-8 byte near pattern endand
Regexp - truncated UTF-8 at subject end) keep a walk to the end of eachbuffer, in the byte-indexed form #7116 added.
Verified
rake teston a full-core build (MRB_UTF8_STRINGthrough mruby-encoding):2253 tests, all green
rake teston the default gembox (noMRB_UTF8_STRING): 2060 tests, allgreen
prek run --all-filespasses, except thatmarkdownlintcould not installlocally (npm engine mismatch); no Markdown is touched here
Summary by CodeRabbit