diff --git a/array.c b/array.c index 3209712e61..ed7bc6ddb0 100644 --- a/array.c +++ b/array.c @@ -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 diff --git a/compile.c b/compile.c index e777fba89a..10d02b2447 100644 --- a/compile.c +++ b/compile.c @@ -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; } diff --git a/error.c b/error.c index 3fc1d96d6c..384a56f5df 100644 --- a/error.c +++ b/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) diff --git a/include/ruby/internal/abi.h b/include/ruby/internal/abi.h index e735a67564..e6d1fa7e8f 100644 --- a/include/ruby/internal/abi.h +++ b/include/ruby/internal/abi.h @@ -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. */ diff --git a/include/ruby/internal/core/rarray.h b/include/ruby/internal/core/rarray.h index 90690fe794..73cc0f5dd9 100644 --- a/include/ruby/internal/core/rarray.h +++ b/include/ruby/internal/core/rarray.h @@ -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 diff --git a/include/ruby/internal/fl_type.h b/include/ruby/internal/fl_type.h index 2a8bdccd46..f80f65ef8f 100644 --- a/include/ruby/internal/fl_type.h +++ b/include/ruby/internal/fl_type.h @@ -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 diff --git a/include/ruby/internal/intern/error.h b/include/ruby/internal/intern/error.h index b1e2c130b9..1fd9ec2f51 100644 --- a/include/ruby/internal/intern/error.h +++ b/include/ruby/internal/intern/error.h @@ -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); } } diff --git a/internal/string.h b/internal/string.h index 81e94ed6b5..efeb0827c9 100644 --- a/internal/string.h +++ b/internal/string.h @@ -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 diff --git a/lib/ruby_vm/rjit/c_type.rb b/lib/ruby_vm/rjit/c_type.rb index 3b313a658b..98011f9f61 100644 --- a/lib/ruby_vm/rjit/c_type.rb +++ b/lib/ruby_vm/rjit/c_type.rb @@ -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 diff --git a/object.c b/object.c index 1bd476b022..ac9fa91867 100644 --- a/object.c +++ b/object.c @@ -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); diff --git a/prism_compile.c b/prism_compile.c index f4a35fa429..fe5fde8d63 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -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; diff --git a/spec/ruby/core/string/chilled_string_spec.rb b/spec/ruby/core/string/chilled_string_spec.rb index 9f81b1af6d..b8fb6eedc9 100644 --- a/spec/ruby/core/string/chilled_string_spec.rb +++ b/spec/ruby/core/string/chilled_string_spec.rb @@ -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 diff --git a/string.c b/string.c index f298d74cfe..f6f67e185c 100644 --- a/string.c +++ b/string.c @@ -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 diff --git a/variable.c b/variable.c index 5e178ef06b..61c188bede 100644 --- a/variable.c +++ b/variable.c @@ -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); diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index bb6ae68651..4635fd9c33 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -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;