From 70b60d24b9c3859a859853ddb2e17c603bd3485b Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 28 Jul 2022 16:41:46 -0700 Subject: [PATCH] Fix inconsistency with opt_aref_with opt_aref_with is an optimized instruction for accessing a Hash using a non-frozen string key (ie. from a file without frozen_string_literal). It attempts to avoid allocating the string, and instead silently using a frozen string (hash string keys are always fstrings). Because this is just an optimization, it should be invisible to the user. However, previously this optimization was could be seen via hashes with default procs. For example, previously: h = Hash.new { |h, k| k.frozen? } str = "foo" h[str] # false h["foo"] # true when optimizations enabled This commit checks that the Hash doesn't have a default proc when using opt_aref_with. --- test/ruby/test_hash.rb | 14 ++++++++++++++ vm_insnhelper.c | 3 ++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/test/ruby/test_hash.rb b/test/ruby/test_hash.rb index 91423f81ea..83d16d462e 100644 --- a/test/ruby/test_hash.rb +++ b/test/ruby/test_hash.rb @@ -304,6 +304,20 @@ class TestHash < Test::Unit::TestCase assert_equal before, ObjectSpace.count_objects[:T_STRING] end + def test_AREF_fstring_key_default_proc + assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}") + begin; + h = Hash.new do |h, k| + k.frozen? + end + + str = "foo" + refute str.frozen? # assumes this file is frozen_string_literal: false + refute h[str] + refute h["foo"] + end; + end + def test_ASET_fstring_key a, b = {}, {} assert_equal 1, a["abc"] = 1 diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 2ff48d2662..2c0a369a43 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -5458,7 +5458,8 @@ vm_opt_aref_with(VALUE recv, VALUE key) { if (!SPECIAL_CONST_P(recv) && RBASIC_CLASS(recv) == rb_cHash && BASIC_OP_UNREDEFINED_P(BOP_AREF, HASH_REDEFINED_OP_FLAG) && - rb_hash_compare_by_id_p(recv) == Qfalse) { + rb_hash_compare_by_id_p(recv) == Qfalse && + !FL_TEST(recv, RHASH_PROC_DEFAULT)) { return rb_hash_aref(recv, key); } else {