ruby-changes:62974
From: Benoit <ko1@a...>
Date: Wed, 16 Sep 2020 04:32:52 +0900 (JST)
Subject: [ruby-changes:62974] 9b535f3ff7 (master): Interpolated strings are no longer frozen with frozen-string-literal: true
https://git.ruby-lang.org/ruby.git/commit/?id=9b535f3ff7 From 9b535f3ff7c2f48e34dd44564df7adc723b81276 Mon Sep 17 00:00:00 2001 From: Benoit Daloze <eregontp@g...> Date: Mon, 31 Aug 2020 21:24:36 +0200 Subject: Interpolated strings are no longer frozen with frozen-string-literal: true * Remove freezestring instruction since this was the only usage for it. * [Feature #17104] diff --git a/NEWS.md b/NEWS.md index a8f2af9..883a20b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -80,6 +80,9 @@ sufficient information, see the ChangeLog file or Redmine https://github.com/ruby/ruby/blob/trunk/NEWS.md#L80 def square(x) = x * x ``` +* Interpolated String literals are no longer frozen when + `# frozen-string-literal: true` is used. [[Feature #17104]] + ## Command line options ### `--help` option @@ -373,6 +376,7 @@ Excluding feature bug fixes. https://github.com/ruby/ruby/blob/trunk/NEWS.md#L376 [Feature #16746]: https://bugs.ruby-lang.org/issues/16746 [Feature #16754]: https://bugs.ruby-lang.org/issues/16754 [Feature #16828]: https://bugs.ruby-lang.org/issues/16828 +[Feature #17104]: https://bugs.ruby-lang.org/issues/17104 [Misc #16961]: https://bugs.ruby-lang.org/issues/16961 [Feature #17122]: https://bugs.ruby-lang.org/issues/17122 [GH-2991]: https://github.com/ruby/ruby/pull/2991 diff --git a/bootstraptest/test_insns.rb b/bootstraptest/test_insns.rb index 8addbf7..bcd3a8e 100644 --- a/bootstraptest/test_insns.rb +++ b/bootstraptest/test_insns.rb @@ -89,8 +89,6 @@ tests = [ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_insns.rb#L89 [ 'putiseq', %q{ -> { true }.() }, ], [ 'putstring', %q{ "true" }, ], [ 'tostring / concatstrings', %q{ "#{true}" }, ], - [ 'freezestring', %q{ "#{true}" }, fsl, ], - [ 'freezestring', %q{ "#{true}" }, '-d', fsl, ], [ 'toregexp', %q{ /#{true}/ =~ "true" && $~ }, ], [ 'intern', %q{ :"#{true}" }, ], diff --git a/compile.c b/compile.c index 96b8bc6..797a170 100644 --- a/compile.c +++ b/compile.c @@ -2693,21 +2693,6 @@ iseq_pop_newarray(rb_iseq_t *iseq, INSN *iobj) https://github.com/ruby/ruby/blob/trunk/compile.c#L2693 } static int -same_debug_pos_p(LINK_ELEMENT *iobj1, LINK_ELEMENT *iobj2) -{ - VALUE debug1 = OPERAND_AT(iobj1, 0); - VALUE debug2 = OPERAND_AT(iobj2, 0); - if (debug1 == debug2) return TRUE; - if (!RB_TYPE_P(debug1, T_ARRAY)) return FALSE; - if (!RB_TYPE_P(debug2, T_ARRAY)) return FALSE; - if (RARRAY_LEN(debug1) != 2) return FALSE; - if (RARRAY_LEN(debug2) != 2) return FALSE; - if (RARRAY_AREF(debug1, 0) != RARRAY_AREF(debug2, 0)) return FALSE; - if (RARRAY_AREF(debug1, 1) != RARRAY_AREF(debug2, 1)) return FALSE; - return TRUE; -} - -static int is_frozen_putstring(INSN *insn, VALUE *op) { if (IS_INSN_ID(insn, putstring)) { @@ -3229,10 +3214,8 @@ iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal https://github.com/ruby/ruby/blob/trunk/compile.c#L3214 * => * concatstrings N+M-1 */ - LINK_ELEMENT *next = iobj->link.next, *freeze = 0; + LINK_ELEMENT *next = iobj->link.next; INSN *jump = 0; - if (IS_INSN(next) && IS_INSN_ID(next, freezestring)) - next = (freeze = next)->next; if (IS_INSN(next) && IS_INSN_ID(next, jump)) next = get_destination_insn(jump = (INSN *)next); if (IS_INSN(next) && IS_INSN_ID(next, concatstrings)) { @@ -3248,44 +3231,15 @@ iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal https://github.com/ruby/ruby/blob/trunk/compile.c#L3231 OPERAND_AT(jump, 0) = (VALUE)label; } label->refcnt++; - if (freeze && IS_NEXT_INSN_ID(next, freezestring)) { - if (same_debug_pos_p(freeze, next->next)) { - ELEM_REMOVE(freeze); - } - else { - next = next->next; - } - } ELEM_INSERT_NEXT(next, &label->link); CHECK(iseq_peephole_optimize(iseq, get_next_insn(jump), do_tailcallopt)); } else { - if (freeze) ELEM_REMOVE(freeze); ELEM_REMOVE(next); } } } - if (IS_INSN_ID(iobj, freezestring) && - NIL_P(OPERAND_AT(iobj, 0)) && - IS_NEXT_INSN_ID(&iobj->link, send)) { - INSN *niobj = (INSN *)iobj->link.next; - const struct rb_callinfo *ci = (struct rb_callinfo *)OPERAND_AT(niobj, 0); - - /* - * freezestring nil # no debug_info - * send <:+@, 0, ARG_SIMPLE> # :-@, too - * => - * send <:+@, 0, ARG_SIMPLE> # :-@, too - */ - if ((vm_ci_mid(ci) == idUPlus || vm_ci_mid(ci) == idUMinus) && - (vm_ci_flag(ci) & VM_CALL_ARGS_SIMPLE) && - vm_ci_argc(ci) == 0) { - ELEM_REMOVE(list); - return COMPILE_OK; - } - } - if (do_tailcallopt && (IS_INSN_ID(iobj, send) || IS_INSN_ID(iobj, opt_aref_with) || @@ -8402,18 +8356,6 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, in https://github.com/ruby/ruby/blob/trunk/compile.c#L8356 if (popped) { ADD_INSN(ret, line, pop); } - else { - if (ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal) { - VALUE debug_info = Qnil; - if (ISEQ_COMPILE_DATA(iseq)->option->debug_frozen_string_literal || RTEST(ruby_debug)) { - debug_info = rb_ary_new_from_args(2, rb_iseq_path(iseq), INT2FIX(line)); - } - ADD_INSN1(ret, line, freezestring, debug_info); - if (!NIL_P(debug_info)) { - RB_OBJ_WRITTEN(iseq, Qundef, rb_obj_freeze(debug_info)); - } - } - } break; } case NODE_XSTR:{ diff --git a/insns.def b/insns.def index 2151ae2..f6f802f 100644 --- a/insns.def +++ b/insns.def @@ -390,16 +390,6 @@ tostring https://github.com/ruby/ruby/blob/trunk/insns.def#L390 val = rb_obj_as_string_result(str, val); } -/* Freeze (dynamically) created strings. if debug_info is given, set it. */ -DEFINE_INSN -freezestring -(VALUE debug_info) -(VALUE str) -(VALUE str) -{ - vm_freezestring(str, debug_info); -} - /* compile str to Regexp and push it. opt is the option for the Regexp. */ diff --git a/spec/ruby/language/string_spec.rb b/spec/ruby/language/string_spec.rb index d19b909..0178083 100644 --- a/spec/ruby/language/string_spec.rb +++ b/spec/ruby/language/string_spec.rb @@ -291,4 +291,21 @@ describe "Ruby String interpolation" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/language/string_spec.rb#L291 -> { "#{a} #{b}" }.should raise_error(Encoding::CompatibilityError) end + + it "creates a non-frozen String" do + code = <<~'RUBY' + "a#{6*7}c" + RUBY + eval(code).should_not.frozen? + end + + ruby_version_is "3.0" do + it "creates a non-frozen String when # frozen-string-literal: true is used" do + code = <<~'RUBY' + # frozen-string-literal: true + "a#{6*7}c" + RUBY + eval(code).should_not.frozen? + end + end end diff --git a/test/ruby/test_iseq.rb b/test/ruby/test_iseq.rb index 1e7dbe0..7fb6268 100644 --- a/test/ruby/test_iseq.rb +++ b/test/ruby/test_iseq.rb @@ -187,8 +187,8 @@ class TestISeq < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_iseq.rb#L187 s1, s2, s3, s4 = compile(code, line, {frozen_string_literal: true}).eval assert_predicate(s1, :frozen?) assert_predicate(s2, :frozen?) - assert_predicate(s3, :frozen?) - assert_predicate(s4, :frozen?) + assert_not_predicate(s3, :frozen?) + assert_predicate(s4, :frozen?) # should probably not be frozen, but unrealistic code end # Safe call chain is not optimized when Coverage is running. diff --git a/test/ruby/test_jit.rb b/test/ruby/test_jit.rb index 6aad924..c0cc28f 100644 --- a/test/ruby/test_jit.rb +++ b/test/ruby/test_jit.rb @@ -247,14 +247,6 @@ class TestJIT < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_jit.rb#L247 assert_compile_once('"a#{}b" + "c"', result_inspect: '"abc"', insns: %i[putstring concatstrings tostring]) end - def test_compile_insn_freezestring - assert_eval_with_jit("#{<<~"begin;"}\n#{<<~'end;'}", stdout: 'true', success_count: 1, insns: %i[freezestring]) - begin; - # frozen_string_literal: true - print proc { "#{true}".frozen? }.call - end; - end - def test_compile_insn_toregexp assert_compile_once('/#{true}/ =~ "true"', result_inspect: '0', insns: %i[toregexp]) end diff --git a/test/ruby/test_literal.rb b/test/ruby/test_literal.rb index 7f4a329..f5dd093 100644 --- a/test/ruby/test_literal.rb +++ b/test/ruby/test_literal.rb @@ -187,7 +187,7 @@ class TestRubyLiteral < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_literal.rb#L187 if defined?(RubyVM::InstructionSequence.compile_option) and RubyVM::InstructionSequence.compile_option.key?(:debug_frozen_string_literal) def test_debug_frozen_string - src = 'n = 1; _="foo#{n ? "-#{n}" : ""}"'; f = "test.rb"; n = 1 + src = '_="foo-1"'; f = "test.rb"; n = 1 opt = {frozen_string_literal: true, debug_frozen_string_literal: true} str = RubyVM::InstructionSequence.compile(src, f, f, n, **opt).eval assert_equal("foo-1", str) diff --git a/test/ruby/test_rubyoptions.rb b/test/ruby/test_rubyoptions.rb index 754918d..7101175 100644 --- a/test/ruby/test_rubyoptions.rb +++ b/test/ruby/test_rubyoptions.rb @@ -1014,11 +1014,11 @@ class TestRubyOptions < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_rubyoptions.rb#L1014 err = !freeze ? [] : debug ? with_debug_pat : wo_debug_pat [ ['"foo" << "bar"', err], - ['"foo#{123}bar" << "bar"', err], + ['"foo#{123}bar" << "bar"', []], ['+"foo#{123}bar" << "bar"', []], - ['-"foo#{123}bar" << "bar"', freeze && debug ? with_debug_pat : wo_debug_pat], + ['-"foo#{123}bar" << "bar"', wo_debug_pat], ].each do |code, expected| - assert_in_out_err(opt, code, [], expected, [opt, code]) + assert_in_out_err(opt, code, [], expected, "#{opt} #{code}") end end end diff --git a/vm_insn (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/