Mark strings returned by Symbol#to_s as chilled (#12065)

* Use FL_USER0 for ELTS_SHARED

This makes space in RString for two bits for chilled strings.

* Mark strings returned by `Symbol#to_s` as chilled

[Feature #20350]

`STR_CHILLED` now spans on two user flags. If one bit is set it
marks a chilled string literal, if it's the other it marks a
`Symbol#to_s` chilled string.

Since it's not possible, and doesn't make much sense to include
debug info when `--debug-frozen-string-literal` is set, we can't
include allocation source, but we can safely include the symbol
name in the warning message, making it much easier to find the source
of the issue.

Co-Authored-By: Étienne Barrié <etienne.barrie@gmail.com>

---------

Co-authored-by: Étienne Barrié <etienne.barrie@gmail.com>
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
This commit is contained in:
Jean byroot Boussier 2024-11-13 15:20:00 +01:00 коммит произвёл GitHub
Родитель 37a16c7812
Коммит 6deeec5d45
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
15 изменённых файлов: 189 добавлений и 93 удалений

Просмотреть файл

@ -42,12 +42,12 @@ VALUE rb_cArray_empty_frozen;
/* Flags of RArray
*
* 0: RARRAY_SHARED_FLAG (equal to ELTS_SHARED)
* The array is shared. The buffer this array points to is owned by
* another array (the shared root).
* 1: RARRAY_EMBED_FLAG
* The array is embedded (its contents follow the header, rather than
* being on a separately allocated buffer).
* 2: RARRAY_SHARED_FLAG (equal to ELTS_SHARED)
* The array is shared. The buffer this array points to is owned by
* another array (the shared root).
* 3-9: RARRAY_EMBED_LEN
* The length of the array when RARRAY_EMBED_FLAG is set.
* 12: RARRAY_SHARED_ROOT_FLAG

Просмотреть файл

@ -8950,7 +8950,7 @@ compile_builtin_attr(rb_iseq_t *iseq, const NODE *node)
if (!SYMBOL_P(symbol)) goto non_symbol_arg;
string = rb_sym_to_s(symbol);
string = rb_sym2str(symbol);
if (strcmp(RSTRING_PTR(string), "leaf") == 0) {
ISEQ_BODY(iseq)->builtin_attrs |= BUILTIN_ATTR_LEAF;
}

11
error.c
Просмотреть файл

@ -4016,7 +4016,7 @@ rb_error_frozen_object(VALUE frozen_obj)
}
void
rb_warn_unchilled(VALUE obj)
rb_warn_unchilled_literal(VALUE obj)
{
rb_warning_category_t category = RB_WARN_CATEGORY_DEPRECATED;
if (!NIL_P(ruby_verbose) && rb_warning_category_enabled_p(category)) {
@ -4047,6 +4047,15 @@ rb_warn_unchilled(VALUE obj)
}
}
void
rb_warn_unchilled_symbol_to_s(VALUE obj)
{
rb_category_warn(
RB_WARN_CATEGORY_DEPRECATED,
"warning: string returned by :%s.to_s will be frozen in the future", RSTRING_PTR(obj)
);
}
#undef rb_check_frozen
void
rb_check_frozen(VALUE obj)

Просмотреть файл

@ -24,7 +24,7 @@
* In released versions of Ruby, this number is not defined since teeny
* versions of Ruby should guarantee ABI compatibility.
*/
#define RUBY_ABI_VERSION 0
#define RUBY_ABI_VERSION 1
/* Windows does not support weak symbols so ruby_abi_version will not exist
* in the shared library. */

Просмотреть файл

@ -80,6 +80,8 @@
* here is at least incomplete.
*/
enum ruby_rarray_flags {
/* RUBY_FL_USER0 is for ELTS_SHARED */
/**
* This flag has something to do with memory footprint. If the array is
* "small" enough, ruby tries to be creative to abuse padding bits of
@ -99,8 +101,6 @@ enum ruby_rarray_flags {
*/
RARRAY_EMBED_FLAG = RUBY_FL_USER1,
/* RUBY_FL_USER2 is for ELTS_SHARED */
/**
* When an array employs embedded strategy (see ::RARRAY_EMBED_FLAG), these
* bits are used to store the number of elements actually filled into

Просмотреть файл

@ -370,7 +370,7 @@ ruby_fl_type {
* 3rd parties. It must be an implementation detail that they should never
* know. Might better be hidden.
*/
RUBY_ELTS_SHARED = RUBY_FL_USER2,
RUBY_ELTS_SHARED = RUBY_FL_USER0,
/**
* This flag has something to do with an object's class. There are kind of

Просмотреть файл

@ -258,7 +258,7 @@ rb_check_frozen_inline(VALUE obj)
/* ref: internal CHILLED_STRING_P()
This is an implementation detail subject to change. */
if (RB_UNLIKELY(RB_TYPE_P(obj, T_STRING) && FL_TEST_RAW(obj, RUBY_FL_USER3))) {
if (RB_UNLIKELY(RB_TYPE_P(obj, T_STRING) && FL_TEST_RAW(obj, RUBY_FL_USER2 | RUBY_FL_USER3))) { // STR_CHILLED
rb_str_modify(obj);
}
}

Просмотреть файл

@ -15,9 +15,11 @@
#include "ruby/encoding.h" /* for rb_encoding */
#include "ruby/ruby.h" /* for VALUE */
#define STR_NOEMBED FL_USER1
#define STR_SHARED FL_USER2 /* = ELTS_SHARED */
#define STR_CHILLED FL_USER3
#define STR_SHARED FL_USER0 /* = ELTS_SHARED */
#define STR_NOEMBED FL_USER1
#define STR_CHILLED (FL_USER2 | FL_USER3)
#define STR_CHILLED_LITERAL FL_USER2
#define STR_CHILLED_SYMBOL_TO_S FL_USER3
enum ruby_rstring_private_flags {
RSTRING_CHILLED = STR_CHILLED,
@ -60,7 +62,8 @@ VALUE rb_str_upto_endless_each(VALUE, int (*each)(VALUE, VALUE), VALUE);
VALUE rb_str_with_debug_created_info(VALUE, VALUE, int);
/* error.c */
void rb_warn_unchilled(VALUE str);
void rb_warn_unchilled_literal(VALUE str);
void rb_warn_unchilled_symbol_to_s(VALUE str);
static inline bool STR_EMBED_P(VALUE str);
static inline bool STR_SHARED_P(VALUE str);
@ -127,14 +130,18 @@ CHILLED_STRING_P(VALUE obj)
static inline void
CHILLED_STRING_MUTATED(VALUE str)
{
VALUE chilled_reason = RB_FL_TEST_RAW(str, STR_CHILLED);
FL_UNSET_RAW(str, STR_CHILLED);
rb_warn_unchilled(str);
}
static inline void
STR_CHILL_RAW(VALUE str)
{
FL_SET_RAW(str, STR_CHILLED);
switch (chilled_reason) {
case STR_CHILLED_SYMBOL_TO_S:
rb_warn_unchilled_symbol_to_s(str);
break;
case STR_CHILLED_LITERAL:
rb_warn_unchilled_literal(str);
break;
default:
rb_bug("RString was chilled for multiple reasons");
}
}
static inline bool

Просмотреть файл

@ -31,8 +31,8 @@ module RubyVM::RJIT
def self.new(fiddle_type)
name = Fiddle.constants.find do |const|
const.start_with?('TYPE_') && Fiddle.const_get(const) == fiddle_type.abs
end&.to_s
name.delete_prefix!('TYPE_')
end&.name
name = name.delete_prefix('TYPE_')
if fiddle_type.negative?
name.prepend('U')
end

Просмотреть файл

@ -500,10 +500,12 @@ rb_obj_clone_setup(VALUE obj, VALUE clone, VALUE kwfreeze)
case Qnil:
rb_funcall(clone, id_init_clone, 1, obj);
RBASIC(clone)->flags |= RBASIC(obj)->flags & FL_FREEZE;
if (CHILLED_STRING_P(obj)) {
STR_CHILL_RAW(clone);
if (RB_TYPE_P(obj, T_STRING)) {
FL_SET_RAW(clone, FL_TEST_RAW(obj, STR_CHILLED));
}
else if (RB_OBJ_FROZEN(obj)) {
if (RB_OBJ_FROZEN(obj)) {
rb_shape_t * next_shape = rb_shape_transition_shape_frozen(clone);
if (!rb_shape_obj_too_complex(clone) && next_shape->type == SHAPE_OBJ_TOO_COMPLEX) {
rb_evict_ivars_to_hash(clone);

Просмотреть файл

@ -3382,7 +3382,7 @@ pm_compile_builtin_attr(rb_iseq_t *iseq, const pm_scope_node_t *scope_node, cons
}
VALUE symbol = pm_static_literal_value(iseq, argument, scope_node);
VALUE string = rb_sym_to_s(symbol);
VALUE string = rb_sym2str(symbol);
if (strcmp(RSTRING_PTR(string), "leaf") == 0) {
ISEQ_BODY(iseq)->builtin_attrs |= BUILTIN_ATTR_LEAF;

Просмотреть файл

@ -2,69 +2,141 @@ require_relative '../../spec_helper'
describe "chilled String" do
guard -> { ruby_version_is "3.4" and !"test".equal?("test") } do
describe "#frozen?" do
it "returns false" do
"chilled".frozen?.should == false
end
end
describe "chilled string literals" do
describe "#-@" do
it "returns a different instance" do
input = "chilled"
interned = (-input)
interned.frozen?.should == true
interned.object_id.should_not == input.object_id
end
end
describe "#+@" do
it "returns a different instance" do
input = "chilled"
duped = (+input)
duped.frozen?.should == false
duped.object_id.should_not == input.object_id
end
end
describe "#clone" do
it "preserves chilled status" do
input = "chilled".clone
-> {
input << "-mutated"
}.should complain(/literal string will be frozen in the future/)
input.should == "chilled-mutated"
end
end
describe "mutation" do
it "emits a warning" do
input = "chilled"
-> {
input << "-mutated"
}.should complain(/literal string will be frozen in the future/)
input.should == "chilled-mutated"
describe "#frozen?" do
it "returns false" do
"chilled".frozen?.should == false
end
end
it "emits a warning on singleton_class creation" do
-> {
"chilled".singleton_class
}.should complain(/literal string will be frozen in the future/)
describe "#-@" do
it "returns a different instance" do
input = "chilled"
interned = (-input)
interned.frozen?.should == true
interned.object_id.should_not == input.object_id
end
end
it "emits a warning on instance variable assignment" do
-> {
"chilled".instance_variable_set(:@ivar, 42)
}.should complain(/literal string will be frozen in the future/)
describe "#+@" do
it "returns a different instance" do
input = "chilled"
duped = (+input)
duped.frozen?.should == false
duped.object_id.should_not == input.object_id
end
end
it "raises FrozenError after the string was explicitly frozen" do
input = "chilled"
input.freeze
-> {
describe "#clone" do
it "preserves chilled status" do
input = "chilled".clone
-> {
input << "mutated"
}.should raise_error(FrozenError)
}.should_not complain(/literal string will be frozen in the future/)
input << "-mutated"
}.should complain(/literal string will be frozen in the future/)
input.should == "chilled-mutated"
end
end
describe "mutation" do
it "emits a warning" do
input = "chilled"
-> {
input << "-mutated"
}.should complain(/literal string will be frozen in the future/)
input.should == "chilled-mutated"
end
it "emits a warning on singleton_class creation" do
-> {
"chilled".singleton_class
}.should complain(/literal string will be frozen in the future/)
end
it "emits a warning on instance variable assignment" do
-> {
"chilled".instance_variable_set(:@ivar, 42)
}.should complain(/literal string will be frozen in the future/)
end
it "raises FrozenError after the string was explicitly frozen" do
input = "chilled"
input.freeze
-> {
-> {
input << "mutated"
}.should raise_error(FrozenError)
}.should_not complain(/literal string will be frozen in the future/)
end
end
end
describe "chilled strings returned by Symbol#to_s" do
describe "#frozen?" do
it "returns false" do
:chilled.to_s.frozen?.should == false
end
end
describe "#-@" do
it "returns a different instance" do
input = :chilled.to_s
interned = (-input)
interned.frozen?.should == true
interned.object_id.should_not == input.object_id
end
end
describe "#+@" do
it "returns a different instance" do
input = :chilled.to_s
duped = (+input)
duped.frozen?.should == false
duped.object_id.should_not == input.object_id
end
end
describe "#clone" do
it "preserves chilled status" do
input = :chilled.to_s.clone
-> {
input << "-mutated"
}.should complain(/string returned by :chilled\.to_s will be frozen in the future/)
input.should == "chilled-mutated"
end
end
describe "mutation" do
it "emits a warning" do
input = :chilled.to_s
-> {
input << "-mutated"
}.should complain(/string returned by :chilled\.to_s will be frozen in the future/)
input.should == "chilled-mutated"
end
it "emits a warning on singleton_class creation" do
-> {
:chilled.to_s.singleton_class
}.should complain(/string returned by :chilled\.to_s will be frozen in the future/)
end
it "emits a warning on instance variable assignment" do
-> {
:chilled.to_s.instance_variable_set(:@ivar, 42)
}.should complain(/string returned by :chilled\.to_s will be frozen in the future/)
end
it "raises FrozenError after the string was explicitly frozen" do
input = :chilled.to_s
input.freeze
-> {
-> {
input << "mutated"
}.should raise_error(FrozenError)
}.should_not complain(/string returned by :chilled\.to_s will be frozen in the future/)
end
end
end
end

Просмотреть файл

@ -80,15 +80,19 @@ VALUE rb_cSymbol;
/* Flags of RString
*
* 0: STR_SHARED (equal to ELTS_SHARED)
* The string is shared. The buffer this string points to is owned by
* another string (the shared root).
* 1: RSTRING_NOEMBED
* The string is not embedded. When a string is embedded, the contents
* follow the header. When a string is not embedded, the contents is
* on a separately allocated buffer.
* 2: STR_SHARED (equal to ELTS_SHARED)
* The string is shared. The buffer this string points to is owned by
* another string (the shared root).
* 3: STR_CHILLED (will be frozen in a future version)
* The string appears frozen but can be mutated with a warning.
* 2: STR_CHILLED_LITERAL (will be frozen in a future version)
* The string was allocated as a literal in a file without an explicit `frozen_string_literal` comment.
* It emits a deprecation warning when mutated for the first time.
* 3: STR_CHILLED_SYMBOL_TO_S (will be frozen in a future version)
* The string was allocated by the `Symbol#to_s` method.
* It emits a deprecation warning when mutated for the first time.
* 4: STR_PRECOMPUTED_HASH
* The string is embedded and has its precomputed hascode stored
* after the terminator.
@ -1959,7 +1963,7 @@ rb_ec_str_resurrect(struct rb_execution_context_struct *ec, VALUE str, bool chil
str_duplicate_setup_heap(klass, str, new_str);
}
if (chilled) {
STR_CHILL_RAW(new_str);
FL_SET_RAW(new_str, STR_CHILLED_LITERAL);
}
return new_str;
}
@ -1970,7 +1974,7 @@ rb_str_with_debug_created_info(VALUE str, VALUE path, int line)
VALUE debug_info = rb_ary_new_from_args(2, path, INT2FIX(line));
if (OBJ_FROZEN_RAW(str)) str = rb_str_dup(str);
rb_ivar_set(str, id_debug_created_info, rb_ary_freeze(debug_info));
STR_CHILL_RAW(str);
FL_SET_RAW(str, STR_CHILLED_LITERAL);
return rb_str_freeze(str);
}
@ -12142,7 +12146,9 @@ sym_inspect(VALUE sym)
VALUE
rb_sym_to_s(VALUE sym)
{
return str_new_shared(rb_cString, rb_sym2str(sym));
VALUE str = str_new_shared(rb_cString, rb_sym2str(sym));
FL_SET_RAW(str, STR_CHILLED_SYMBOL_TO_S);
return str;
}
VALUE

Просмотреть файл

@ -1814,7 +1814,7 @@ void rb_obj_freeze_inline(VALUE x)
if (RB_FL_ABLE(x)) {
RB_FL_SET_RAW(x, RUBY_FL_FREEZE);
if (TYPE(x) == T_STRING) {
RB_FL_UNSET_RAW(x, FL_USER3); // STR_CHILLED
RB_FL_UNSET_RAW(x, FL_USER2 | FL_USER3); // STR_CHILLED
}
rb_shape_t * next_shape = rb_shape_transition_shape_frozen(x);

Просмотреть файл

@ -251,7 +251,7 @@ pub const RUBY_FL_USER16: ruby_fl_type = 268435456;
pub const RUBY_FL_USER17: ruby_fl_type = 536870912;
pub const RUBY_FL_USER18: ruby_fl_type = 1073741824;
pub const RUBY_FL_USER19: ruby_fl_type = -2147483648;
pub const RUBY_ELTS_SHARED: ruby_fl_type = 16384;
pub const RUBY_ELTS_SHARED: ruby_fl_type = 4096;
pub const RUBY_FL_SINGLETON: ruby_fl_type = 8192;
pub type ruby_fl_type = i32;
pub const RSTRING_NOEMBED: ruby_rstring_flags = 8192;
@ -705,7 +705,7 @@ pub struct rb_call_data {
pub ci: *const rb_callinfo,
pub cc: *const rb_callcache,
}
pub const RSTRING_CHILLED: ruby_rstring_private_flags = 32768;
pub const RSTRING_CHILLED: ruby_rstring_private_flags = 49152;
pub type ruby_rstring_private_flags = u32;
pub const RHASH_PASS_AS_KEYWORDS: ruby_rhash_flags = 8192;
pub const RHASH_PROC_DEFAULT: ruby_rhash_flags = 16384;