string.c: ask a string what encoding it is, not whether a bit is set - #7157

MRB_STR_BINARY is the whole of what a string says about how its bytes are read: a string carrying it is read by byte, a string without it is read the way the build reads strings by default. The macros around it are named after the bit rather than after the question, so every write names one value of the field instead of the field:

RSTR_SET_BINARY_FLAG(s);
RSTR_UNSET_BINARY_FLAG(s);
RSTR_COPY_BINARY_FLAG(dst, src);

This PR gives the field a name and a small vocabulary, and routes the 22 writes and copies through it. The bit stays where it is; MRB_STR_BINARY is still 16 and the layout of flags does not move.

What goes into include/mruby/string.h

#define MRB_STR_ENCODING_DEFAULT 0
#define MRB_STR_ENCODING_BINARY  1
#ifdef MRB_UTF8_STRING
# define MRB_STR_ENCODING_UTF8   MRB_STR_ENCODING_DEFAULT
#endif

#define RSTR_ENCODING(s) \
  (((s)->flags & MRB_STR_BINARY) ? MRB_STR_ENCODING_BINARY : MRB_STR_ENCODING_DEFAULT)
#define RSTR_ENCODING_SET(s, e) \
  ((s)->flags = ((s)->flags & ~MRB_STR_BINARY) | \
                (((e) == MRB_STR_ENCODING_BINARY) ? MRB_STR_BINARY : 0))
#define RSTR_BINARY_P(s) (RSTR_ENCODING(s) == MRB_STR_ENCODING_BINARY)
#define RSTR_ENC_COPY(dst, src) RSTR_ENCODING_SET(dst, RSTR_ENCODING(src))

RSTR_SET_BINARY_FLAG(), RSTR_UNSET_BINARY_FLAG() and RSTR_COPY_BINARY_FLAG() go.

Index 0 is the build's default

A fresh string is zero filled, so index 0 costs no store on the path every string is made on. That is where the literals the parser hands over end up, and the strings numbers and times spell themselves with. __ENCODING__ is what names index 0 for a given build: UTF-8 where MRB_UTF8_STRING is set, ASCII-8BIT where it is not.

Binary cannot share index 0 with the default even in a build that indexes by byte anyway. String#b and Integer#chr mark strings there too, and mruby-regexp reads the mark through re_binary_string_p() (mrbgems/mruby-regexp/src/regexp.c) to decide whether to walk the subject a byte at a time, so folding the two together would change what that build answers.

MRB_STR_ENCODING_UTF8 is defined only where the build carries UTF-8. A build without it has no UTF-8 string to name, so naming one should stop the compiler rather than quietly write the default. The only place that names it is String#force_encoding, in mruby-encoding, which sets the define itself.

RSTR_BINARY_P() stays

Whether a string is read by byte is a question about the encoding, not about the shape the answer is kept in, so the predicate keeps its name and its 18 readers are untouched. It reads the field now and answers 1 rather than 16. re_binary_string_p() returns it as mrb_bool, which is bool in the usual configurations and uint8_t in the fallback (include/mruby/value.h); 16 and 1 are both true in either, so the narrowing simply happens where the answer is made.

The 22 sites

Each replacement is bit for bit the write it replaces. RSTR_ENCODING_SET(s, MRB_STR_ENCODING_BINARY) expands to flags = (flags & ~16) | 16, and with MRB_STR_ENCODING_UTF8 it expands to flags = (flags & ~16) | 0, since (0 == 1) is always false.

Generated code

Two full-core builds, with mruby-encoding and with the gem removed from the box, gcc -O3:

Comparison is objdump -d over every .o in the build against the same object built from master. The files differ as bytes only in debug info, since the line numbers in include/mruby/string.h moved.

Testing

rake -m test on both builds above, plus the C++ ABI build, all green:

• full-core with MRB_UTF8_STRING: 2291 tests, 2281 OK, 10 skip, 0 KO, 0 crash, 0 warnings.
• full-core without mruby-encoding, where strings index by byte: 2229 tests, 2200 OK, 29 skip, 0 KO, 0 crash, 0 warnings.
• the same box with enable_cxx_abi: 2291 tests, 2281 OK, 10 skip.

No test accompanies the change. No string answers anything different, so there is nothing new to pin.

Rebased on 8bd5bd0

re_byte_substr() in mruby-regexp used to copy the flag by hand and now hands the work to mrb_str_byte_subseq(), so the copy this PR would have converted there is gone. That is the one site the count above lost.

Not in this PR

The bit layout of flags does not change, the 18 RSTR_BINARY_P() readers do not change, and the comments that still say "flag" about this bit stay as they are while it still is one.

Summary by CodeRabbit

• Bug Fixes
  • Improved string encoding consistency across string creation, copying, slicing, formatting, substitutions, and bitwise operations.
  • Improved handling of binary and UTF-8 string encodings.
  • Preserved existing string behavior while ensuring encoding metadata propagates correctly through derived strings.