Warn on modified future frozen string literal by casperisfine · Pull Request #10262 · ruby/ruby

casperisfine · 2024-03-15T09:42:06Z

[Feature #20205]

As a path toward enabling frozen string literals by default in the future, this commit introduce "chilled strings". From a user perspective chilled strings pretend to be frozen, but on the first attempt to mutate them, they lose their frozen status and emit a warning rather than to raise a FrozenError.

Implementation wise, rb_compile_option_struct.frozen_string_literal is no longer a boolean but a tri-state of enabled/disabled/unset.

When code is compiled with frozen string literals neither explictly enabled or disabled, string literals are compiled with a new putchilledstring instruction. This instruction is identical to putstring except it marks the String with the STR_CHILLED (FL_USER3) and FL_FREEZE flags.

Chilled strings have the FL_FREEZE flag as to minimize the need to check for chilled strings across the codebase, and to improve compatibility with C extensions.

Notes:

String#freeze: clears the chilled flag.
String#-@: acts as if the string was mutable.
String#+@: acts as if the string was mutable.
String#clone: copies the chilled flag.

cc @etiennebarrie

Ref: https://bugs.ruby-lang.org/issues/20205 Ref: ruby/ruby#10262 Ruby 3.4 will very likely add a `putchilledstring` instruction which for what concerns typeprof is pretty much an alias of `putstring`.

eregon

Specs look great

KJTsanaktsidis

I had a couple of questions but overall it looks right to me 👍

[Feature #20205] As a path toward enabling frozen string literals by default in the future, this commit introduce "chilled strings". From a user perspective chilled strings pretend to be frozen, but on the first attempt to mutate them, they lose their frozen status and emit a warning rather than to raise a `FrozenError`. Implementation wise, `rb_compile_option_struct.frozen_string_literal` is no longer a boolean but a tri-state of `enabled/disabled/unset`. When code is compiled with frozen string literals neither explictly enabled or disabled, string literals are compiled with a new `putchilledstring` instruction. This instruction is identical to `putstring` except it marks the String with the `STR_CHILLED (FL_USER3)` and `FL_FREEZE` flags. Chilled strings have the `FL_FREEZE` flag as to minimize the need to check for chilled strings across the codebase, and to improve compatibility with C extensions. Notes: - `String#freeze`: clears the chilled flag. - `String#-@`: acts as if the string was mutable. - `String#+@`: acts as if the string was mutable. - `String#clone`: copies the chilled flag. Co-authored-by: Jean Boussier <byroot@ruby-lang.org>

…rettyPrint#text (#177) Fixes #176 https://bugs.ruby-lang.org/issues/20205 ruby/ruby#10262

ruby/ruby#10262

…gainst Ruby 3.4.0dev This commit addresses the following Rails Nightly CI error since ruby/ruby#10262 . https://buildkite.com/rails/rails-nightly/builds/310#018e5929-ff70-4397-b978-9a0a03cd4706/1255-1265 - Without this commit: ```ruby $ ruby -v ruby 3.4.0dev (2024-03-19T08:26:49Z master 12be40ae6b) [x86_64-linux] $ cd actionpack $ RAILS_STRICT_WARNINGS=true bin/test test/controller/renderer_test.rb:37 Running 25 tests in a single process (parallelization threshold is 50) Run options: --seed 14013 /home/yahonda/src/github.com/rails/rails/actionpack/test/fixtures/ruby_template.ruby:2: warning: literal string will be frozen in the future E Error: RendererTest#test_rendering_with_a_class_renderer: RuntimeError: Neutered Exception ActionView::Template::Error: /home/yahonda/src/github.com/rails/rails/actionpack/test/fixtures/ruby_template.ruby:2: warning: literal string will be frozen in the future /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/strict_warnings.rb:33:in 'ActiveSupport::RaiseWarnings#warn' test/fixtures/ruby_template.ruby:2:in '_home_yahonda_src_github_com_rails_rails_actionpack_test_fixtures_ruby_template_ruby__3648742137162546161_6360' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/base.rb:282:in 'Kernel#public_send' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/base.rb:282:in 'ActionView::Base#_run' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/template.rb:275:in 'block in ActionView::Template#render' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications.rb:212:in 'ActiveSupport::Notifications.instrument' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/template.rb:567:in 'ActionView::Template#instrument_render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/template.rb:263:in 'ActionView::Template#render' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:66:in 'block (2 levels) in ActionView::TemplateRenderer#render_template' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications.rb:212:in 'ActiveSupport::Notifications.instrument' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:60:in 'block in ActionView::TemplateRenderer#render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:80:in 'ActionView::TemplateRenderer#render_with_layout' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:59:in 'ActionView::TemplateRenderer#render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:11:in 'ActionView::TemplateRenderer#render' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/renderer.rb:58:in 'ActionView::Renderer#render_template_to_object' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/renderer.rb:31:in 'ActionView::Renderer#render_to_object' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/rendering.rb:135:in 'block in ActionView::Rendering#_render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/base.rb:309:in 'ActionView::Base#in_rendering_context' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/rendering.rb:134:in 'ActionView::Rendering#_render_template' lib/action_controller/metal/streaming.rb:258:in 'ActionController::Streaming#_render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/rendering.rb:121:in 'ActionView::Rendering#render_to_body' lib/action_controller/metal/rendering.rb:186:in 'ActionController::Rendering#render_to_body' lib/action_controller/metal/renderers.rb:142:in 'ActionController::Renderers#render_to_body' lib/abstract_controller/rendering.rb:47:in 'AbstractController::Rendering#render_to_string' lib/action_controller/metal/rendering.rb:175:in 'ActionController::Rendering#render_to_string' lib/action_controller/renderer.rb:136:in 'ActionController::Renderer#render' test/controller/renderer_test.rb:37:in 'block in <class:RendererTest>' bin/test test/controller/renderer_test.rb:35 Finished in 0.294798s, 3.3921 runs/s, 0.0000 assertions/s. 1 runs, 0 assertions, 0 failures, 1 errors, 0 skips $ ``` Refer to these Ruby issue and pull request for this change: https://bugs.ruby-lang.org/issues/20205 ruby/ruby#10262

…gainst Ruby 3.4.0dev This commit addresses the following Rails Nightly CI error since ruby/ruby#10262 . https://buildkite.com/rails/rails-nightly/builds/310#018e5929-ff70-4397-b978-9a0a03cd4706/1255-1265 - Without this commit: ```ruby $ ruby -v ruby 3.4.0dev (2024-03-19T08:26:49Z master 12be40ae6b) [x86_64-linux] $ cd actionpack $ RAILS_STRICT_WARNINGS=true bin/test test/controller/renderer_test.rb:37 Running 25 tests in a single process (parallelization threshold is 50) Run options: --seed 14013 /home/yahonda/src/github.com/rails/rails/actionpack/test/fixtures/ruby_template.ruby:2: warning: literal string will be frozen in the future E Error: RendererTest#test_rendering_with_a_class_renderer: RuntimeError: Neutered Exception ActionView::Template::Error: /home/yahonda/src/github.com/rails/rails/actionpack/test/fixtures/ruby_template.ruby:2: warning: literal string will be frozen in the future /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/strict_warnings.rb:33:in 'ActiveSupport::RaiseWarnings#warn' test/fixtures/ruby_template.ruby:2:in '_home_yahonda_src_github_com_rails_rails_actionpack_test_fixtures_ruby_template_ruby__3648742137162546161_6360' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/base.rb:282:in 'Kernel#public_send' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/base.rb:282:in 'ActionView::Base#_run' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/template.rb:275:in 'block in ActionView::Template#render' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications.rb:212:in 'ActiveSupport::Notifications.instrument' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/template.rb:567:in 'ActionView::Template#instrument_render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/template.rb:263:in 'ActionView::Template#render' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:66:in 'block (2 levels) in ActionView::TemplateRenderer#render_template' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications.rb:212:in 'ActiveSupport::Notifications.instrument' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:60:in 'block in ActionView::TemplateRenderer#render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:80:in 'ActionView::TemplateRenderer#render_with_layout' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:59:in 'ActionView::TemplateRenderer#render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:11:in 'ActionView::TemplateRenderer#render' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/renderer.rb:58:in 'ActionView::Renderer#render_template_to_object' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/renderer.rb:31:in 'ActionView::Renderer#render_to_object' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/rendering.rb:135:in 'block in ActionView::Rendering#_render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/base.rb:309:in 'ActionView::Base#in_rendering_context' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/rendering.rb:134:in 'ActionView::Rendering#_render_template' lib/action_controller/metal/streaming.rb:258:in 'ActionController::Streaming#_render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/rendering.rb:121:in 'ActionView::Rendering#render_to_body' lib/action_controller/metal/rendering.rb:186:in 'ActionController::Rendering#render_to_body' lib/action_controller/metal/renderers.rb:142:in 'ActionController::Renderers#render_to_body' lib/abstract_controller/rendering.rb:47:in 'AbstractController::Rendering#render_to_string' lib/action_controller/metal/rendering.rb:175:in 'ActionController::Rendering#render_to_string' lib/action_controller/renderer.rb:136:in 'ActionController::Renderer#render' test/controller/renderer_test.rb:37:in 'block in <class:RendererTest>' bin/test test/controller/renderer_test.rb:35 Finished in 0.294798s, 3.3921 runs/s, 0.0000 assertions/s. 1 runs, 0 assertions, 0 failures, 1 errors, 0 skips $ ``` Refer to these Ruby issue and pull request for this change: https://bugs.ruby-lang.org/issues/20205 ruby/ruby#10262 Co-authored-by: Rafael Mendonça França <rafael@franca.dev>

Earlopain · 2024-03-22T12:52:13Z

Hey @casperisfine , I don't have an ruby issue tracker account so this will have to do:

This works in Ruby 3.3 but raises on head and results in some test failures for unicorn:

require "stringio"

StringIO.new("").write("foo")
#=> 'StringIO#write': not opened for writing (IOError)

This is because the chilld string is not considered writeable in this codepath: https://github.com/ruby/stringio/blob/779f71359d4760df49b70cfbcf908bc012e7e4ba/ext/stringio/stringio.c#L156-L165

casperisfine · 2024-03-22T13:01:46Z

Yes this is known and somewhat expected. But perhaps we could detect this in StringIO and turn this into a warning.

casperisfine · 2024-03-22T13:03:28Z

For the short term, you can set RUBYOPT="--disable-frozen-string-literal"

Earlopain · 2024-03-22T13:29:26Z

Hm, I guess this is something like raise if frozen?. Since they pretent to be, this now fails. I'll try to open a PR so that this prints the deprecation instead.

casperisfine · 2024-03-22T13:31:47Z

Yes, it's right there: https://github.com/ruby/stringio/blob/779f71359d4760df49b70cfbcf908bc012e7e4ba/ext/stringio/stringio.c#L290-L294

…gainst Ruby 3.4.0dev This commit addresses the following Rails Nightly CI error since ruby/ruby#10262 . https://buildkite.com/rails/rails-nightly/builds/310#018e5929-ff70-4397-b978-9a0a03cd4706/1255-1265 - Without this commit: ```ruby $ ruby -v ruby 3.4.0dev (2024-03-19T08:26:49Z master 12be40ae6b) [x86_64-linux] $ cd actionpack $ RAILS_STRICT_WARNINGS=true bin/test test/controller/renderer_test.rb:37 Running 25 tests in a single process (parallelization threshold is 50) Run options: --seed 14013 /home/yahonda/src/github.com/rails/rails/actionpack/test/fixtures/ruby_template.ruby:2: warning: literal string will be frozen in the future E Error: RendererTest#test_rendering_with_a_class_renderer: RuntimeError: Neutered Exception ActionView::Template::Error: /home/yahonda/src/github.com/rails/rails/actionpack/test/fixtures/ruby_template.ruby:2: warning: literal string will be frozen in the future /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/strict_warnings.rb:33:in 'ActiveSupport::RaiseWarnings#warn' test/fixtures/ruby_template.ruby:2:in '_home_yahonda_src_github_com_rails_rails_actionpack_test_fixtures_ruby_template_ruby__3648742137162546161_6360' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/base.rb:282:in 'Kernel#public_send' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/base.rb:282:in 'ActionView::Base#_run' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/template.rb:275:in 'block in ActionView::Template#render' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications.rb:212:in 'ActiveSupport::Notifications.instrument' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/template.rb:567:in 'ActionView::Template#instrument_render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/template.rb:263:in 'ActionView::Template#render' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:66:in 'block (2 levels) in ActionView::TemplateRenderer#render_template' /home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/notifications.rb:212:in 'ActiveSupport::Notifications.instrument' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:60:in 'block in ActionView::TemplateRenderer#render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:80:in 'ActionView::TemplateRenderer#render_with_layout' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:59:in 'ActionView::TemplateRenderer#render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/template_renderer.rb:11:in 'ActionView::TemplateRenderer#render' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/renderer.rb:58:in 'ActionView::Renderer#render_template_to_object' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/renderer/renderer.rb:31:in 'ActionView::Renderer#render_to_object' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/rendering.rb:135:in 'block in ActionView::Rendering#_render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/base.rb:309:in 'ActionView::Base#in_rendering_context' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/rendering.rb:134:in 'ActionView::Rendering#_render_template' lib/action_controller/metal/streaming.rb:258:in 'ActionController::Streaming#_render_template' /home/yahonda/src/github.com/rails/rails/actionview/lib/action_view/rendering.rb:121:in 'ActionView::Rendering#render_to_body' lib/action_controller/metal/rendering.rb:186:in 'ActionController::Rendering#render_to_body' lib/action_controller/metal/renderers.rb:142:in 'ActionController::Renderers#render_to_body' lib/abstract_controller/rendering.rb:47:in 'AbstractController::Rendering#render_to_string' lib/action_controller/metal/rendering.rb:175:in 'ActionController::Rendering#render_to_string' lib/action_controller/renderer.rb:136:in 'ActionController::Renderer#render' test/controller/renderer_test.rb:37:in 'block in <class:RendererTest>' bin/test test/controller/renderer_test.rb:35 Finished in 0.294798s, 3.3921 runs/s, 0.0000 assertions/s. 1 runs, 0 assertions, 0 failures, 1 errors, 0 skips $ ``` Refer to these Ruby issue and pull request for this change: https://bugs.ruby-lang.org/issues/20205 ruby/ruby#10262 Co-authored-by: Rafael Mendonça França <rafael@franca.dev>

casperisfine mentioned this pull request Mar 15, 2024

Warn on modified future frozen string literal Shopify/ruby#549

Closed

casperisfine mentioned this pull request Mar 15, 2024

Add support for future putchilledstring instruction ruby/typeprof#158

Merged

This comment was marked as outdated.

Sign in to view

casperisfine force-pushed the warn-on-modified-future-frozen-string-literal branch from 84be7d6 to 548ed3b Compare March 15, 2024 15:08

This comment has been minimized.

Sign in to view

casperisfine commented Mar 15, 2024

View reviewed changes

Comment thread test/ruby/test_compile_prism.rb Outdated

casperisfine force-pushed the warn-on-modified-future-frozen-string-literal branch 3 times, most recently from 628e076 to 557f0bb Compare March 15, 2024 17:28

casperisfine force-pushed the warn-on-modified-future-frozen-string-literal branch 2 times, most recently from 47e125e to 504d4fc Compare March 18, 2024 09:50

casperisfine force-pushed the warn-on-modified-future-frozen-string-literal branch 2 times, most recently from 2ec1aa6 to af2895e Compare March 18, 2024 11:15

casperisfine marked this pull request as ready for review March 18, 2024 11:20

casperisfine requested a review from a team as a code owner March 18, 2024 11:20

eregon approved these changes Mar 18, 2024

View reviewed changes

Comment thread spec/ruby/core/string/chilled_string_spec.rb Outdated

Comment thread spec/ruby/core/string/uplus_spec.rb Outdated

casperisfine force-pushed the warn-on-modified-future-frozen-string-literal branch 3 times, most recently from 851fc04 to 2774757 Compare March 18, 2024 16:31

tenderlove approved these changes Mar 18, 2024

View reviewed changes

casperisfine mentioned this pull request Mar 18, 2024

Make the test suite compatible with --enable-frozen-string-literal minitest/minitest#991

Closed

KJTsanaktsidis approved these changes Mar 18, 2024

View reviewed changes

Comment thread spec/ruby/core/string/chilled_string_spec.rb

Comment thread error.c

Comment thread include/ruby/internal/intern/error.h

Comment thread iseq.c

Comment thread prism_compile.c

casperisfine force-pushed the warn-on-modified-future-frozen-string-literal branch from 2774757 to 5dc8ddc Compare March 19, 2024 07:41

byroot merged commit 12be40a into ruby:master Mar 19, 2024

casperisfine deleted the warn-on-modified-future-frozen-string-literal branch March 19, 2024 09:12

andrykonchin mentioned this pull request Mar 19, 2024

Additionally polish specs to run with --enable-frozen-string-literal ruby/spec#1144

Merged

kachick mentioned this pull request Mar 20, 2024

Test is failing against ruby-head kachick/irb-power_assert#176

Closed

kachick added a commit to kachick/irb-power_assert that referenced this pull request Mar 20, 2024

Ignore frozen string literal warning in ruby-head(3.4.0dev) and the P…

a113d11

…rettyPrint#text (#177) Fixes #176 https://bugs.ruby-lang.org/issues/20205 ruby/ruby#10262

kachick added a commit to kachick/ruby-ulid that referenced this pull request Mar 20, 2024

Ignore perfect_toml have warnings in ruby-head

a699625

ruby/ruby#10262

yahonda mentioned this pull request Mar 20, 2024

Address RendererTest#test_rendering_with_a_class_renderer failure against Ruby 3.4.0dev rails/rails#51357

Merged

4 tasks

Earlopain mentioned this pull request Mar 22, 2024

Print a deprecation warning for chilled strings ruby/stringio#92

Closed

yahonda mentioned this pull request Apr 30, 2024

test_render_mutate_string_literal shows inline template:1: warning: literal string will be frozen in the future rails/rails#51689

Closed

kachick mentioned this pull request May 1, 2024

Ignore frozen string warning in latest ruby kachick/rspec-matchers-power_assert_matchers#192

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warn on modified future frozen string literal#10262

Warn on modified future frozen string literal#10262
byroot merged 1 commit intoruby:masterfrom
Shopify:warn-on-modified-future-frozen-string-literal

casperisfine commented Mar 15, 2024 •

edited

Loading

Uh oh!

This comment was marked as outdated.

This comment has been minimized.

Uh oh!

eregon left a comment

Uh oh!

Uh oh!

Uh oh!

KJTsanaktsidis left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Earlopain commented Mar 22, 2024

Uh oh!

casperisfine commented Mar 22, 2024

Uh oh!

casperisfine commented Mar 22, 2024

Uh oh!

Earlopain commented Mar 22, 2024

Uh oh!

casperisfine commented Mar 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

casperisfine commented Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment has been minimized.

Uh oh!

eregon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

KJTsanaktsidis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Earlopain commented Mar 22, 2024

Uh oh!

casperisfine commented Mar 22, 2024

Uh oh!

casperisfine commented Mar 22, 2024

Uh oh!

Earlopain commented Mar 22, 2024

Uh oh!

casperisfine commented Mar 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

casperisfine commented Mar 15, 2024 •

edited

Loading