mruby-regexp: drop the private UTF-8 encoder in favor of mrb_utf8_to_buf - #7094
Merged
Conversation
The regexp gem carried its own UTF-8 encoder, byte for byte the same as `mrb_utf8_to_buf` in src/string.c. Core gems already call the shared encoder through <mruby/internal.h>, as mruby-sprintf, mruby-pack, and mruby-string-ext do, and regexp.c in this gem already includes that header, so the private copy only duplicated code. The shared function returns 0 for a codepoint above U+10FFFF where the private copy assumed a validated input. The regexp compiler satisfies that assumption: every codepoint reaching `emit_codepoint` comes through `check_unicode_cp`, which rejects surrogates and anything past the last plane as "invalid Unicode range", so the zero-length case is unreachable and no caller changes behavior.
|
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 (3)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThe regexp compiler now uses ChangesRegexp UTF-8 encoding
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 |
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
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 regexp gem carried a private UTF-8 encoder,
mrb_re_utf8_encodeinre_utf8.c, whose implementation is byte for byte the same as
mrb_utf8_to_bufin src/string.c. This PR deletes the private copy andswitches its only caller,
emit_codepointin re_compile.c, to theshared function.
Using the shared encoder from a gem follows existing practice:
mruby-sprintf, mruby-pack, and mruby-string-ext all call
mrb_utf8_to_bufthrough<mruby/internal.h>, and regexp.c in thisgem already includes that header.
The one difference between the two functions is that
mrb_utf8_to_bufreturns 0 for a codepoint above U+10FFFF, where the private copy
assumed the caller had validated its input. The compiler satisfies that
assumption: every codepoint reaching
emit_codepointcomes throughcheck_unicode_cp, which rejects surrogates and anything past the lastplane as "invalid Unicode range" before encoding, matching CRuby:
So the zero-length case is unreachable and no caller changes behavior.
Net effect: 3 insertions, 31 deletions, no functional change.
rake testpasses (2050 tests, 0 failures; bintest 105 OK).Summary by CodeRabbit