string.h: drop mrb_str_modify_keep_ascii() - #7181

Stacked on #7180, which is the first commit here. The second commit is this PR's own change, and the diff to read is git diff 7e12663cf..HEAD. Merging #7180 first leaves this one a single commit.

This removes a published function, mrb_str_modify_keep_ascii().

With #7180 in, it has no caller left. What it offered was a prepare that keeps a string standing at 7BIT, and the promise a caller had to make to use it was that its write puts ASCII where ASCII stood. Every write that keeps that promise also keeps the wider one str_modify_keep_cr() wants, since an ASCII byte spells a character of its own in UTF-8. So the answer it gives is the same answer or a poorer one, and there is nothing it can be asked that the other cannot.

Nothing calls it

On master it has five callers among the bang methods and one in mrb_str_modify():

$ git grep -n modify_keep 406205fa0
include/mruby/string.h:225:MRB_API void mrb_str_modify_keep_ascii(mrb_state *mrb, struct RString *s);
mrbgems/mruby-encoding/test/string.rb:50:  # own rather than through mrb_str_modify_keep_ascii(). Nothing else here
src/string.c:1257:mrb_str_modify_keep_ascii(mrb_state *mrb, struct RString *s)
src/string.c:1273: * Prepares a string for modification. Similar to `mrb_str_modify_keep_ascii`,
src/string.c:1281:  mrb_str_modify_keep_ascii(mrb, s);
src/string.c:2019:  mrb_str_modify_keep_ascii(mrb, s);
src/string.c:2072:  mrb_str_modify_keep_ascii(mrb, s);
src/string.c:2180:  mrb_str_modify_keep_ascii(mrb, s);
src/string.c:2252:  mrb_str_modify_keep_ascii(mrb, s);
src/string.c:3192:   * Even after str_modify_keep_ascii(), NULL termination is not ensured if
src/string.c:3431:  mrb_str_modify_keep_ascii(mrb, s);

With #7180 in, the five are gone and what is left is the declaration, the definition, two comments naming it, and the one call inside mrb_str_modify():

$ git grep -n modify_keep 7e12663cf
include/mruby/string.h:225:MRB_API void mrb_str_modify_keep_ascii(mrb_state *mrb, struct RString *s);
mrbgems/mruby-encoding/test/string.rb:50:  # own rather than through mrb_str_modify_keep_ascii(). Nothing else here
src/string.c:1257:mrb_str_modify_keep_ascii(mrb_state *mrb, struct RString *s)
src/string.c:1273: * Prepares a string for modification. Similar to `mrb_str_modify_keep_ascii`,
src/string.c:1281:  mrb_str_modify_keep_ascii(mrb, s);

and after this PR the name is nowhere:

$ git grep -n modify_keep_ascii
$

The two comments and the test comment go with it. That grep is over the whole tree, doc/ included, so nothing published names it either.

Why the two collapse into one

While 7BIT, VALID and BROKEN were three flags, keeping one of them and dropping the rest was a distinct thing to offer, and mrb_str_modify_keep_ascii() offered it:

/* before the coderange became a field */
MRB_API void
mrb_str_modify_keep_ascii(mrb_state *mrb, struct RString *s)
{
  mrb_check_frozen(mrb, s);
  str_unshare_buffer(mrb, s);
  RSTR_UNSET_VALID_ENC_FLAG(s);
  RSTR_UNSET_BROKEN_ENC_FLAG(s);
}

With one field holding one of four answers, a prepare either keeps what the string says or takes it back to MRB_STR_CODERANGE_UNKNOWN. There is no third thing left for a second published prepare to do.

What is left

MRB_API void mrb_str_modify(mrb_state *mrb, struct RString *s);

a prepare that asks nothing of its caller, and str_modify_keep_cr() in string.c, which asks a promise no caller can be held to. This is the pair CRuby publishes and keeps back: rb_str_modify() in include/ruby/intern.h, str_modify_keep_cr() static in string.c.

mrb_str_modify() takes the two lines of prologue back rather than calling through the removed one:

MRB_API void
mrb_str_modify(mrb_state *mrb, struct RString *s)
{
  mrb_check_frozen(mrb, s);
  str_unshare_buffer(mrb, s);
  RSTR_CODERANGE_SET(s, MRB_STR_CODERANGE_UNKNOWN);
}

For a gem that called it

mrb_str_modify(). It is correct, and it pays for a walk the gem was promising to skip. The build says so rather than changing under it.

Generated code

gcc 13.3.0 -O3, x86-64, against the same objects built from master. .text of bin/mruby, with #7180 in the middle column so the two commits can be read apart:

Objects differing between this commit and #7180, comparing objdump -d over every .o in the build:

src/string.o and nothing else, in every build.

Where the strings index by byte, the removed function is the whole of it. Per symbol, byte-string:

mrb_str_modify_keep_ascii    838 -> gone

A published function whose body folds to mrb_check_frozen() and an unshare goes, and .text falls by 952 with the padding around it.

Where they index by character it is smaller and goes the other way, because mrb_str_modify() stops calling out. On master and in #7180 it is a call to mrb_str_modify_keep_ascii() followed by one field write, so a caller that inlines mrb_str_modify() still ends at that call. Here there is nothing to call, and str_unshare_buffer() comes along. mrb_str_resize() in bintest, #7180 above and this branch below, by the relocations of what it calls:

mrb_str_modify_keep_ascii

mrb_check_frozen   mrb_malloc x2   mrb_free x2   memcpy x2

which is where the +849 in the per-symbol account comes from:

mrb_str_modify_keep_ascii    942 -> gone
mrb_str_resize               208 -> 1057   (+849)
mrb_str_concat               442 ->  758   (+316)
mrb_str_cat                 1057 -> 1065     (+8)
mrb_str_aset_m              1496 -> 1480    (-16)
mrb_str_reverse_bang         418 ->  368    (-50)
mrb_str_append               574 ->  542    (-32)
mrb_str_cat_str              574 ->  542    (-32)
mrb_str_setbyte              294 ->  262    (-32)
str_bytesplice               807 ->  775    (-32)

+37 over the symbols, +56 in .text. full-debug is where gcc inlines least, and there the removal is nearly all of it:

mrb_str_modify_keep_ascii    123 -> gone
mrb_str_modify                82 ->  101    (+19)

Testing

rake -m test, on each commit, 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 comes with this. A function with no caller is removed and the one line it held is written where its last caller stood, so no string answers anything different. The one test file that named it, mrbgems/mruby-encoding/test/string.rb, names mrb_str_modify() in a comment now and tests what it tested.

Not in this PR

mrb_str_modify() does what it did: the two lines it reached for through the removed function are written where the call stood, and what it clears and when are unchanged. str_modify_keep_cr() gains no caller here either, so which other in-place writes could make its promise stays where #7180 left it. No other published string function is touched.

Summary by CodeRabbit

• API Changes
  • Removed the public mrb_str_modify_keep_ascii(), which string.c: keep what a string reads as across a write that cannot change it #7180 left without a caller. mrb_str_modify() remains and is what a caller of the removed one wants.