mruby-regexp: rename __match_pattern to __check_pattern - #7005
Merged
Conversation
The helper was named after `String#match` and `String#match?`, the two methods it was written for. It now serves six: `sub`, `gsub`, `scan` and `split` check their pattern through it as well, so the name points at a minority of its callers. `__check_pattern` names what the helper does instead. It is a check and nothing more: an accepted argument comes back untouched and anything else raises. No caller uses the return value as a conversion. `match` and `match?` compile an accepted String themselves, `sub`, `gsub` and `scan` quote it first, and in `split` the String branch is unreachable because nil and String patterns have already gone to `__split`. The C function is renamed alongside the Ruby name, so the mismatch is not merely moved from one side of the binding to the other. The private `__check_pattern` on the `StringMatchHelperOverride` test subclass follows too: that test asserts a same-named method on the receiver cannot widen what the check accepts, which only holds while the name matches. `__check_pattern` is an undocumented `__`-prefixed helper and the rename changes no observable behaviour.
📝 WalkthroughWalkthroughThe regexp pattern-validation helper is renamed from ChangesRegexp pattern helper rename
Estimated code review effort: 2 (Simple) | ~10 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/src/regexp.c`:
- Line 1160: Preserve the existing public Regexp.__match_pattern API by
retaining or adding a class-method alias alongside the updated internal method
registration in regexp_check_pattern. Update internal mruby-regex and Ruby
callers to use the new method as needed, while ensuring direct callers of
__match_pattern continue to resolve compatibly.
🪄 Autofix (Beta)
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: c4b81380-4019-4336-a129-e468b0e03cde
📒 Files selected for processing (3)
mrbgems/mruby-regexp/mrblib/string_regexp.rbmrbgems/mruby-regexp/src/regexp.cmrbgems/mruby-regexp/test/regexp.rb
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The pattern type check is a single C function serving six
Stringmethods,but it is still named after the two it was written for.
string_regexp.rbString#match:14String#match?:20String#sub:45String#gsub:72String#scan:115String#split:153Line numbers are those of 8f85a84, the current master.
matchandmatch?are two of six now, so the name points at a minorityof the callers.
__check_patternnames what the helper does rather than who calls it. Itis a check: an accepted argument comes back untouched, anything else
raises, and no caller treats the return value as a conversion.
matchandmatch?compile an acceptedStringthemselves.sub,gsubandscanquote it first.splitnever sees one: nil andStringpatterns have already gone to__splitby then, so that branch is unreachable.The C function
regexp_match_pattern()is renamed to
regexp_check_pattern()in the same change, so the mismatchis not moved from the Ruby side of the binding to the C side.
The private
__check_patternon theStringMatchHelperOverridetest subclass is renamed too. It is not a caller, but the assertion built
on it is that a same-named method on the receiver cannot widen what the
check accepts, and that only means anything while the name matches the
helper.
Compatibility
None affected.
__check_patternis an undocumented__-prefixed helper,and the rename changes no observable behaviour.
Why this was not folded into #7001
#7001 is what widened the caller set, and the rename was kept out of it
deliberately: it touches the
matchandmatch?lines, which #7001 doesnot go near, and
CONTRIBUTING.mdasks that a pull request not mixseveral things. The comment above the C function was updated there to name
all six callers instead, so the name and the reality did not contradict
each other in the meantime.
Testing
rake testpasses: 1922 OK, 0 KO, 18 skipped (all pre-existing).Summary by CodeRabbit