I was working on issue related to code in rb_fstring(https://github.com/ruby/ruby/pull/2233) and saw some weird behavior in the function,
freezing the given string if it's not a "bare" string and it's small enough to be embedded.
The issue comes up in the following edge case:
class MyString < String
end
non_frozen = MyString.new("nonfrozen")
frozen = -non_frozen # deduplicates, but shouldn't freeze receiver
non_frozen << " added" # raises FrozenError
I'm not sure what the correct behavior should be with a subclass and String#uminus. Should it return
a frozen regular String or a frozen copy of the given class's string?
Not a practical concern (not often come upon I'm sure), but I think a valid one.
I agree that this is a bug. I'm not sure if rb_fstring's behavior should be changed (hopefully a more knowledgeable committer can weigh in), so a conservative approach to fix this is to dup the receiver before passing to rb_fstring, if the receiver's class is not String and the object is not already frozen. The attached patch does this.
Hi Jeremy, thanks for the patch. I agree with the strategy of just dealing with this in str_uminus directly instead of fiddling around in the tricky rb_fstring function. However there's still an issue when the string is a regular String, but tainted or has ivars. In this case, it still gets frozen. I think the check should be something like:
if (!BARE_STRING_P(str) && !rb_obj_frozen_p(str)) {
str = rb_str_dup(str);
}
The documentation of str_uminus says the receiver will be deduplicated and returned unless tainted or has ivars, which is all well and good, but IMO it should never freeze the receiver under any circumstance. If it can't be deduplicated, it should return a frozen copy.
Hi Jeremy, thanks for the patch. I agree with the strategy of just dealing with this in str_uminus directly instead of fiddling around in the tricky rb_fstring function. However there's still an issue when the string is a regular String, but tainted or has ivars. In this case, it still gets frozen. I think the check should be something like:
if (!BARE_STRING_P(str) && !rb_obj_frozen_p(str)) {
str = rb_str_dup(str);
}
I agree, that is a better way to go. Attached is a new patch that does that and adds a couple more tests.
Make String#-@ not freeze receiver if called on unfrozen subclass instance
rb_fstring behavior in this case is to freeze the receiver. I'm
not sure if that should be changed, so this takes the conservative
approach of duping the receiver in String#-@ before passing
to rb_fstring.
Updated by 
Updated by 
Updated by jeremyevans (Jeremy Evans)