mruby-regexp: Unicode simple case folding for /i behind an option - #7058
Conversation
|
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 (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds optional Unicode case folding for mruby-regexp. The change adds generated Unicode tables, UTF-8 folding APIs, folded literal and class compilation, variable-width backreference matching, build-specific tests, CI configuration, and documentation. ChangesUnicode regexp case folding
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant RegexpCompiler
participant UTF8Folding
participant UnicodeTables
participant Matcher
RegexpCompiler->>UTF8Folding: Fold literals and character-class ranges
UTF8Folding->>UnicodeTables: Look up Unicode fold runs
UnicodeTables-->>UTF8Folding: Return fold mappings
UTF8Folding-->>RegexpCompiler: Return folded classes and literals
Matcher->>UTF8Folding: Fold decoded backreference characters
UTF8Folding-->>Matcher: Return folded value and consumed byte length
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
9a09a20 to
af2c4b1
Compare
|
Thank you for asking the question before the patch, and for saying plainly that you would rather be told to drop it. That framing is what made it easy to answer. The answer
So: your The reason is the case you singled out. You wrote that the negated class is the one you found hardest to file under "ASCII only" rather than under "wrong", and I agree, but I would go further: it is not a separate defect. Raising is what makes the two builds differ in a way I can defend. Between them the answer changes from "correct" to "explicitly refused", never from "correct" to "wrong". A One thing the rejection must not beDo not reject on "the pattern contains a byte above 127". That would break a large body of patterns that are correct today. I checked on current master: /日本/i.match?("日本") # true
/です/i.match?("です") # true
/العربية/i.match?("العربية") # true
/😀/i.match?("😀") # trueThose are right, and they are right for a reason that survives the change: there is nothing to fold, so folding ASCII only is the whole of the correct answer. A script without case is not a limitation of the build. The test is therefore "does this codepoint have a case folding", not "is this codepoint non-ASCII". You need less data for it than for the mapping: a list of the ranges that contain cased characters, not the foldings themselves. Latin supplements and extended, Greek, Cyrillic, Armenian, Georgian, the Latin and Greek extended additional blocks, letterlike symbols so that What I measuredBuilt your branch both ways,
23 KB when it is on and nothing when it is off, and the test file excludes itself from a build that does not define it. The run-length encoding is doing real work there; a flat mapping table would have been several times that. On the cost to existing codePatterns like |
bd0bb7d to
844fae7
Compare
|
Done, and thank you for the answer. It is on the branch as three commits on top The refusal
The merge distance is the only knob, and I set it at 16. It pulls 309 uncased The test is on the folding rather than the byte, so nothing above is what /日本/i.match?("日本") # true
/です/i.match?("です") # true
/العربية/i.match?("العربية") # true
/😀/i.match?("😀") # trueThe one place I did not do it your wayTwo foldings are carried by every build rather than refused: U+017F folds to Refusing them was not open to me. A fold that reaches ASCII cannot be refused /k/i.match?("K") # CRuby: true, mruby before: false
/[^k]/i.match?("K") # CRuby: false, mruby before: trueThat is the negated class again, in the build that has no data, on a pattern It is also most of what a build without the option now pays. Of the 1855 bytes Two things the refusal turned upBoth are in the build that defines the option, both are the shape you refused A character class closed one hop from what was written rather than under /[Σ]/i.match?("ς") # CRuby: true, mruby before: false
/[^Σ]/i.match?("ς") # CRuby: false, mruby before: trueU+03A3 and U+03C2 both fold to U+03C3. Closing it properly takes two rounds, The table took a source only when its full fold was one codepoint, which threw /ß/i.match?("ẞ") # CRuby: true, mruby before: false
/[^ß]/i.match?("ẞ") # CRuby: false, mruby before: trueU+1E9E folds to "ss" and lower cases to U+00DF, and the two fold alike, so What I measuredx86_64-linux, full-core, gcc, text plus rodata. What the option costs, same
Down from +5955 on the gem, because the compile side of the fold now serves What a build without the option pays, against master:
Not the few hundred bytes you sized the table at, and I want to be plain about On your configuration, full-core host debug, What the sweep saysEvery folding against CRuby 4.0.6 in six forms each: literal in both Without the option, 1481 of the 1483 pairs are refused in every form and two With the option, 36 of the 8898 answers differ, every one of them mruby Left standingWith the option, The README carries the error next to the option, as you asked, and the Still openThe CI job is the question you did not answer, and it is the part of this that |
|
Sorry for leaving the CI question unanswered. You were right to hold the draft on it rather than guess. Keep the coverage, but put the option in an existing config rather than in a job of its own. CI should build with Concretely: drop What that gives up is Everything else in your reply I am glad to take as written:
I verified both builds here before answering: 2186 OK / 0 KO default, 2177 OK / 0 KO with the option, ASan and UBSan clean in both, and the thirteen rows I compared against CRuby agree in the configuration that claims to. Mark it ready once the CI change is in and I will merge it. |
The gem README documents ASCII-only folding as a limitation, and mruby core
carries no Unicode case data at all, so this is a scope change rather than a
bug fix. It is a draft for that reason: the question is whether the limitation
should stand, not whether this patch is ready.
Covers the 1:1 foldings. A source whose fold is several codepoints (U+00DF to
"ss", 104 sources) is left alone; CRuby itself is unsettled there.
`tools/gen_casefold.rb` generates `re_casefold.h` from the host CRuby's
Unicode data. Run-length encoding the 1455 non-ASCII sources by stride and
delta collapses them to 198 runs.
Three changes, no new opcode and no change to any data structure:
`emit_char_folded()` emits a non-ASCII literal as a class rather than as a run
of `RE_CHAR` bytes when `/i` is on. This is what lets a counterpart of a
different width work: `RE_CLASS` decodes one codepoint and compares that, so
U+212A against `/k/i` is one comparison rather than three bytes against one.
Both spellings of a literal go through it, since a backslash before a
multibyte character has no escape meaning and `/\Ā/i` is `/Ā/i`.
`compile_charclass()` folds the codepoint list the way it already folds the
bitmap. Ranges are walked run by run, so a wide range costs 198 intersections
rather than its own length.
`memcmp_ci()` compares codepoints instead of bytes and reports how many bytes
it consumed, since a folded comparison need not consume as many bytes as the
captured text holds.
Everything above sits behind `MRB_REGEXP_UNICODE_CASE`. The `full-debug` build
in `build_config/ci/gcc-clang.rb` defines it, because without a build that
does the tests never execute: the gem's `mrbgem.rake` drops
`test/unicode_case.rb` from a build that does not ask for the option, and
every assertion in it would fail otherwise. That build already runs in the
`gcc-clang` job, so the coverage costs no runner, and the other two builds in
the file keep the default.
Measured on x86_64-linux, text segment. What the option costs, same
configuration with and without the define:
re_compile.o 18789 -> 20821 +2032
re_exec.o 14383 -> 14567 +184
re_utf8.o 528 -> 4299 +3771
regexp.o 21969 -> 21969 0
gem total 55669 -> 61656 +5987
bin/mruby 1789525 -> 1795565 +6040
Of the 3771 bytes in `re_utf8.o`, 2376 are the table and 1619 the three lookup
functions. Packing the run struct from 12 bytes to 8 would take the table to
1584.
What a build without the option pays, against the parent commit:
re_exec.o 14231 -> 14383 +152
gem total 51549 -> 51701 +152
bin/mruby 1698039 -> 1698199 +160
Those 152 bytes are `memcmp_ci()` keeping one signature across both builds
rather than two. Splitting the call site under `#ifdef` too would take it to
zero, at the cost of a third conditional in the middle of `RE_BACKREF`.
Swept all 1455 pairs against CRuby 4.0.6 in six forms each: literal in both
directions, class, negated class, backreference, and the same pattern without
`/i`. 1420 pairs agree exactly. The 35 that differ are exactly the pairs whose
UTF-8 length changes, and in every one mruby matches a superset of what CRuby
matches, never less: 33 differ only in the backreference form and 2 in one
literal direction, where Onigmo declines to fold across a width change. No
negated class matched in either implementation, and no pattern without `/i`
matched in either, so the fold leaks into neither.
A class under `/i` took the counterparts of what was written and stopped
there. A fold can have more than one source, so the class missed whatever it
could only have reached through the fold it shares with them:
```ruby
/[Σ]/i.match?("ς") # CRuby: true, mruby: false
/[^Σ]/i.match?("ς") # CRuby: false, mruby: true
```
U+03A3 and U+03C2 both fold to U+03C3. The class held U+03A3, the one hop
reported U+03C3, and U+03C2 was never asked for. The negated form is the same
gap with its sign flipped, so it accepts what it was written to reject.
Closing the class properly means: x belongs to it whenever some written member
folds the same way x does. That takes two rounds. The first adds the fold of
every member, the second adds every source of a member, and the members the
second round reads include what the first round put there. A third round finds
nothing, since whatever the second adds folds to something the first already
added.
`mrb_re_case_unfold_range()` walked both directions at once, which cannot
serve two rounds, so it splits into `mrb_re_case_fold_range()` for the folds
of the sources in a span and `mrb_re_case_unfold_range()` for the sources of
the folds in it. Neither is more work than the single walk was: each still
reads the table once, run by run.
The ordering half of the same defect was the ASCII closure running before the
Unicode pass rather than after it, so a counterpart landing in the bitmap
arrived too late for it:
```ruby
/[K]/i.match?("K") # CRuby: true, mruby: false; pattern holds U+212A
```
The class held U+212A, the pass folded it to "k", and nothing then asked for
the other case of "k". Round two now walks the bitmap upwards and adds the
upper case letter as it goes, which is behind its own cursor and so is never
asked for sources of its own. That is correct rather than lucky: nothing folds
to an upper case letter.
Both are defects of the build that defines `MRB_REGEXP_UNICODE_CASE`. Without
it the class holds no non-ASCII member to walk and nothing changes, which is
also why the assertions here are in `test/unicode_case.rb`. The ordering half
has no home there yet: asserting it means writing U+212A against an ASCII
class, and that is a pattern both builds should agree on rather than one, so
it waits for the commit that gives the ASCII-only build the same folding and
lands in `test/regexp.rb` beside it.
The table took a source only when its full fold was one codepoint, which threw
away sources whose fold is longer but which still have one codepoint to pair
with. U+1E9E is the one that matters: it folds to "ss", so it was skipped, and
it lower cases to U+00DF, so pairing it needs nothing the 1:1 machinery does
not already do.
```ruby
/ß/i.match?("ẞ") # CRuby: true, mruby: false
/[^ß]/i.match?("ẞ") # CRuby: false, mruby: true
```
The negated form is the same gap with its sign flipped, which is the shape
that answers the opposite of what was written rather than merely missing.
`tools/gen_casefold.rb` now falls back to the simple lower case mapping when
the fold is longer than one codepoint, and takes it when it is a single
codepoint that folds the same way the source does. That adds 28 pairs: U+1E9E,
and the 27 Greek capitals with prosgegrammeni (U+1F88 to U+1FFC) that pair
with their small forms. The table goes from 1455 sources in 198 runs to 1483
in 205.
What stays out is a source with no single counterpart at all, 76 of them,
U+00DF and U+FB00 to "ff" among them. Matching those means expanding one
character into several, which no structure here has a place for and which
CRuby is itself unsettled about, see
https://bugs.ruby-lang.org/issues/17989 and
https://bugs.ruby-lang.org/issues/17990. They are missed matches in every
form, negated class included, so none of them answers the opposite of what it
says.
Only the build that defines `MRB_REGEXP_UNICODE_CASE` reads the table, so
nothing else changes.
A build without `MRB_REGEXP_UNICODE_CASE` used to fold ASCII and answer
anyway, so `/Ā/i` missed "ā" and, the same gap with its sign flipped, `[^Ā]/i`
accepted it. It raises `RegexpError` at compile time now instead. Between the
two builds the answer changes from correct to explicitly refused, never from
correct to wrong.
The test is whether a codepoint has a case folding, not whether it is
non-ASCII. A script without case has nothing to fold, so folding ASCII is the
whole of the right answer for it and these go on working:
```ruby
/日本/i.match?("日本") # true
/です/i.match?("です") # true
/العربية/i.match?("العربية") # true
/😀/i.match?("😀") # true
```
`re_cased.h` carries what the test needs: 2982 cased codepoints as 32 coarse
ranges, 256 bytes against the 2460 the mapping costs. Merging neighbours less
than 16 apart pulls 309 uncased codepoints into those ranges, and they are
refused as well. That costs a pattern ASCII folding would have answered
correctly, which is the better failure of the two. The generator emits it in
the same pass that emits the mapping, so the two cannot drift into letting
through a codepoint the other would have folded.
Two foldings are carried by every build rather than refused: U+017F folds to
"s" and U+212A to "k", the only two whose result is an ASCII letter. Refusing
a fold that reaches ASCII would mean refusing `/k/i` and `/s/i`, which breaks
a large body of patterns that are correct today. Answering without them is no
better, for the same reason the refusal exists at all:
```ruby
/k/i.match?("K") # CRuby: true, mruby before: false
/[^k]/i.match?("K") # CRuby: false, mruby before: true
```
So "ASCII case folding" now means the whole of the equivalence class an ASCII
letter belongs to rather than the part of it that happens to be ASCII.
`mrb_re_case_fold()` and `memcmp_ci()` are shared by both builds for that
reason, which is also what makes a backreference under `/i` fold. It is what
lets the class form of that pattern be asserted for both builds at once, and
with it the ordering the previous commit but one fixed.
`test/ascii_case.rb` holds the refusals and runs only in a build without the
option, mirroring `test/unicode_case.rb`. What `/i` does the same way in both
builds is in `test/regexp.rb` and always runs. The straddling range there
moves its upper bound to an uncased codepoint, since what `/i` does with a
range whose non-ASCII half has case is now one of the things the two builds
answer differently. The README documents the error next to the option, and the
backreference superset next to the other limitations.
Measured on x86_64-linux with full-core, gcc, text plus rodata.
What the option costs, same configuration with and without the define:
re_compile.o 20457 -> 21376 +919
re_exec.o 14535 -> 14535 0
re_utf8.o 1126 -> 4496 +3370
regexp.o 21985 -> 21985 0
gem total 58103 -> 62392 +4289
bin/mruby 1792389 -> 1796689 +4300
Down from +5955 on the gem, because the compile side of the fold now serves
both builds rather than only the one that asks for the option.
What a build without the option pays, against master:
re_compile.o 19360 -> 20457 +1097
re_exec.o 14231 -> 14535 +304
re_utf8.o 672 -> 1126 +454
gem total 56248 -> 58103 +1855
bin/mruby 1790869 -> 1792389 +1520
Of the 454 bytes in `re_utf8.o`, 342 are the range table and the test that
reads it. The rest is the two foldings that reach ASCII: 1097 bytes in
`re_compile.o` for the refusal and for reaching them from the class and the
literal paths, and 304 in `re_exec.o` for `memcmp_ci()` comparing codepoints
rather than bytes, 152 of which the option already cost a build without it.
Swept every folding against CRuby 4.0.6 in six forms each: literal in both
directions, class, negated class, backreference, and the same pattern without
`/i`.
Without the option, 1481 of the 1483 pairs are refused in every form and two
answer, U+017F and U+212A. Nothing answers differently from CRuby except those
two in the backreference form, where mruby matches a superset. The 76 sources
with no single counterpart are refused in every form. Eighteen uncased
codepoints spanning CJK, kana, Arabic, Hangul, combining marks and emoji are
refused in no form and agree with CRuby everywhere.
With the option, 36 of the 8898 answers differ from CRuby, every one of them
mruby matching where CRuby does not: two in one literal direction and 34 in
the backreference form, where Onigmo declines to fold across a width change.
844fae7 to
1ff3550
Compare
|
Thank you. The CI change is in and this is ready.
One thing you offered to give up you do not have to. Putting the define on one What the rebase turned up#7066 landed while this sat, and it made /\Ā/i.match?("ā") # with the option: false, where /Ā/i answers true
/\Ā/i # without the option: compiled, where /Ā/i raisesNeither answers the opposite of what it says, but the second one is exactly Measured againThe size tables and the CRuby sweep in the description are re-run on the Without the option, 9190 of those are refusals, 1481 of the 1483 pairs are
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
mrbgems/mruby-regexp/tools/gen_casefold.rb (1)
83-84: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRequire
rbconfigbefore usingRbConfig::CONFIG.With
ruby --disable-gems,RbConfigis undefined, so the generator raisesNameErrorwhen it builds the generated header. Addrequire 'rbconfig'next torequire 'set'.🤖 Prompt for 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. In `@mrbgems/mruby-regexp/tools/gen_casefold.rb` around lines 83 - 84, Add require 'rbconfig' alongside the existing require 'set' in gen_casefold.rb before the generated-header code references RbConfig::CONFIG, ensuring the generator also works with ruby --disable-gems.
🤖 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/include/re_internal.h`:
- Around line 180-186: Correct the contract described near the no-fold codepoint
check: it must not claim that every listed codepoint could be folded by a full
Unicode build, since multi-codepoint sources such as U+00DF and U+FB00–U+FB17
are only present as case-folding data. Either revise the comment to describe the
check as detecting available case-folding data, or update gen_casefold.rb so
no-counterpart sources are excluded from cased and both builds handle them
consistently.
In `@mrbgems/mruby-regexp/src/re_compile.c`:
- Around line 804-812: Pass the pattern’s binary state through mrb_re_compile
into emit_char_folded, and bypass Unicode folding for binary patterns or when
mrb_re_utf8_decode reports len == 1 for invalid/truncated UTF-8. In both cases,
emit the original byte via emit_char_bytes; retain existing folding behavior for
valid non-binary UTF-8 characters.
---
Nitpick comments:
In `@mrbgems/mruby-regexp/tools/gen_casefold.rb`:
- Around line 83-84: Add require 'rbconfig' alongside the existing require 'set'
in gen_casefold.rb before the generated-header code references RbConfig::CONFIG,
ensuring the generator also works with ruby --disable-gems.
🪄 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: 752784ee-0df0-4bca-9cef-bed6327dd34b
📒 Files selected for processing (13)
build_config/ci/gcc-clang.rbmrbgems/mruby-regexp/README.mdmrbgems/mruby-regexp/include/re_internal.hmrbgems/mruby-regexp/mrbgem.rakemrbgems/mruby-regexp/src/re_cased.hmrbgems/mruby-regexp/src/re_casefold.hmrbgems/mruby-regexp/src/re_compile.cmrbgems/mruby-regexp/src/re_exec.cmrbgems/mruby-regexp/src/re_utf8.cmrbgems/mruby-regexp/test/ascii_case.rbmrbgems/mruby-regexp/test/regexp.rbmrbgems/mruby-regexp/test/unicode_case.rbmrbgems/mruby-regexp/tools/gen_casefold.rb
`emit_char_folded()` decoded every byte above 127 as UTF-8, and
`mrb_re_utf8_decode()` answers an invalid or truncated sequence with one byte
consumed and that byte's own value for the codepoint. A lone 0xB5 therefore
reached the folding path as U+00B5, a character the pattern does not hold:
```ruby
# with MRB_REGEXP_UNICODE_CASE
Regexp.new("\xB5", Regexp::IGNORECASE) =~ "µ" # was 0, master: nil
# without it
Regexp.new("\xB5", Regexp::IGNORECASE) # was RegexpError, master: compiles
```
The refusal is the same mistake read the other way round. U+00B5 is one of the
codepoints a build without the option cannot fold, so a pattern holding a byte
that is not that character at all failed to compile.
A byte that starts no whole character is not a character to fold. The literal
path emits those as bytes everywhere else, which is what `emit_char_bytes()`
is for, so a decode that consumed one byte falls back to it and both builds
answer what they answered before this branch.
The class path is left as it was. It reads such a byte as a codepoint, which
it did before this branch too: a class compares the decoded codepoint and a
literal compares bytes, so `[\xB5]` has matched "µ" for as long as classes
have decoded, and a build without the option refuses `[\xB5]` under `/i` for
the same reason it refuses `[µ]`.
The test is in `test/regexp.rb`, since the two builds agree on all of it.
The comment on `mrb_re_needs_case_data()` said it answers TRUE for a codepoint
this build cannot fold but a build with the table could. Two kinds inside it
fold in no build at all.
`gen_casefold.rb` records a codepoint as cased as soon as it has a folding,
before it knows whether that folding pairs it with a single counterpart, so
the 76 sources whose fold expands into several codepoints are in `re_cased.h`
as well. The coarse ranges then close over 309 uncased neighbours, and those
fold nowhere either.
Both kinds are refused by a build without the option and compiled by a build
with it, which matches them literally:
```ruby
/ff/i.match?("ff") # RegexpError without the option, false with it
/ff/i.match?("ff") # RegexpError without the option, true with it
```
So the two builds differ there in what they refuse rather than in what they
answer, which is the direction the refusal runs in everywhere else. The
generator keeps recording them and the comment says what the test is: having
the data, not being foldable. `re_cased.h` already describes itself that way.
`gen_casefold.rb` reads `RbConfig::CONFIG['UNICODE_VERSION']` for the header comment it writes. `ruby --disable-gems` leaves `RbConfig` undefined, so the generator raised `NameError` there, after it had opened `re_casefold.h` and before it had written anything to either file. Both headers regenerate byte for byte identical under `--disable-gems` with the require in place.
|
The Reproduced first. |
2 participants
This started as a question rather than a patch: should
/istay ASCII only?It was answered in #7058 (comment).
/imay widen behind the option, and a build that does not define the optionshould raise rather than fold ASCII and answer anyway. This is that
decision implemented.
The CI question is answered too, in
#7058 (comment), so the draft
is lifted.
Since the last revision
build_config/ci/unicode-case.rband theUnicode-casejob are deleted, and
build_config/ci/gcc-clang.rbsets the define on itsfull-debugbuild instead, as asked. No new runner, and no change to.github/workflows/build.ymlat all.keeps what you were willing to give up:
full-debugrunstest/unicode_case.rb, the other two builds keep the default and runtest/ascii_case.rb, so both sides are covered by the one job.\Āone atomrather than a lead byte followed by continuation bytes. That path reached
neither the folding nor the refusal, so
/\Ā/iquietly missed"ā"with theoption and quietly compiled without it. Both spellings now go through one
helper,
emit_char_folded(), and both test files carry the escaped form.they do not line up with the numbers in the previous revision.
What the two builds do
With
MRB_REGEXP_UNICODE_CASE,/ifolds the Unicode pairings:Without it, a pattern that needs one of those does not compile:
Between the two builds the answer changes from correct to explicitly refused,
never from correct to wrong. Before this,
[Ā]under/imissed"ā"and[^Ā]accepted it, which is the same missing data with its sign flipped.The test is whether a codepoint has a case folding, not whether it is
non-ASCII. A script without case has nothing to fold, so ASCII folding is the
whole of the right answer for it and both builds agree:
Two foldings are carried by every build rather than refused: U+017F folds to
"s"and U+212A to"k", the only two whose result is an ASCII letter.Refusing them would mean refusing
/k/iand/s/i, and answering without themleaves the same false accept the refusal exists to prevent, so both builds
carry the two codepoints:
Folding "ASCII only" therefore covers the whole of the equivalence class an
ASCII letter belongs to rather than the part of it that happens to be ASCII.
What it costs
Unicode 17.0.0 has 1585 codepoints whose simple fold differs from themselves.
1483 pair with a single other codepoint, 26 of which are the ASCII letters
handled inline. Run-length encoding the 1483 sources by stride and delta
collapses them to 205 runs, 2460 bytes. The other 76 have no single
counterpart at all (U+FB00 to
"ff") and are out of scope; CRuby is unsettledaround those, see https://bugs.ruby-lang.org/issues/17989 and
https://bugs.ruby-lang.org/issues/17990.
re_cased.h, the table a build without the option reads to decide what torefuse, is 2982 cased codepoints as 32 coarse ranges, 256 bytes. Both headers
come from one pass over the same data, so they cannot drift into letting one
build refuse what the other would not have folded.
Measured on
x86_64-linux, full-core, gcc, text plus rodata. What the optioncosts, same configuration with and without the define:
re_compile.ore_exec.ore_utf8.oregexp.obin/mrubyWhat a build without the option pays, against master:
re_compile.ore_exec.ore_utf8.obin/mrubyI want to be plain about the second table. Of the 414 bytes in
re_utf8.o,342 are the range table and the test that reads it, which is the refusal
itself. The rest is the two foldings that reach ASCII: 1288 bytes in
re_compile.ofor reaching them from the class and the literal paths, and 176in
re_exec.oformemcmp_ci()comparing codepoints rather than bytes so abackreference folds them too. Drop those two foldings and the cost falls back
to a few hundred bytes, at the price of
[^k]/iaccepting U+212A again. Thatpiece is isolated and I will take it out if that trade is preferred.
How it works
No new opcode and no change to any data structure.
emit_char_folded()emits a non-ASCII literal as a class rather than a run ofRE_CHARbytes when/iis on and the character has a counterpart. This iswhat lets a counterpart of a different width work at all:
RE_CLASSdecodesone codepoint and compares that, so U+212A against
/k/iis one comparisonrather than three bytes against one. A character with no counterpart, which is
most of the non-ASCII range, still emits as bytes and costs
/inothing. Bothspellings of a literal go through it, since a backslash before a multibyte
character has no escape meaning and
/\Ā/iis/Ā/i.compile_charclass()closes the class under folding: x belongs to it wheneversome written member folds the same way x does. That takes two rounds, the folds
of the members and then the sources of those, because a fold can have more than
one source (U+03A3 and U+03C2 both fold to U+03C3) and a class written with one
of them reaches the others only through the fold they share. Ranges are walked
run by run, so a wide range costs 205 intersections rather than its own length.
memcmp_ci()compares codepoints instead of bytes and reports how many bytesit consumed, since a folded comparison need not consume as many as the captured
text holds.
Without the option, the same closure is restricted to the two foldings the
build has, and anything else in the codepoint list is refused before it is
reached.
mrb_re_needs_case_data()compiles away entirely in the build thathas the data.
tools/gen_casefold.rbgenerates both headers from the host CRuby's Unicodedata, so the tables can be regenerated rather than hand-maintained.
Testing
test/unicode_case.rbandtest/ascii_case.rbassert opposite things aboutthe same patterns, so
mrbgem.rakegives each build only the one that belongsto it. What
/idoes the same way in both builds is intest/regexp.rbandalways runs.
Verified on
x86_64-linux:rake test, the default build with the option off: 2002 OK, 0 KO, 0 crash,and bintest 105 OK.
full-debugbuild ofci/gcc-clangwith the define, which is theconfiguration this PR adds the option to and which carries
MRB_GC_STRESS:2194 OK, 0 KO, 0 crash.
MRB_INT32with clang and-Wall -Wextra, option off: 2079 OK, 0 KO, 0crash. Option on: 2078 OK, 0 KO, 0 crash. No warning from any file this
branch touches either way; the four
-Wunused-parameterwarnings under thegem are in
regexp.c, which this branch does not change.both directions, class, negated class, backreference, escaped literal, and
the same pattern without
/i. With the 76 sources that have no singlecounterpart and 18 uncased codepoints spanning CJK, kana, Arabic, Hangul,
combining marks and emoji, that is 10757 answers per build.
Without the option, 9190 of those answers are refusals and nothing answers
differently from CRuby except two: 1481 of the 1483 pairs are refused in
every
/iform, and the two that answer, U+017F and U+212A, differ only inthe backreference form, where mruby matches a superset. Every form of the 76
sources with no single counterpart is refused. The uncased codepoints are
refused in no form and agree with CRuby everywhere.
With the option, nothing is refused and 36 of the 10757 answers differ, every
one of them mruby matching where CRuby does not: 34 in the backreference form,
where Onigmo declines to fold across a width change, and 2 in one literal
direction. Adding the escaped form to the sweep turned up no difference of its
own in either build, which is the point of routing both spellings through one
helper.
Left standing
With the option,
/ß/istill does not match"ss", nor/ff/imatch"ff".Those need a fold that expands one character into several, which nothing here
has a place for. It is a missed match in every form, negated class included,
so no pattern answers the opposite of what it says. Without the option all 76
are refused. The README carries the error next to the option and the
backreference superset next to the other limitations.
Base
Rebased onto master at 0db635c. The branch carries only its own four
commits.
Summary by CodeRabbit
New Features
/i.Bug Fixes
Documentation
Tests