mruby-regexp: add Symbol#match, #match? and #=~ - #6993
Conversation
CRuby defines `match`, `match?` and `=~` on Symbol so that a symbol can be matched against a regexp without spelling out the `to_s`; mruby-regexp provided none of them, so any regexp use on a symbol raised NoMethodError. ```ruby :abc.match?(/b/) # CRuby: true, mruby: NoMethodError :abc =~ /b/ # CRuby: 1, mruby: NoMethodError ``` CRuby implements all three as the String method applied to the symbol's name (`rb_sym2str` then `rb_str_match_m` / `rb_str_match`), so they delegate to `to_s` rather than repeat the pattern handling. That inherits the String pattern compilation, the `pos` argument, the block form, and the `TypeError` for a String argument to `=~`. `$~` and `$1`-`$9` are set by the engine itself, so delegating does not lose them. `Symbol#match` needs the block to survive the delegation, which mruby#6991 made `String#match` do. Delegating also inherits one difference from CRuby that is worth naming: for an argument that is neither a Regexp nor a String, `String#=~` dispatches `re =~ self`, so `:a =~ nil` raises NoMethodError where CRuby returns nil. Fixing that belongs to `String#=~`, not to the Symbol wrapper. This covers the symbol-on-the-left direction only. The Regexp side still takes strings only, and rejects symbols in every entry point rather than just in `#===`: ```ruby /l/ =~ :hello # TypeError (CRuby: 2) /l/.match(:hello) # TypeError (CRuby: MatchData) /l/.match?(:hello) # TypeError (CRuby: true) /l/ === :hello # false (CRuby: true) [:to_s, :abc].grep(/^to_/) # [] -- Enumerable#grep goes through Regexp#=== ``` `sym[/re/]` is a third gap: `mruby-symbol-ext` already delegates `Symbol#[]` to `String#slice`, but this gem does not implement the regexp form of `String#[]` / `#slice`, so `"hello"[/l+/]` does not work either. Both are fixes on the String and Regexp side, so they are left alone here; the README Limitations section and the comment in symbol_regexp.rb now spell them out.
📝 WalkthroughWalkthroughThe change adds ChangesSymbol regexp matching
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Symbol
participant String
participant Regexp
Symbol->>String: convert symbol to string
String->>Regexp: perform match operation
Regexp-->>String: return match result
String-->>Symbol: return delegated result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
A note on merge order with #6994, which adds a The two PRs touch disjoint files, so they merge in either order without a "abc".match(:b) # before: NoMethodError
# after this PR: nil
# after #6994: TypeErrorMerging #6994 first avoids that window. Nothing here needs to change either |
`String#match` and `#match?` converted a `String` argument to a `Regexp` and handed `self` straight to anything else, so a pattern of the wrong type reached a method call it does not respond to. The failure named the argument as the receiver, which says nothing about the pattern being wrong. ```ruby "abc".match(:b) # CRuby: TypeError, mruby: NoMethodError for :b "abc".match(nil) # CRuby: TypeError, mruby: NoMethodError for nil ``` Resolve the pattern in one helper so both methods reject the same set, and follow CRuby in naming `nil`, `true` and `false` by value and everything else by class. The check runs before `pos` is inspected, matching CRuby's order. The helper is private, so the gem does not grow `String`'s public API with a method only `match` and `match?` call. ```ruby "abc".match(:b) # TypeError: wrong argument type Symbol (expected Regexp) "abc".match(nil, 99) # TypeError on the pattern, not an error about pos ``` CRuby also accepts an object that defines `to_str`, converting it through `rb_check_string_type` before the type check. mruby has no implicit `String` conversion in core, so `"a" + obj` and `"abc".index(obj)` already raise `TypeError` for such an object; honouring `to_str` in `match` alone would make the gem more permissive than the core it sits on. Rejecting `to_str` objects therefore stays an intentional difference from CRuby. `String#=~` deliberately keeps its current behaviour. CRuby's `rb_str_match` special-cases `String` only and dispatches everything else as `other =~ self`. ```ruby "abc" =~ 1 # CRuby: NoMethodError, mruby: NoMethodError "abc" =~ nil # CRuby: nil, mruby: NoMethodError ``` A blanket type check would turn both into `TypeError`, losing the case mruby already matches and freezing the other in a state it can never grow out of. `Object#=~` was removed in Ruby 3.2, so `nil` is the only receiver left to dispatch to, and the NoMethodError becomes CRuby's `nil` the moment core gains `NilClass#=~`. That is a core fix rather than one for this gem. This becomes more urgent once `Symbol#match` exists (#6993). `Symbol#match` and `#match?` delegate to `to_s`, so without the check the receiver and the argument swap places and the call quietly succeeds. ```ruby "abc".match(:b) # without the check: :b.to_s.match("abc") -> nil # with the check: TypeError ```
CRuby matches a Symbol against its name wherever a Regexp is given a subject, through `reg_operand()`. mruby-regexp took strings only, so all four entry points refused a symbol: ```ruby /a/ =~ :ab # CRuby: 0, mruby: TypeError /a/.match(:ab) # CRuby: MatchData, mruby: TypeError /a/.match?(:ab) # CRuby: true, mruby: TypeError /a/ === :ab # CRuby: true, mruby: false ``` `#===` is the one that matters most. It answers false instead of raising, so a `case` over symbols quietly falls through to `else` and `Enumerable#grep` returns an empty array, with nothing to show that the pattern was never given a chance to run. Convert the operand with `mrb_sym_str()` in one helper and call it from `regexp_match()`, `regexp_match_p()` and `regexp_match_op()` in place of `mrb_ensure_string_type()`. `regexp_case_match()` uses the same helper behind a type test, because `#===` has to keep answering false for a type it cannot match rather than start raising. `__byte_match` is left alone: it is internal, is only reached from mrblib with `self` as the subject, and its argument spec already pins the type. For a symbol too long for the inline representation, `mrb_sym_str()` returns an `mrb_str_new_static()` string sharing the symbol table's buffer. Mutating the `MatchData#string` that comes out of one is safe, because `str_modify()` copies an `RSTR_NOFREE` buffer before writing, but a test pins that. This is the Regexp-side half of the symbol support mruby#6993 adds on the Symbol side, and neither closes the other. mruby#6993 delegates `Symbol#match` and friends through `to_s`, so it never reaches the C code changed here; the two are independent and can land in either order. If mruby#6993 lands first, its "Symbols only on the left of a match" entry in the gem README becomes stale and should be dropped as part of the merge. `sym[/re/]` stays a third gap, waiting on the regexp form of `String#[]`.
CRuby defines
match,match?and=~on Symbol so that a symbol can bematched against a regexp without spelling out the
to_s; mruby-regexpprovided none of them, so any regexp use on a symbol raised NoMethodError.
CRuby implements all three as the String method applied to the symbol's name
(
rb_sym2strthenrb_str_match_m/rb_str_match), so they delegate toto_srather than repeat the pattern handling. That inherits the Stringpattern compilation, the
posargument, the block form, and theTypeErrorfor a String argument to
=~.$~and$1-$9are set by the engineitself, so delegating does not lose them.
Symbol#matchneeds the block to survive the delegation, which #6991 madeString#matchdo.Delegating also inherits one difference from CRuby that is worth naming: for
an argument that is neither a Regexp nor a String,
String#=~dispatchesre =~ self, so:a =~ nilraises NoMethodError where CRuby returns nil.Fixing that belongs to
String#=~, not to the Symbol wrapper.The argument type of
matchandmatch?is inherited the same way, andthere it interacts with #6994.
String#matchcurrently handsselftoanything that is not a String, so once
Symbol#matchexists the receiverand the argument swap places and a symbol pattern quietly matches instead of
raising:
#6994 adds that check in one helper on
String, which both receivers gothrough, so this PR needs no code for it. Merging #6994 first closes the
window; merging this one first opens it for as long as the two are apart.
The tests here therefore assert nothing about the argument type, and the
rows above are covered by #6994's tests instead.
This covers the symbol-on-the-left direction only. The Regexp side still
takes strings only, and rejects symbols in every entry point rather than just
in
#===:sym[/re/]is a third gap:mruby-symbol-extalready delegatesSymbol#[]to
String#slice, but this gem does not implement the regexp form ofString#[]/#slice, so"hello"[/l+/]does not work either. Both arefixes on the String and Regexp side, so they are left alone here; the README
Limitations section and the comment in symbol_regexp.rb now spell them out.
rake testpasses.Summary by CodeRabbit
match,match?, and=~.