Skip to content

mruby-regexp: add the regexp form of String#[]= and String#slice! - #7063

Merged
matz merged 1 commit into
mruby:masterfrom
takumin:string-aset-regexp
Aug 10, 2026
Merged

mruby-regexp: add the regexp form of String#[]= and String#slice!#7063
matz merged 1 commit into
mruby:masterfrom
takumin:string-aset-regexp

Conversation

@takumin

@takumin takumin commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

String#[] and String#slice learned their regexp form, but the write
side was left out and still funnels a Regexp through an Integer
conversion:

$ ./build/host/bin/mruby -e 's = +"hello"; s[/l+/] = "X"'
-e:1:in []=: Regexp cannot be converted to Integer (TypeError)
$ ./build/host/bin/mruby -e 's = +"hello"; s.slice!(/l+/)'
-e:1:in slice!: Regexp cannot be converted to Integer (TypeError)

mrb_str_aset() shares str_convert_range() with mrb_str_aref(), and
mrb_str_slice_bang() reaches mrb_as_int() for anything that is
neither a String nor a Range. String#slice! is registered as
MRB_SYM_B(slice), so the literal name appears nowhere in the tree
outside its own tests.

What this adds

s = +"hello"
s[/l+/] = "X"            # => "X",  s is now "heXo"
s[/(?<x>l+)/, :x] = "Y"  # => "Y",  s is now "heYo"
s.slice!(/l+/)           # => "ll", s is now "heo"
s.slice!(/(l)(o)/, 1)    # => "l",  s is now "helo"

Both are overridden in mrblib/string_regexp.rb in the shape [] uses
there: the C-defined method is captured under a __ name, every argument
list that does not start with a Regexp is handed straight back to it, and
the argument's type is read with Regexp === rather than is_a?. No
new C is needed, because MatchData#begin and #end resolve a group
name and report character offsets, which is the space the two-integer
form of []= works in.

Behaviour

Checked against CRuby 4.0.6; every row below is covered by a test.

Expression Result
s[/l+/] = "X" s becomes "heXo", evaluates to "X"
s[/z/] = "X" IndexError: regexp not matched
s[/(?<x>l+)/, :x] = "Y" s becomes "heYo"
s[/(l+)(o)/, -1] = "X" s becomes "hellX"
s[/l+/, -1] = "X" IndexError: index -1 out of regexp
s[/l+/, 5] = "X" IndexError: index 5 out of regexp
s[/(h)|(z)/, 2] = "X" IndexError: regexp group 2 not matched
s[/(l+)/, "zz"] = "Y" IndexError: undefined group name reference: zz
s.slice!(/l+/) "ll", s becomes "heo"
s.slice!(/(l+)(o)/, -1) "o", s becomes "hell"
s.slice!(/l+/, -1) nil, s unchanged
s.slice!(/l+/, 5) nil, s unchanged
s.slice!(/(h)|(z)/, 2) "", s unchanged
s.slice!(/z/) nil, s unchanged, $~ cleared

The three places the two halves disagree are followed as CRuby has them.
An unusable capture argument raises for []= and answers nil or "" for
slice!; the "" is what rb_str_slice_bang() produces from a
non-participating group's -1 offset. A negative index is normalized
the same way on both sides, which puts group 0 out of its reach. And the
frozen check lands on opposite sides of the search, which $~ makes
observable:

$~ = nil
"hello".freeze[/l+/] = "X"   # FrozenError, and $~[0] is "ll"
$~ = nil
"hello".freeze.slice!(/l+/)  # FrozenError, and $~ is still nil

[]= gets its ordering for free by letting the mutation be what raises,
which also makes a pattern that does not match raise IndexError on a
frozen receiver rather than FrozenError. slice! asks frozen?
before it searches, matching both CRuby and mrb_str_slice_bang(), whose
own check is its first statement.

One divergence: for a name that resolves to a group which did not take
part in the match, CRuby reports the group's number and the override
repeats the name, because matchdata_name_to_group() is not reachable
from Ruby. The exception class and the position are the same.

Notes

Unlike the read side, this override is not free. vm_op_getidx()
answers str[Integer], str[String] and str[Range] from C, but
vm_op_setidx() optimizes Array and Hash only and sends []= for
everything else, so every str[...] = repl pays a Ruby frame on its way
to __aset once this gem is in the build. The delegation guard is a
single Regexp === before any other work.

String#sub! and String#gsub! are the other half of the destructive
work and are not touched here.

Testing

rake test passes, and so does a MRB_UTF8_STRING build, which the
multibyte cases are there for.

Summary by CodeRabbit

  • New Features

    • Added regular expression support for replacing full matches or capture groups in strings.
    • Added regular expression support for removing full matches or capture groups from strings.
    • Supports named and negative capture references, multibyte text, and existing non-regexp operations.
  • Bug Fixes

    • Added validation for invalid replacements, frozen strings, and unsupported argument types.
  • Documentation

    • Documented regexp-based string replacement and deletion syntax.

The read side landed as `String#[]` and `String#slice`, but the write
side still funnels a Regexp through an Integer conversion and fails with
a type error that names Integer:

```ruby
s = +"hello"
s[/l+/] = "X"            # CRuby: "heXo", mruby: TypeError
s[/(?<x>l+)/, :x] = "Y"  # CRuby: "heYo", mruby: TypeError
s.slice!(/l+/)           # CRuby: "ll",   mruby: TypeError
s.slice!(/(l)(o)/, 1)    # CRuby: "l",    mruby: TypeError
```

`mrb_str_aset()` shares `str_convert_range()` with `mrb_str_aref()`, so
it has the same three accepted index types and the same fallback to
`mrb_ensure_int_type()`.  `mrb_str_slice_bang()` does its own conversion
and reaches `mrb_as_int()` for anything that is neither a String nor a
Range.  It is easy to miss when grepping: it is registered as
`MRB_SYM_B(slice)`, so the literal `slice!` appears nowhere in the tree
outside its own tests.

Override both in `mrblib/string_regexp.rb`, in the shape `[]` uses there:
capture the C-defined method under a `__` name, hand every argument list
that does not start with a Regexp straight back to it, and read the
argument's type with `Regexp ===` rather than `is_a?`.  No new C is
needed.  `MatchData#begin` and `#end` resolve a group name and report
character offsets, which is the space the two-integer form of `[]=` works
in, so a named group's span is reachable from Ruby and a multibyte
subject needs no further conversion.

The two halves disagree in three places, and CRuby is followed in each:

- An unusable capture argument.  `[]=` raises `IndexError` both for an
  index that reaches no group and for a group that exists but did not
  take part in the match, where `slice!` answers nil for the first and
  `""` for the second.  The `""` falls out of `rb_str_slice_bang()`
  building its result from the group's `-1` offset rather than out of a
  decision, but it is what CRuby answers.  A negative index is
  normalized the same way on both sides, which puts group 0 out of its
  reach; `MatchData#begin` rejects every negative index with a wording
  of its own, so neither the normalization nor the range check can be
  left to it.
- A name that resolves to no group is the one case that raises for both,
  and `MatchData#begin` already raises it with CRuby's message.  A name
  that resolves to a group which did not take part in the match is
  reported by CRuby with the group's number, which is not reachable from
  Ruby, so the message repeats the name as it was given.
- The frozen check.  CRuby searches before it modifies for `[]=`, so a
  frozen receiver raises only once the match has been published, and a
  pattern that does not match raises `IndexError` rather than
  `FrozenError`; letting the mutation be what raises reproduces both.
  For `slice!` CRuby checks first, as `mrb_str_slice_bang()` already
  does, so the override asks `frozen?` before it searches and leaves
  `$~` alone.

A Regexp goes through `Regexp#match` rather than `#match?`, because the
match globals have to be published here, including the clearing a failed
match does before `[]=` raises and before `slice!` returns nil.  The
MatchData left behind describes the subject as it was before the
replacement, since `create_matchdata()` snapshots it.

The replacement reaches the core method's type check unconverted, the way
the `sub` override refuses to invent an implicit conversion.  Its message
reads `Symbol cannot be converted to String` where CRuby says `no
implicit conversion of Symbol into String`; that wording is the core's
throughout and is not this change's to fix.

Unlike the read side, this override is not free.  `vm_op_getidx()`
answers `str[Integer]`, `str[String]` and `str[Range]` from C, which is
what keeps `[]` off the common paths, but `vm_op_setidx()` optimizes
Array and Hash only and sends `[]=` for everything else, so every
`str[...] = repl` pays a Ruby frame on its way to `__aset` once this gem
is in the build.  The delegation guard is a single `Regexp ===` before
any other work.
@takumin
takumin requested a review from matz as a code owner August 9, 2026 23:02
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

String#[]= and String#slice! now accept regular expressions and capture selectors. They validate arguments, update match state, handle frozen strings, support multibyte text, and retain existing behavior for non-regexp arguments.

Changes

Regexp String mutation

Layer / File(s) Summary
Regexp assignment
mrbgems/mruby-regexp/mrblib/string_regexp.rb, mrbgems/mruby-regexp/test/regexp.rb
String#[]= replaces full regexp matches or captures. Tests cover named captures, offsets, match globals, validation, frozen strings, and fallback behavior.
Regexp destructive slicing
mrbgems/mruby-regexp/mrblib/string_regexp.rb, mrbgems/mruby-regexp/test/regexp.rb, mrbgems/mruby-regexp/README.md
String#slice! removes full matches or captures and returns removed text. Tests cover capture selection, mutation, errors, frozen strings, and documented usage.

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

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant String
  participant Regexp
  participant MatchGlobals
  Caller->>String: []= or slice! with regexp
  String->>Regexp: search subject and resolve capture
  Regexp-->>String: match range or failure
  String->>MatchGlobals: publish or preserve match state
  String-->>Caller: replace, remove, or return result
Loading

Possibly related PRs

  • mruby/mruby#7006: Introduces the regexp-aware String methods extended by this PR.
  • mruby/mruby#7025: Updates match-global behavior used by these mutation methods.
  • mruby/mruby#7054: Adds related regexp-aware String method behavior in the same implementation and tests.

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 and concisely describes the main change: adding regexp support to String#[]= and String#slice!.
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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@mrbgems/mruby-regexp/mrblib/string_regexp.rb`:
- Line 288: Replace the overridable args[0].match(self) dispatch in both
String#[]= at mrbgems/mruby-regexp/mrblib/string_regexp.rb:288-288 and
String#slice! at mrbgems/mruby-regexp/mrblib/string_regexp.rb:333-333 with the
resolved non-overridable regexp helper already used by String#match and
String#[].
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a711a366-ec96-4f55-8754-4816bbf841e6

📥 Commits

Reviewing files that changed from the base of the PR and between 1f3f28b and 70aa23d.

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

Comment thread mrbgems/mruby-regexp/mrblib/string_regexp.rb
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