Skip to content

Share the character count between core and mruby-string-ext - #7104

Merged
matz merged 4 commits into
mruby:masterfrom
takumin:core-str-char-len
Aug 12, 2026
Merged

Share the character count between core and mruby-string-ext#7104
matz merged 4 commits into
mruby:masterfrom
takumin:core-str-char-len

Conversation

@takumin

@takumin takumin commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Core counts the characters of a string in utf8_strlen(), reached through the
RSTRING_CHAR_LEN macro. mruby-string-ext counts them again in
str_char_count(), with the same rule in the same order: a binary or
single-byte string has one character per byte, anything else goes through
mrb_utf8_strlen().

The count is the last of the three questions asked about character positions
that had no shared answer. mrb_str_char_to_byte() and mrb_str_byte_to_char()
became internal API in #7097; this does the same for the count, so the three
sit together in internal.h and read alike where they are called.

mrb_str_char_len(mrb, str)

utf8_strlen() promoted, declared outside the MRB_UTF8_STRING guard with a
byte-length definition on the other side of it, exactly as the two conversions
are. Both RSTRING_CHAR_LEN definitions go away and the six uses in core call
the function.

Nothing in core changes behavior: the same function is doing the same work
under a different name.

mruby-string-ext

str_char_count() is deleted and its eight call sites, in ljust, rjust,
center and slice!, call mrb_str_char_len(). Because the helper is
available on a build without UTF-8 support, String#slice! loses one
#ifdef MRB_UTF8_STRING.

One difference comes with the move: core marks a string single-byte once it
finds its byte length and character count agree, so a later question about the
same string is answered without walking it again. The copy in the gem dropped
that.

Base

Branched off #7103, which is still open, because both touch String#slice!.
The first two commits here are that PR and drop out once it merges.

Verified

  • rake test on a full-core build (MRB_UTF8_STRING through mruby-encoding):
    2237 tests, all green
  • rake test on the default gembox (no MRB_UTF8_STRING): 2053 tests, all
    green
  • prek run --all-files passes, except that markdownlint could not install
    locally (npm engine mismatch); no Markdown is touched here

Summary by CodeRabbit

  • Bug Fixes
    • Improved UTF-8 handling for String#slice!, including character-based ranges, negative indexes, and multibyte matches.
    • Prevented byte sequences beginning inside a multibyte character from being incorrectly matched.
    • Improved consistency of string indexing, slicing, assignment, reversal, justification, and centering across UTF-8 and binary strings.
    • Preserved correct behavior for single-byte and binary strings while optimizing character counting.

`String#slice!` walked the string itself to turn a character index into a
byte offset, in `str_char_to_byte_offset()` and `str_chars_to_byte_len()`.
Both handed `mrb_utf8len()` an end of `p + byte_len - byte_offset`, which
is the end of the string only while `byte_offset` is zero: every character
consumed moves that end one character closer to the start. A character
reaching past the moved end is measured as truncated, which `mrb_utf8len()`
reports as a single byte, so the conversion answers an offset that lands
inside a character:

```ruby
"あいうえお".slice!(3, 2)  #=> "\xE3\x81"  (expected "えお")
"あいう".slice!(1..2)      #=> "い\xE3"    (expected "いう")
"あいう".slice!(-1)        #=> "\xE3"      (expected "う")
```

The bytes the receiver keeps are the ones the result did not take, so the
receiver is broken as well: the first line above leaves it holding
`"あいう\x88お"`.

`mrb_str_char_to_byte()` does this conversion in core and is what
`String#[]` goes through, which is why `"あいうえお"[3, 2]` answers `"えお"`
where `slice!` does not. Call it for both offsets and drop the two static
helpers. It is declared outside the `MRB_UTF8_STRING` guard and is the
identity on a build without UTF-8 support, so the guarded branch goes with
them.
`String#slice!(str)` searches with `mrb_str_index()`, which answers a byte
offset, and turned that into a character index by counting the characters
of `mrb_str_substr(mrb, self, 0, pos)`. `mrb_str_substr()` reads its
arguments as character positions, so the byte offset arrived there as a
character count and the substring it cut is longer than the part before the
match. The index that comes out is too large:

```ruby
"あいう".slice!("い")  #=> ""  (expected "い")
```

`"い"` starts at byte 3, `mrb_str_substr()` takes the 3 as three characters
and hands back the whole receiver, and the match is reported at character
3 rather than 1. The length is then clamped against the receiver, which
leaves nothing to cut.

`mrb_str_byte_to_char()` is the conversion this wants, and it also settles
what to do when the offset is not the start of a character: the search runs
over bytes and can land inside one, which it reports as -1. CRuby finds no
match in that position, so answer nil for it.
`utf8_strlen()` counts the characters of a string the way the string
itself is indexed: a binary or single-byte string has one character per
byte, everything else is measured as UTF-8. It belongs with
`mrb_str_char_to_byte()` and `mrb_str_byte_to_char()`, which answer
positions under that same rule, but unlike them it was static and
reachable only through the `RSTRING_CHAR_LEN` macro. mruby-string-ext
carries its own copy of it for want of a shared one.

Give the count the shape the two conversions already have:
`mrb_str_char_len(mrb, str)`, declared in `internal.h` outside the
`MRB_UTF8_STRING` guard, with a definition on the other side of the
guard that returns the byte length. Both `RSTRING_CHAR_LEN` definitions
go away and the six uses call the function.

The `mrb` argument goes unused here as it does in the two conversions.
It is in the signature so the three read alike where they are called.
`str_char_count()` is `mrb_str_char_len()` written a second time: a
binary or single-byte string counts one character per byte, anything
else goes through `mrb_utf8_strlen()`. Call the core helper and drop
the copy.

The helper is declared outside the `MRB_UTF8_STRING` guard and returns
the byte length on a build without UTF-8 support, so `String#slice!`
loses the guarded branch it chose its length with. `ljust`, `rjust` and
`center` were already calling the copy from outside any guard.

The core helper also keeps what it learns: a string whose byte length
and character count agree is marked single-byte, so the next question
about it is answered without walking the bytes again. The copy threw
that away.
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 91f0a92d-c93a-4a5f-a473-610542e40092

📥 Commits

Reviewing files that changed from the base of the PR and between b9f2400 and 8f169ce.

📒 Files selected for processing (4)
  • include/mruby/internal.h
  • mrbgems/mruby-string-ext/src/string.c
  • mrbgems/mruby-string-ext/test/string.rb
  • src/string.c

📝 Walkthrough

Walkthrough

The PR adds the internal mrb_str_char_len API and replaces local character-counting logic. Core string operations and string extensions now use shared character and byte conversion functions. UTF-8 String#slice! behavior gains focused tests.

Changes

String character length handling

Layer / File(s) Summary
Character-length API contract and implementations
include/mruby/internal.h, src/string.c
The internal API documents character and byte counting and implements mrb_str_char_len for UTF-8, binary, and single-byte strings.
Core string operation migration
src/string.c
String sizing, boundary checks, range handling, assignment, indexing, and reversal use mrb_str_char_len.
String extension slicing and justification
mrbgems/mruby-string-ext/src/string.c, mrbgems/mruby-string-ext/test/string.rb
Justification and centering use the shared API. String#slice! uses shared conversions and adds UTF-8 coverage for slicing and matching boundaries.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • mruby/mruby#7081: Directly precedes the centralized binary-string character-length handling.
  • mruby/mruby#7103: Covers the same String#slice! multibyte conversion behavior and tests.
  • mruby/mruby#7098: Modifies related UTF-8 character and byte index conversion behavior.

Suggested reviewers: matz

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@matz
matz merged commit 15e2deb into mruby:master Aug 12, 2026
19 of 21 checks passed
@takumin
takumin deleted the core-str-char-len branch August 12, 2026 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants