string.c: write MRB_STR_SINGLE_BYTE through the accessors it has - #7156

#7150 gave MRB_STR_BINARY the two accessors it was missing and listed what it deliberately left behind. The first item on that list:

mrbgems/mruby-string-ext/src/string.c, in str_ascii_only_p(): flags |= MRB_STR_SINGLE_BYTE, where RSTR_SET_SINGLE_BYTE_FLAG() exists but compiles to nothing without MRB_UTF8_STRING, so swapping it in is a behavior question, not a spelling one.

This PR answers that question, and picks up two more writes the grep behind that list could not see.

The three sites

/* mrbgems/mruby-string-ext/src/string.c, str_ascii_only_p() */
mrb_str_ptr(str)->flags |= MRB_STR_SINGLE_BYTE;

/* src/string.c, str_escape() */
mrb_str_ptr(str)->flags |= src_sb_flag;
mrb_str_ptr(result)->flags |= sb_flag;

The grep in #7150 was for flags |= MRB_STR_SINGLE_BYTE, so it found the first and not the other two: those spell the value as a local holding MRB_STR_SINGLE_BYTE or zero, which is the same write with the constant moved a few lines up.

MRB_STR_SINGLE_BYTE is not short of accessors the way MRB_STR_BINARY was. It already has all four, so nothing is added to include/mruby/string.h here; the three sites simply start using what is there.

str_ascii_only_p(): what the behavior question comes to

The site sits outside the #ifdef MRB_UTF8_STRING block that gives the flag its meaning, so the raw write also runs in a build without that define. What it writes there is a bit nothing on a string reads:

• RSTR_SINGLE_BYTE_P() is TRUE for every string in that build, so no reader of the flag consults the bit.
• The one other flag at bit 5, MRB_FL_OBJ_SHAPED, is only ever read or cleared through a struct RObject* (src/variable.c), never on a string.

So the behavior question comes to this: the MRB_UTF8_STRING build keeps writing exactly what it wrote before, and the build without it stops making a write no one can observe. That is the whole of the difference.

The accessor used is RSTR_SET_ASCII_FLAG(), not RSTR_SET_SINGLE_BYTE_FLAG() directly. They are the same macro, but the first is the name this codebase uses at the moment it knows the bytes in hand are ASCII, including twice in this very file (int_chr_binary(), int_chr_utf8()) and in src/object.c, src/symbol.c and mruby-time. str_ascii_only_p() has just walked the bytes and proved precisely that. It also passes the struct RString* it already holds as s, matching the two reads above it, rather than calling mrb_str_ptr(str) a second time.

str_escape(): two locals that carry a bit

The two locals exist so that the walk can record what it saw and the write can happen once at the end. Carrying the flag bit itself is what let that write be an unconditional |=; with the accessor, a plain truth value does the same job:

-  uint32_t sb_flag = MRB_STR_SINGLE_BYTE;      /* what `result` comes out as */
-  uint32_t src_sb_flag = MRB_STR_SINGLE_BYTE;  /* what the walk found `str` to be */
+  mrb_bool sb_flag = TRUE;      /* whether `result` comes out single byte */
+  mrb_bool src_sb_flag = TRUE;  /* whether the walk found `str` single byte */

Both stay under the #ifdef they are already in. Only a build that reads whole characters out of str has an answer to record at all.

Generated code

gcc -S -O3 over both changed files, with MRB_UTF8_STRING and without, so all four combinations:

Both moves shrink. In str_escape() the flags word now takes a constant under a branch instead of having a runtime value inserted into the bitfield; in str_ascii_only_p() the insert disappears with the write. Every differing line falls inside those two functions, and no other function in either file changes.

Testing

MRUBY_CONFIG=ci/gcc-clang rake -m test, all builds green, 0 KO, 0 crash, 0 warnings:

• full-core gembox with MRB_UTF8_STRING: 2286 tests, 2276 OK, 10 skip.
• default gembox, where mruby-encoding is absent and strings index by byte: 2070 tests, 2042 OK, 28 skip.

No test accompanies the change. Neither String#ascii_only? nor String#inspect gives a different answer anywhere, so there is nothing new to pin.

Still left out

The other item on #7150's list, RSTR_SET_EMBED_LEN() written out by hand in mrbgems/mruby-sprintf/src/sprintf.c, is untouched. It is a different flag family, and folding it into RSTR_SET_LEN() next to a branch that sets the heap length is not a rename.

After this change, grep -rn "MRB_STR_SINGLE_BYTE" --include='*.c' src mrbgems finds one line, and it is a comment.