Skip to content

mruby-regexp: close the pattern dispatch in the String overrides - #7079

Merged
matz merged 5 commits into
mruby:masterfrom
takumin:regexp-close-pattern-dispatch
Aug 10, 2026
Merged

mruby-regexp: close the pattern dispatch in the String overrides#7079
matz merged 5 commits into
mruby:masterfrom
takumin:regexp-close-pattern-dispatch

Conversation

@takumin

@takumin takumin commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Follows #7067, which documented that the String overrides in
mrbgems/mruby-regexp/mrblib/string_regexp.rb reach their pattern through
ordinary Ruby-visible methods, and accepted that surface in the file comment.
This PR proposes the other side of that question: close the surface, because
working out the fix showed it is smaller than the comment assumed, and the
boundary it draws is the one CRuby already has.

What was open

Every override reached the engine by calling methods on the pattern
(match, match?, =~, __byte_match, __sub_str, __gsub_str,
__scan), so a method rewritten on a Regexp instance replaced the search
itself, and every String method in this file followed the replacement:

r = /l+/
def r.match(*args); "PWNED"; end

"hello"[r]  # CRuby: "ll", mruby: "P"

The quietest paths handed back the redefinition's own value with nothing
raising, so the caller had no way to tell:

r = /l+/
def r.__sub_str(*args); "PWNED"; end

"hello".sub(r, "X")       # CRuby: "heXo", mruby: "PWNED"
"hello".dup.sub!(r, "X")  # CRuby: "heXo", mruby: "PWNED", written into the receiver

String#match? and String#=~ diverged even one level earlier: their CRuby
counterparts rb_str_match_m_p() and rb_str_match() search a real Regexp
without asking it anything, so for these two the dispatch itself was the
divergence, before any redefinition enters.

The shape of the fix

The overrides now reach the engine through class methods that take the
pattern as an argument: Regexp.__search, Regexp.__byte_search and
Regexp.__search_p are thin wrappers over the existing exec_match(), and
__sub_str, __gsub_str and __scan become class forms of the operation
cores they already were. A class method leaves no instance for a singleton
method to ride on, so the search stops being redefinable while the loops,
blocks and field lists stay in mrblib, which keeps the C side free of
callbacks into the VM.

This is architecture parity with CRuby rather than case-by-case behaviour
parity: CRuby's rb_str_sub_bang(), rb_str_subpat(), rb_str_index_m()
and the rest reach the engine through C internals that never consult the
pattern's method table, and now this gem does the same. Two consequences
fall out rather than being patched individually. A rewritten pattern method
no longer steers any String method, and the MatchData the overrides read
always comes from create_matchdata(), so no argument can have planted a
singleton method on it either; what remains reachable is a class-wide
redefinition of a MatchData method, the same category as redefining
String#sub itself.

Two dispatches stay, both deliberate and both CRuby's own shape:
String#match sends match to the pattern because rb_str_match_m() does
so on purpose, and String#=~ still forwards an argument that is not a
Regexp to the argument's own =~, as rb_str_match() does. The file
comment from #7067 is rewritten in the last commit to describe exactly these
two exceptions instead of the open surface.

Cost

Four internal instance methods are removed (__byte_match, __sub_str,
__gsub_str, __scan) and six class methods added, so the method table
grows by two entries net. The operation core bodies are unchanged apart from
taking the pattern as an argument; the new code is the three thin search
wrappers and a shared type-check backstop.

Commits

Each commit closes one dispatch and carries a test that rewrites the method
it closes on a pattern instance and asserts the CRuby answer, receivers of
the bang forms included. Every CRuby answer above was checked against
CRuby 4.0.6.

Summary by CodeRabbit

  • Bug Fixes
    • Improved the consistency and reliability of regular-expression operations on strings.
    • String searches, substitutions, scans, slicing, partitioning, and prefix checks now produce expected results even when regular-expression behavior is customized.
    • Preserved match-state handling across supported matching operations.
    • Fixed bang substitution methods to correctly modify the original string and return it when a match is found.

The String overrides in string_regexp.rb reached their search through
`pattern.match(self)`, an ordinary method call on the pattern, so a
method rewritten on a Regexp instance replaced the search itself:

```ruby
r = /l+/
def r.match(*args); "PWNED"; end

"hello"[r]  # CRuby: "ll", mruby: "P"
```

CRuby dispatches `match` to the pattern only in `rb_str_match_m()`, on
purpose; everywhere else (`rb_str_sub_bang()`, `rb_str_subpat()`,
`rb_str_index_m()` and the rest) it reaches the engine through C
internals that never consult the pattern's method table. This commit
gives the gem the same boundary: `Regexp.__search` is `Regexp#match`
with the pattern as an argument and no block form, a thin wrapper over
the same `exec_match()`. A class method leaves no instance for a
singleton method to ride on, so the mrblib callers keep their loops and
blocks in Ruby while the search itself stops being redefinable.

The call sites moved over are the block form of `sub`, the guards of
`sub!` and `gsub!`, `[]`, `slice`, `[]=`, `slice!`, `index`, `rindex`,
the miss paths of `byteindex` and `byterindex`, `partition`,
`rpartition` (through `__regexp_rsearch`) and `start_with?`.
`String#match` keeps dispatching, as CRuby does. The `match?`, `=~`,
`__byte_match`, `__sub_str`, `__gsub_str` and `__scan` call sites are
the same shape and move in the following commits.

The MatchData these call sites read (`pre_match`, `begin`, `[]` and the
rest) now always comes from `create_matchdata()`, so no argument can
have planted a singleton method on it either. What remains reachable is
a class-wide redefinition of a MatchData method, which is the same
category as redefining `String#sub` itself and is not steered by the
argument.
The byte-space loops in `gsub` (block form), `split` and `byteindex`
drove their search through `pattern.__byte_match(self, pos)`, an
ordinary method call on the pattern, so a method rewritten on a Regexp
instance replaced the search under the loop:

```ruby
r = /l+/
def r.__byte_match(*args); "PWNED"; end

"hello".gsub(r) { |m| m.upcase }
# CRuby: "heLLo", mruby: NoMethodError for `__byte_begin` on String
```

`Regexp.__byte_search` is the same thin wrapper over `exec_match()`
with the pattern as an argument, so the search stops being redefinable
while both loops stay in Ruby, where the block call and the field list
belong. As with `Regexp.__search`, there is no position normalization,
because the callers already work in byte space, and no operand
conversion, because they always pass a String.

The instance form had no caller left outside these three sites, so it
is removed rather than kept alongside: `__byte_match` was an internal
of this gem, and its replacement takes over its method table slot
rather than adding one.
`__sub_str`, `__gsub_str` and `__scan` are the whole-operation entry
points of `sub`, `gsub` and `scan` with a replacement string: each runs
an entire operation in C and hands back its result. They took the
pattern from `self`, so the mrblib callers reached them through an
ordinary method call on the pattern, and a method rewritten on a Regexp
instance replaced the whole operation. This was the quietest part of
the surface, because the caller got the redefinition's return value
verbatim and nothing raised on the way:

```ruby
r = /l+/
def r.__sub_str(*args); "PWNED"; end

"hello".sub(r, "X")       # CRuby: "heXo", mruby: "PWNED"
"hello".dup.sub!(r, "X")  # worse: the receiver became "PWNED" too
```

CRuby closes all of these: its `sub`, `gsub` and `scan` reach the
pattern through `rb_pat_search()`, which searches a real Regexp without
asking it anything.

Each core becomes a class method taking the pattern as its first
argument, the boundary `Regexp.__search` already draws for the plain
searches. The instance forms had no caller left outside the gem, so the
class forms take over their method table slots rather than adding new
ones. The bodies themselves are unchanged: they already ran without
calling back into Ruby, which is why they are in C at all.

`sub!` and `gsub!` reach the same two cores by calling `sub` and `gsub`
with a pattern they already resolved, so they are closed by the same
change, receiver included.
These two are the overrides whose CRuby counterparts do not dispatch to
the pattern at all: `rb_str_match_m_p()` resolves the argument and
searches it, and `rb_str_match()` sends `=~` to the argument only when
it is neither a String nor a Regexp. The overrides here asked the
pattern on every path, so for a real Regexp they answered whatever a
rewritten method answered, and nothing raised on the way:

```ruby
r = /l+/
def r.match?(*args); "PWNED"; end
def r.=~(*args); 99; end

"hello".match?(r)  # CRuby: true, mruby: "PWNED"
"hello" =~ r       # CRuby: 2,    mruby: 99
```

`String#match?` now hands the pattern to `Regexp.__search_p`, which is
`Regexp#match?` with the pattern as an argument: the same search with a
NULL capture buffer, so it allocates no MatchData and leaves the match
globals alone. `String#=~` searches a real Regexp through the existing
`Regexp.__search` and reads `begin(0)` from the MatchData, so no second
entry point is needed; the globals it publishes and the character
offset it answers are the ones `Regexp#=~` produced before.

The forward for everything that is not a Regexp stays, because CRuby
forwards there too: `"hello" =~ obj` still reaches the argument's own
`=~`. Only the branch in front of it changed, from every argument to
the non-Regexp ones.
The comment at the top of string_regexp.rb described a file where every
override reached its pattern through redefinable methods, and argued
that the gem accepts that surface rather than closing it. The previous
commits closed it, so the description no longer holds; what is left to
say is smaller.

The type-check paragraph survives unchanged, because nothing about the
argument checks moved. The surface paragraphs give way to what the file
does now: the searches go through class methods that take the pattern
as an argument, the MatchData comes from C, and the reachable remainder
is a class-wide redefinition of a MatchData method, which is the same
category as redefining `String#sub` itself.

The CRuby comparison stays, inverted: `String#match` dispatching
`match` on purpose (`rb_str_match_m()`) and `=~` forwarding a
non-Regexp argument (`rb_str_match()`) are now the two deliberate
exceptions rather than the two ends of an open range.
@takumin
takumin requested a review from matz as a code owner August 10, 2026 15:24
@coderabbitai

coderabbitai Bot commented Aug 10, 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: e89ff063-21be-4671-89a9-471f18432e8d

📥 Commits

Reviewing files that changed from the base of the PR and between 7184392 and d1dbcfd.

📒 Files selected for processing (3)
  • mrbgems/mruby-regexp/mrblib/string_regexp.rb
  • mrbgems/mruby-regexp/src/regexp.c
  • mrbgems/mruby-regexp/test/regexp.rb

📝 Walkthrough

Walkthrough

Regexp-aware String methods now call validated class-level Regexp C helpers for search, substitution, and scanning. Tests cover overridden Regexp methods, match state, bang operations, indexing, partitioning, and prefix checks.

Changes

Regexp dispatch refactor

Layer / File(s) Summary
Class-level search primitives
mrbgems/mruby-regexp/src/regexp.c, mrbgems/mruby-regexp/mrblib/string_regexp.rb, mrbgems/mruby-regexp/test/regexp.rb
Added Regexp.__search, Regexp.__byte_search, and Regexp.__search_p. Regexp-aware String search operations now use these helpers while preserving non-Regexp =~ dispatch and match-global behavior.
Substitution and scan cores
mrbgems/mruby-regexp/src/regexp.c, mrbgems/mruby-regexp/mrblib/string_regexp.rb, mrbgems/mruby-regexp/test/regexp.rb
Converted __gsub_str, __sub_str, and __scan to class methods with explicit Regexp arguments. Updated substitution and scan callers and added regression coverage for overrides and bang-operation results.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • mruby/mruby#7001: Modifies the same sub, gsub, scan, and =~ dispatch paths.
  • mruby/mruby#7061: Modifies the same sub! and gsub! substitution paths and tests.
  • mruby/mruby#7075: Introduces related regexp-aware search, partition, and prefix paths.

Suggested labels: mrbgems

Suggested reviewers: matz, nattzn

🚥 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 clearly summarizes the main change: preventing Regexp instance method dispatch in String regexp overrides.
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.

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