Skip to content

mruby-regexp: ask the byte cases of a byte-indexed subject alone - #7120

Merged
matz merged 1 commit into
mruby:masterfrom
takumin:regexp-drop-refused-subject-tests
Aug 12, 2026
Merged

mruby-regexp: ask the byte cases of a byte-indexed subject alone#7120
matz merged 1 commit into
mruby:masterfrom
takumin:regexp-drop-refused-subject-tests

Conversation

@takumin

@takumin takumin commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

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() 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.

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:

  • a match position inside a character ("あ".match(Regexp.new("\x81")) and the
    "ĵ" cases) keeps its subject
  • a class that holds a character rather than a byte (mu =~ /[\xB5]/ and the
    \u{B5} spellings) keeps its subject
  • the pattern side of an overlong sequence
    (Regexp.new("[\xC0\xBC]").match?("<")) keeps its subject
  • Regexp - match positions on malformed UTF-8 agree with string indexing is
    left 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 end
and Regexp - truncated UTF-8 at subject end) keep a walk to the end of each
buffer, in the byte-indexed form #7116 added.

Verified

  • rake test on a full-core build (MRB_UTF8_STRING through mruby-encoding):
    2253 tests, all green
  • rake test on the default gembox (no MRB_UTF8_STRING): 2060 tests, all
    green
  • prek run --all-files passes, except that markdownlint could not install
    locally (npm engine mismatch); no Markdown is touched here

Summary by CodeRabbit

  • Tests
    • Expanded UTF-8 regular expression coverage for malformed, truncated, overlong, and invalid sequences.
    • Added validation for byte-based match positions, extraction offsets, character classes, and quantifier boundaries.
    • Improved coverage of decoding boundaries in both patterns and input text.

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.
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b65b22b-816d-41f7-8456-a169a457ecdc

📥 Commits

Reviewing files that changed from the base of the PR and between 1c034f7 and 148db20.

📒 Files selected for processing (1)
  • mrbgems/mruby-regexp/test/regexp_utf8.rb

📝 Walkthrough

Walkthrough

The 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.

Changes

UTF-8 regexp test coverage

Layer / File(s) Summary
Byte-indexed matching and offsets
mrbgems/mruby-regexp/test/regexp_utf8.rb
Malformed patterns, standalone bytes, quantifier boundaries, and match positions now use byte-indexed subjects.
UTF-8 decoding boundaries
mrbgems/mruby-regexp/test/regexp_utf8.rb
Tests cover binary subjects, truncated endings, malformed class bytes, overlong sequences, and out-of-range code points.
Byte-pattern classes and ranges
mrbgems/mruby-regexp/test/regexp_utf8.rb
Class tests distinguish complete UTF-8 characters from standalone bytes and preserve mixed byte/codepoint range assertions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • mruby/mruby#7115: Updates the same UTF-8 regexp test file and overlaps in byte-indexed subject coverage.
  • mruby/mruby#7119: Covers byte-indexed invalid UTF-8 subjects and regexp matching behavior.
  • mruby/mruby#7068: Covers UTF-8 validation behavior for overlong and malformed sequences.

Suggested reviewers: matz

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title identifies the main change: testing byte cases only for byte-indexed subjects.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@takumin

takumin commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

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 (+39/-79, every added line beginning with #), and it says in each block, and in a table in the description, which assertions go and why.

@takumin takumin closed this Aug 12, 2026
@matz
matz merged commit 8d5ae77 into mruby:master Aug 12, 2026
20 of 21 checks passed
@takumin
takumin deleted the regexp-drop-refused-subject-tests branch August 12, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants