string.h: name what makes a character index a byte index - #7178

Ten places ask the same question of a string and spell it out each time:

if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s)) {

The two halves arrive from different sides, but they say one thing to every reader of them: a character index into this string is already a byte index. mrb_str_char_to_byte() returns the index it was handed, mrb_str_byte_to_char() likewise, mrb_str_check_byte_pos() has no boundary left to check, and mrb_str_rindex_m() hands the whole search to mrb_str_byterindex_m().

This PR gives that question a name.

What goes into include/mruby/string.h

#define RSTR_SINGLE_BYTE_P(s) \
  (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s))

It sits next to RSTR_BINARY_P, which is the other predicate derived from a field rather than read straight out of one.

This is not MRB_STR_SINGLE_BYTE coming back. That was a stored flag, one bit a writer had to set and keep true; this is derived from what the string already carries, so nothing stores it and nothing can leave it stale. RSTR_CODERANGE() folds to MRB_STR_CODERANGE_7BIT on a build without MRB_UTF8_STRING, where every byte is a character, so the predicate folds to a constant there as it did before.

The call sites are all of them

Ten on master, and no eleventh:

$ git grep -nE 'CODERANGE_7BIT.*RSTR_BINARY_P|RSTR_BINARY_P.*CODERANGE_7BIT'
mrbgems/mruby-string-ext/src/string.c:1036:  if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s)) {
mrbgems/mruby-string-ext/src/string.c:1126:  if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s)) {
mrbgems/mruby-string-ext/src/string.c:1169:  if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s)) {
mrbgems/mruby-string-ext/src/string.c:1204:  if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s)) {
mrbgems/mruby-string-ext/src/string.c:1802:  if (RSTR_CODERANGE(s) != MRB_STR_CODERANGE_7BIT && !RSTR_BINARY_P(s)) {
src/string.c:731:  if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s)) {
src/string.c:772:  if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s)) {
src/string.c:2128:    if (!RSTR_BINARY_P(s) && RSTR_CODERANGE(s) != MRB_STR_CODERANGE_7BIT &&
src/string.c:2440:  if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s)) return;
src/string.c:2794:  if (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s)) {

and after, one definition and the same ten:

$ git grep -c RSTR_SINGLE_BYTE_P
include/mruby/string.h:1
mrbgems/mruby-string-ext/src/string.c:5
src/string.c:5

$ git grep -nE 'CODERANGE_7BIT.*RSTR_BINARY_P|RSTR_BINARY_P.*CODERANGE_7BIT'
include/mruby/string.h:189:  (RSTR_CODERANGE(s) == MRB_STR_CODERANGE_7BIT || RSTR_BINARY_P(s))

Eight were the positive form above. Two were the negation, and one of those spells the halves in the other order:

/* src/string.c, mrb_str_chomp_bang() */
-    if (!RSTR_BINARY_P(s) && RSTR_CODERANGE(s) != MRB_STR_CODERANGE_7BIT &&
-        mrb_utf8_char_head(p, pp, p + len) != pp) {
+    if (!RSTR_SINGLE_BYTE_P(s) && mrb_utf8_char_head(p, pp, p + len) != pp) {

/* mrbgems/mruby-string-ext/src/string.c, str_chars_ary() */
-  if (RSTR_CODERANGE(s) != MRB_STR_CODERANGE_7BIT && !RSTR_BINARY_P(s)) {
+  if (!RSTR_SINGLE_BYTE_P(s)) {

str_chars_ary() had !A && !B with the halves in the macro's order, so !(A || B) is what it spelled. mrb_str_chomp_bang() had !B && !A, and that is the one place where the macro asks for something other than what the source said.

The one thing that changes in the object code

Nine call sites expand to the instructions they had. mrb_str_chomp_bang() tests the two halves in the other order, master above and this branch below, from bintest:

    5986:  mov    %r14d,%eax
    5989:  and    $0x1800,%eax        <- the encoding index, for RSTR_BINARY_P
    598e:  cmp    $0x800,%eax
    5993:  je     59c8 <mrb_str_chomp_bang+0x268>
    5995:  mov    %r14d,%eax
    5998:  and    $0x600,%eax         <- the coderange
    599d:  cmp    $0x200,%eax
    59a2:  je     59c8 <mrb_str_chomp_bang+0x268>

    5986:  mov    %r14d,%eax
    5989:  and    $0x600,%eax         <- the coderange first
    598e:  cmp    $0x200,%eax
    5993:  je     59c8 <mrb_str_chomp_bang+0x268>
    5995:  mov    %r14d,%eax
    5998:  and    $0x1800,%eax
    599d:  cmp    $0x800,%eax
    59a2:  je     59c8 <mrb_str_chomp_bang+0x268>

Same instructions, same count, the two tests swapped. Both are pure reads of one word already in a register, so neither order can raise, and the second test is reached only when the first says no in either order. That is the whole of the difference between master and this branch in the object code, over every build measured below.

Generated code

gcc 13.3.0 -O3, x86-64, against the same objects built from master. .text of bin/mruby:

Objects differing, comparing objdump -d over every .o in the build:

The one object is src/string.o, and the swap above is what differs in it. A build that indexes by byte carries no coderange, so RSTR_SINGLE_BYTE_P() folds to true there and both orders fold with it: byte-string and default.rb come out identical to master, object by object.

Testing

rake -m test, all green, 0 KO, 0 crash, 0 warnings, and the same counts as master:

build_config/host-m32.rb needs a multilib toolchain, so the 32-bit build above is full-core with the compiler set to i686-linux-gnu-gcc instead.

No test accompanies the change. Nine call sites are the expression they were and the tenth is the same two tests in the other order, so no string answers anything different and there is nothing new to pin.

Not in this PR

The macro is put where the ten call sites already asked the question, and nowhere else: no path that walks a string gains a shortcut it did not have. The write side of these two fields is untouched, and so is what either field means.

Summary by CodeRabbit

• Refactor
  • Introduced a shared predicate for "one character per byte" and routed the ten places that spelled it out through it, across string indexing, reverse searching, line trimming, character enumeration, and scrubbing.