Show where mutated chilled strings were allocated

[Feature #20205] The warning now suggests running with --debug-frozen-string-literal: ``` test.rb:3: warning: literal string will be frozen in the future (run with --debug-frozen-string-literal for more information) ``` When using --debug-frozen-string-literal, the location where the string was created is shown: ``` test.rb:3: warning: literal string will be frozen in the future test.rb:1: the string was created here ``` When resurrecting strings and debug mode is not enabled, the overhead is a simple FL_TEST_RAW. When mutating chilled strings and deprecation warnings are not enabled, the overhead is a simple warning category enabled check. Co-authored-by: Jean Boussier <byroot@ruby-lang.org>
Shopify · Oct 14, 2024 · 1d04830 · 1d04830
1 parent 1001ea9
commit 1d04830
Show file tree

Hide file tree

Showing 8 changed files with 49 additions and 9 deletions.
diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb
@@ -4726,7 +4726,7 @@ def test(klass, args)
 }
 
 # Chilled string setivar trigger warning
-assert_equal 'literal string will be frozen in the future', %q{
+assert_match(/literal string will be frozen in the future/, %q{
   Warning[:deprecated] = true
   $VERBOSE = true
   $warning = "no-warning"
@@ -4754,7 +4754,7 @@ def setivar!(str)
 
   setivar!("chilled") # Emit warning
   $warning
-}
+})
 
 # arity=-2 cfuncs
 assert_equal '["", "1/2", [0, [:ok, 1]]]', %q{

diff --git a/common.mk b/common.mk
@@ -8621,6 +8621,7 @@ io_buffer.$(OBJEXT): {$(VPATH)}config.h
 io_buffer.$(OBJEXT): {$(VPATH)}defines.h
 io_buffer.$(OBJEXT): {$(VPATH)}encoding.h
 io_buffer.$(OBJEXT): {$(VPATH)}fiber/scheduler.h
+io_buffer.$(OBJEXT): {$(VPATH)}id.h
 io_buffer.$(OBJEXT): {$(VPATH)}intern.h
 io_buffer.$(OBJEXT): {$(VPATH)}internal.h
 io_buffer.$(OBJEXT): {$(VPATH)}internal/abi.h
@@ -16287,6 +16288,7 @@ ruby_parser.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h
 ruby_parser.$(OBJEXT): {$(VPATH)}config.h
 ruby_parser.$(OBJEXT): {$(VPATH)}defines.h
 ruby_parser.$(OBJEXT): {$(VPATH)}encoding.h
+ruby_parser.$(OBJEXT): {$(VPATH)}id.h
 ruby_parser.$(OBJEXT): {$(VPATH)}intern.h
 ruby_parser.$(OBJEXT): {$(VPATH)}internal.h
 ruby_parser.$(OBJEXT): {$(VPATH)}internal/abi.h
@@ -17618,6 +17620,7 @@ strftime.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h
 strftime.$(OBJEXT): {$(VPATH)}config.h
 strftime.$(OBJEXT): {$(VPATH)}defines.h
 strftime.$(OBJEXT): {$(VPATH)}encoding.h
+strftime.$(OBJEXT): {$(VPATH)}id.h
 strftime.$(OBJEXT): {$(VPATH)}intern.h
 strftime.$(OBJEXT): {$(VPATH)}internal.h
 strftime.$(OBJEXT): {$(VPATH)}internal/abi.h

diff --git a/ext/-test-/string/depend b/ext/-test-/string/depend
@@ -174,6 +174,7 @@ capacity.o: $(hdrdir)/ruby/subst.h
 capacity.o: $(top_srcdir)/internal/compilers.h
 capacity.o: $(top_srcdir)/internal/string.h
 capacity.o: capacity.c
+capacity.o: $(top_srcdir)/id.h
 chilled.o: $(RUBY_EXTCONF_H)
 chilled.o: $(arch_hdrdir)/ruby/config.h
 chilled.o: $(hdrdir)/ruby.h
@@ -680,6 +681,7 @@ cstr.o: $(top_srcdir)/internal.h
 cstr.o: $(top_srcdir)/internal/compilers.h
 cstr.o: $(top_srcdir)/internal/string.h
 cstr.o: cstr.c
+cstr.o: $(top_srcdir)/id.h
 ellipsize.o: $(RUBY_EXTCONF_H)
 ellipsize.o: $(arch_hdrdir)/ruby/config.h
 ellipsize.o: $(hdrdir)/ruby.h
@@ -1530,6 +1532,7 @@ fstring.o: $(hdrdir)/ruby/subst.h
 fstring.o: $(top_srcdir)/internal/compilers.h
 fstring.o: $(top_srcdir)/internal/string.h
 fstring.o: fstring.c
+fstring.o: $(top_srcdir)/id.h
 init.o: $(RUBY_EXTCONF_H)
 init.o: $(arch_hdrdir)/ruby/config.h
 init.o: $(hdrdir)/ruby.h

diff --git a/internal/string.h b/internal/string.h
@@ -14,6 +14,7 @@
 #include "ruby/internal/stdbool.h"     /* for bool */
 #include "ruby/encoding.h"      /* for rb_encoding */
 #include "ruby/ruby.h"          /* for VALUE */
+#include "id.h"                 /* for id_debug_created_info */
 
 #define STR_NOEMBED      FL_USER1
 #define STR_SHARED       FL_USER2 /* = ELTS_SHARED */
@@ -123,8 +124,28 @@ CHILLED_STRING_P(VALUE obj)
 static inline void
 CHILLED_STRING_MUTATED(VALUE str)
 {
+    bool rb_warning_category_enabled_p(rb_warning_category_t category);
+
     FL_UNSET_RAW(str, STR_CHILLED);
-    rb_category_warn(RB_WARN_CATEGORY_DEPRECATED, "literal string will be frozen in the future");
+
+    if (RB_UNLIKELY(rb_warning_category_enabled_p(RB_WARN_CATEGORY_DEPRECATED))) {
+        VALUE debug_info = rb_attr_get(str, id_debug_created_info);
+        if (NIL_P(debug_info)) {
+            rb_category_warn(
+                RB_WARN_CATEGORY_DEPRECATED,
+                "literal string will be frozen in the future "
+                "(run with --debug-frozen-string-literal for more information)");
+        } else {
+            VALUE path = rb_ary_entry(debug_info, 0);
+            VALUE line = rb_ary_entry(debug_info, 1);
+
+            rb_category_warn(
+               RB_WARN_CATEGORY_DEPRECATED,
+               "literal string will be frozen in the future\n%"PRIsVALUE":%"PRIsVALUE": the string was created here",
+               path,
+               line);
+        }
+    }
 }
 
 static inline void

diff --git a/spec/ruby/command_line/fixtures/debug_info.rb b/spec/ruby/command_line/fixtures/debug_info.rb
@@ -1,4 +1,3 @@
-# frozen_string_literal: true
 a = 'string'
 b = a
 c = b

diff --git a/spec/ruby/command_line/frozen_strings_spec.rb b/spec/ruby/command_line/frozen_strings_spec.rb
@@ -57,9 +57,18 @@
 
 describe "The --debug flag produces" do
   it "debugging info on attempted frozen string modification" do
-    error_str = ruby_exe(fixture(__FILE__, 'debug_info.rb'), options: '--debug',  args: "2>&1")
+    error_str = ruby_exe(fixture(__FILE__, 'debug_info.rb'), options: '--enable-frozen-string-literal --debug',  args: "2>&1")
     error_str.should include("can't modify frozen String")
     error_str.should include("created at")
-    error_str.should include("command_line/fixtures/debug_info.rb:2")
+    error_str.should include("command_line/fixtures/debug_info.rb:1")
+  end
+
+  guard -> { ruby_version_is "3.4" and !"test".frozen? } do
+    it "debugging info on mutating chilled string" do
+      error_str = ruby_exe(fixture(__FILE__, 'debug_info.rb'), options: '-w --debug',  args: "2>&1")
+      error_str.should include("literal string will be frozen in the future")
+      error_str.should include("the string was created here")
+      error_str.should include("command_line/fixtures/debug_info.rb:1")
+    end
   end
 end
diff --git a/string.c b/string.c
@@ -1914,6 +1914,12 @@ rb_ec_str_resurrect(struct rb_execution_context_struct *ec, VALUE str, bool chil
     RUBY_DTRACE_CREATE_HOOK(STRING, RSTRING_LEN(str));
     VALUE new_str = ec_str_duplicate(ec, rb_cString, str);
     if (chilled) {
+        if (RB_UNLIKELY(FL_TEST_RAW(str, FL_EXIVAR))) {
+            VALUE debug_info = rb_ivar_get(str, id_debug_created_info);
+            if (!NIL_P(debug_info)) {
+                rb_ivar_set(new_str, id_debug_created_info, debug_info);
+            }
+        }
         STR_CHILL_RAW(new_str);
     }
     return new_str;

diff --git a/test/ruby/test_rubyoptions.rb b/test/ruby/test_rubyoptions.rb
@@ -1262,9 +1262,8 @@ def test_frozen_string_literal_debug_chilled_strings
     code = <<~RUBY
       "foo" << "bar"
     RUBY
-    warning = ["-:1: warning: literal string will be frozen in the future"]
-    assert_in_out_err(["-W:deprecated"], code, [], warning)
-    assert_in_out_err(["-W:deprecated", "--debug-frozen-string-literal"], code, [], warning)
+    assert_in_out_err(["-W:deprecated"], code, [], ["-:1: warning: literal string will be frozen in the future (run with --debug-frozen-string-literal for more information)"])
+    assert_in_out_err(["-W:deprecated", "--debug-frozen-string-literal"], code, [], ["-:1: warning: literal string will be frozen in the future", "-:1: the string was created here"])
     assert_in_out_err(["-W:deprecated", "--disable-frozen-string-literal", "--debug-frozen-string-literal"], code, [], [])
     assert_in_out_err(["-W:deprecated", "--enable-frozen-string-literal", "--debug-frozen-string-literal"], code, [], ["-:1:in '<main>': can't modify frozen String: \"foo\", created at -:1 (FrozenError)"])
   end