mruby-regexp: follow a block that changes the receiver in gsub, sub! and scan - #7267

Rebased onto master now that #7274, which moves the block walk of gsub into C, is merged: the two commits here are all this PR is. The semantics, the tests and the differential run are the ones reviewed above, with the gsub part living in the C loop, where the cost question of that review does not arise. Every figure below is measured against master as it stands after that merge.

The block forms of String#gsub, gsub!, sub! and scan in mruby-regexp did not answer for a block that writes to the receiver the way CRuby's str_gsub, rb_str_sub_bang and rb_str_scan do. Measured against CRuby 4.0.6:

s = "hello"; s.gsub(/l/) { s.tr!("h", "H"); "X" }
# CRuby: "HeXXo"   mruby before: "heXXo"   mruby after: "HeXXo"

s = "abc"; s.gsub(/b/) { s.replace("xyz"); "!" }
# CRuby: "x!z"     mruby before: "a!z"     mruby after: "x!z"

t = "hello"; n = 0
t.gsub(/l/) { n += 1; t.upcase! if n == 2; "X" }
[$&, $`, $']
# CRuby: [nil, nil, nil]   mruby before: ["l", "hel", "o"]   mruby after: [nil, nil, nil]

s = "abc"; s.gsub(/b/) { s << "zz"; "!" }
# CRuby: RuntimeError (string modified)   mruby before: "a!czz"   mruby after: RuntimeError (string modified)

s = "hello"; s.sub!(/l/) { s.upcase!; "X" }
# CRuby: "HEXLO"   mruby before: "heXlo"   mruby after: "HEXLO"

s = "hello"; s.scan(/l/) { s.upcase! }; $~ && $~[0]
# CRuby: nil       mruby before: "l"       mruby after: nil

Three things were off in what the loops read and when. The gsub loop copied the stretch before each match before the block ran, so a change the block made there was lost from the answer while the next match was already searched for in the changed string. A block that changed the length was let through in every loop, and the offsets of the match then named other bytes than the ones it matched; CRuby refuses one in str_mod_check right after the block returns. And the match left in $~ was the MatchData of the last match the loop had, published again; CRuby searches once more when the loop is over, from the offset the last match was found from and on the receiver as it stands then, which is nil where the block wrote the match away and a fresh match on the changed string otherwise. sub! went down to sub, which builds the answer from the snapshot the MatchData holds, where rb_str_sub_bang splices the replacement into the receiver as the block left it.

Fix

Two commits.

The first takes gsub and sub!. Regexp.__gsub_block, the C loop #7274 put on master, calls the block first and copies the stretch before the match afterwards, compares the receiver's byte length after each call against what it was when the loop began and raises RuntimeError where it differs (str_mod_check compares the buffer pointer too, which is no test here: mruby answers a write into a shared string with a buffer of its own, which is also why the bytes, their reading and the encoding check are taken afresh from the receiver the block left rather than through the pointer the search was given), keeps the offset each match was found from and the match itself, and ends on a search from that offset where the receiver no longer reads as it did when that match was made. Where it still does, the search could only find that match again, so the loop publishes it again: a MatchData holds a frozen copy of its subject, and what a search reads of a subject is its bytes and whether they are read by byte, so the receiver is compared against that copy on both counts. A change of length fails the comparison, so do tr! and upcase!, which keep the length and change the bytes, and so does s.replace(s.b), which keeps every byte and changes the reading; where the receiver still shares its buffer with the copy, the pointer settles it. In C the length check is one comparison per turn and the copy test a pointer compare or a memcmp in the same function. gsub! inherits all of it. The block form of sub! runs the block itself in mrblib, applies the same length check, and splices the replacement into the receiver by the byte offsets of the match. sub is unchanged: CRuby's rb_str_sub works on a copy, so the block reaches nothing it reads.

The second takes scan, whose block loop stays in mrblib. It hands the length it began with to every search it makes after the block, and Regexp.__byte_search takes it as a fourth argument and raises RuntimeError before it looks where the receiver no longer has it, so the loop pays one argument per search rather than one bytesize call per match; it keeps the offset each match was found from and the match itself, and ends on a search from that offset unless MatchData#__republish(str), the copy test above asked from mrblib, publishes the match again. MatchData#__set_globals, which republished the last match whatever the block had done to the receiver, loses its last caller there and goes.

Testing

• mrbgems/mruby-regexp/test/match_data.rb: the test that pinned the republished MatchData after a gsub block mutated the subject now pins CRuby's answer ($~ nil where the match was written away, a match on the changed string where it was left in place, and the closing search running from the offset the last match was found from).
• mrbgems/mruby-regexp/test/string_regexp.rb: return values of gsub / gsub! / sub! / scan under a block that changes the receiver in place, RuntimeError for a change of length (in bytes, and against the length at the start of the loop, so a change undone inside the block passes), the receiver keeping what the block did to it, sub unaffected, FrozenError for a receiver the block freezes, a receiver long enough that the write moves it off the buffer the match was made on, and, for the empty-match step, s.clear and s.chop! under gsub(/x*/) and s.clear under scan(/x*/), which raise RuntimeError on CRuby. The assertion master gained in ba8a7c8, a replace of the same length under gsub(/(?=a)/) that tells reading the receiver's bytes again from reading the pointer the search was made with, stands inside this rewritten test as it was written.
• mrbgems/mruby-regexp/test/regexp_utf8.rb: the match left behind after a block that changed the reading of the receiver without changing a byte (s.replace(s.b)) counts its offsets in bytes, as CRuby's does after a force_encoding in the block; a comparison of bytes alone would take that receiver for unchanged.

Every value in the tests was measured on CRuby 4.0.6 first. The differential run from the earlier review, the four block forms over 7 subjects, 8 patterns and 16 kinds of in-block change, 3584 cases printed the same way on both sides and diffed against CRuby 4.0.6: master differs on 992, the first commit here on 223 (all of them scan), the tip on 0.

Full suite green at every commit: rake -m test on the default configuration at each commit, and MRUBY_CONFIG=build_config/ci/gcc-clang.rb rake -m test and MRUBY_CONFIG=build_config/gcc-asan.rb rake -m test at the tip, no sanitizer report.

Cost

Wall clock, the cases of the earlier review plus the String pattern block forms and a scan over the long subject, minimum of 5 alternating runs, -O3, default configuration. master has the C walk of #7274, so the block rows start from where that PR left them; the percentages are against master.

s480 = "hello world foo bar " * 24
30000.times { s480.gsub(/o/) { } }        # gsub480_blk
100000.times { "abc".gsub(/b/) { } }      # gsub_abc_blk
100000.times { "abc".scan(/b/) { } }      # scan_abc_blk
100000.times { "abc".sub!(/b/) { } }      # sub!_abc_blk
30000.times { s480.gsub(/o/, "0") }       # gsub480_str
100000.times { "abc".scan(/b/) }          # scan_abc
100000.times { "abc".sub!(/b/, "0") }     # sub!_abc_str
30000.times { s480.gsub(/z/) { } }        # gsub480_none
30000.times { s480.scan(/o/) { } }        # scan480_blk
30000.times { s480.gsub("o") { } }        # lit_gsub480_blk
100000.times { "abc".gsub("b") { } }      # lit_gsub_abc_blk
100000.times { "abc".sub!("b") { } }      # lit_sub!_abc_blk
100000.times { s = "hello"; s.gsub(/l/) { s.tr!("h", "H"); "X" } }        # gsub_tr
30000.times { s = s480.dup; s.gsub(/o/) { s.tr!("h", "H"); "0" } }        # gsub480_tr
100000.times { s = "hello"; s.scan(/l/) { s.tr!("h", "H") } }             # scan_tr

The gsub rows: the length comparison, the reading and the encoding flag read per turn cost about 2% on the 480 byte subject in the first commit (24ms over 2.16 million turns), a spread the tip's run of the same code lands inside, and 3% at most on the short ones; the closing test is the pointer alone there, since the receiver still shares its buffer with the snapshot. sub! with a block no longer goes down to sub, which is the -20% and -37% on its two rows. The scan rows carry the argument the search now parses and the __republish call, 1% on the long subject and 5% on the short one, and scan on "abc" without a block, gsub(/o/, "0") and sub!(/b/, "0") are unchanged. The mutating rows (gsub_tr, gsub480_tr, scan_tr) are where the closing search runs, and they sit on master to within 2%.

Size

.text of bin/mruby, build_config/ci/gcc-clang.rb, each side from a clean
build directory. regexp.o is the object that changes, and it accounts for the
whole delta in every build (to within 5 bytes of alignment in full-debug):
the copy test and the closing search in __gsub_block, __republish and the
length argument of __byte_search. The mrblib side, the block form of sub!
and the closing search of scan, is bytecode in mruby-regexp/gem_init.o's
.rodata, which grows by 128 (160 in full-debug).

On the default configuration, the one the wall clock was measured on, .text is
1,204,174 on master, 1,204,478 after the first commit and 1,204,894 at the tip
(+720).

Environment

# full-debug
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_GC_STRESS -DMRB_USE_DEBUG_HOOK -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER mrbgems/mruby-regexp/src/regexp.c
# bintest
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_GC_FIXED_ARENA -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -DMRB_USE_DEBUG_HOOK mrbgems/mruby-regexp/src/regexp.c
# cxx_abi
gcc -g -O3 -Wall -Wundef -Wwrite-strings -x c++ -std=gnu++03 -DMRB_GC_FIXED_ARENA -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_USE_CXX_EXCEPTION -DMRB_USE_CXX_ABI -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER mrbgems/mruby-regexp/src/regexp.c
# byte-string
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER mrbgems/mruby-regexp/src/regexp.c
# ascii-ctype
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_USE_ASCII_CTYPE -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER mrbgems/mruby-regexp/src/regexp.c
# gcc-asan
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -fsanitize=address,undefined -g3 -O0 -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER mrbgems/mruby-regexp/src/regexp.c
# default (no MRUBY_CONFIG), the build every figure above was measured on
gcc -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DMRB_USE_COMPLEX -DMRB_USE_BIGINT -DMRB_USE_DEBUG_HOOK mrbgems/mruby-regexp/src/regexp.c

Summary by CodeRabbit

• Bug Fixes
  • Improved String#sub, gsub, scan, and bang variants when the source string is modified during processing.
  • Added validation to detect source-length changes and prevent inconsistent results.
  • Preserved match information more reliably after substitutions and callbacks.
  • Improved handling of literal string patterns, replacement expansions, empty patterns, frozen strings, and invalid byte sequences.
  • Corrected UTF-8 and byte-based match indexing after block substitutions.