From 4cb3c44e424bfca6b97e99f6e084331cb4e09622 Mon Sep 17 00:00:00 2001 From: Peter Arato Date: Mon, 3 Apr 2023 17:05:19 -0400 Subject: [PATCH 1/5] Make an empty hash shift return nil. --- src/main/java/org/truffleruby/core/hash/HashNodes.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/truffleruby/core/hash/HashNodes.java b/src/main/java/org/truffleruby/core/hash/HashNodes.java index 42a083dc1ccf..8c438fc23e6f 100644 --- a/src/main/java/org/truffleruby/core/hash/HashNodes.java +++ b/src/main/java/org/truffleruby/core/hash/HashNodes.java @@ -511,9 +511,9 @@ protected Object setDefault(RubyHash hash, Object defaultValue, public abstract static class ShiftNode extends CoreMethodArrayArgumentsNode { @Specialization(guards = "hash.empty()") - protected Object shiftEmpty(RubyHash hash, + protected Nil shiftEmpty(RubyHash hash, @Cached DispatchNode callDefault) { - return callDefault.call(hash, "default", nil); + return nil; } @Specialization(guards = "!hash.empty()", limit = "hashStrategyLimit()") From 3a404867aff2159bb96635b46f403a115659cef9 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Thu, 6 Apr 2023 13:28:14 +0300 Subject: [PATCH 2/5] Tag Hash#shift specs for old behaviour as failed --- spec/tags/core/hash/shift_tags.txt | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 spec/tags/core/hash/shift_tags.txt diff --git a/spec/tags/core/hash/shift_tags.txt b/spec/tags/core/hash/shift_tags.txt new file mode 100644 index 000000000000..157bf58557ab --- /dev/null +++ b/spec/tags/core/hash/shift_tags.txt @@ -0,0 +1,2 @@ +fails:Hash#shift calls #default with nil if the Hash is empty +fails:Hash#shift returns (computed) default for empty hashes From 6b9156bc4bd7c1e815b824e8da4db3889dad11ad Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Thu, 6 Apr 2023 13:31:12 +0300 Subject: [PATCH 3/5] Add existing Hash#shift specs for the new behaviour to the :next specs list --- spec/truffleruby.mspec | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/truffleruby.mspec b/spec/truffleruby.mspec index a26f8459b21f..653e9fb5a2fa 100644 --- a/spec/truffleruby.mspec +++ b/spec/truffleruby.mspec @@ -104,6 +104,7 @@ class MSpecScript # Use spec/ruby/core/nil/nil_spec.rb as a dummy file to avoid being empty set :next, %w[ spec/ruby/core/nil/nil_spec.rb + spec/ruby/core/hash/shift_spec.rb ] set :tags_patterns, [ From 7056cbae876210d5fa44e0d8041105802e23c6de Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Thu, 6 Apr 2023 13:50:49 +0300 Subject: [PATCH 4/5] Mention the change in the changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6c9a40f7566..746d847f83e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Bug fixes: Compatibility: +- Fix `Hash#shift` when Hash is empty but has initial default value or initial default proc (@itarato). Performance: From a3973949214f7326d30fc0343cfb91e5b24976d6 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Thu, 6 Apr 2023 16:49:48 +0300 Subject: [PATCH 5/5] Exclude failing MRI Array tests The following test cases in test_array.rb fail now: - test_shift2 - test_shift_none --- test/mri/excludes/TestHash.rb | 49 ++++++++++------------- test/mri/excludes/TestHash/TestSubHash.rb | 49 ++++++++++------------- 2 files changed, 44 insertions(+), 54 deletions(-) diff --git a/test/mri/excludes/TestHash.rb b/test/mri/excludes/TestHash.rb index 5aed40d453d6..621136562c34 100644 --- a/test/mri/excludes/TestHash.rb +++ b/test/mri/excludes/TestHash.rb @@ -1,34 +1,29 @@ exclude :test_ar2st, "mutating Hash during key.hash" -exclude :test_ASET_fstring_key, "needs investigation" -exclude :test_ASET_fstring_non_literal_key, "needs investigation" -exclude :test_NEWHASH_fstring_key, "needs investigation" -exclude :test_callcc_escape, "needs investigation" -exclude :test_callcc_reenter, "needs investigation" -exclude :test_create, "needs investigation" -exclude :test_dup_equality, "needs investigation" -exclude :test_dup_will_rehash, "needs investigation" -exclude :test_fetch_error, "needs investigation" -exclude :test_huge_iter_level, "needs investigation" -exclude :test_inverse_hash, "needs investigation" -exclude :test_recursive_hash_value_struct, "needs investigation" -exclude :test_rehash2, "needs investigation" -exclude :test_reject, "needs investigation" -exclude :test_ruby2_keywords_hash?, "needs investigation" -exclude :test_s_AREF, "needs investigation" -exclude :test_callcc, "needs investigation" -exclude :test_callcc_iter_level, "needs investigation" -exclude :test_bug_12706, "needs investigation" -exclude :test_to_proc, "needs investigation" -exclude :test_transform_keys_bang, "needs investigation" -exclude :test_AREF_fstring_key, "needs investigation" -exclude :test_except, "needs investigation" -exclude :test_transform_keys, "needs investigation" +exclude :test_to_proc, " expected but was" +exclude :test_huge_iter_level, "RuntimeError expected but nothing was raised." +exclude :test_merge!, "FrozenError expected but nothing was raised." +exclude :test_create, "ArgumentError expected but nothing was raised." +exclude :test_callcc_reenter, "[ruby-dev:47803] [Bug #9105]." +exclude :test_select_reject_will_not_rehash, "<1> expected but was" +exclude :test_reject_on_identhash, "<{}> expected but was" exclude :test_select_on_identhash, "<{\"str\"=>1, \"str\"=>2}> expected but was" exclude :test_slice_on_identhash, "<{\"str\"=>1, \"str\"=>2}> expected but was" -exclude :test_reject_on_identhash, "<{}> expected but was" -exclude :test_select_reject_will_not_rehash, "<1> expected but was" -exclude :test_merge!, "FrozenError expected but nothing was raised." +exclude :test_shift2, "<:foo> expected but was" +exclude :test_NEWHASH_fstring_key, "Expected \"ABC\" (oid=4680) to be the same as \"ABC\" (oid=48)." +exclude :test_inverse_hash, "[ruby-core:34334]." +exclude :test_rehash2, "RuntimeError expected but nothing was raised." +exclude :test_shift_none, "<\"FOO\"> expected but was" exclude :test_update5, "FrozenError expected but nothing was raised." exclude :test_transform_values_on_identhash, "<{1=>2, 3=>4, 5=>6, \"str\"=>1, \"str\"=>2}> expected but was" exclude :test_s_AREF_from_pairs, "ArgumentError expected but nothing was raised." exclude :test_any_hash_fixable, "too slow" +exclude :test_recursive_hash_value_struct, "[ruby-core:58567] [Bug #9151]." +exclude :test_reject, "Expected 42 to be nil." +exclude :test_transform_keys_bang, "<{false=>1, :b=>2, :c=>3}> expected but was" +exclude :test_callcc_escape, "[ruby-dev:47803] [Bug #9105]." +exclude :test_ASET_fstring_key, "Expected \"abc\" (oid=39832) to be the same as \"abc\" (oid=39848)." +exclude :test_fetch_error, "Expected /\\.\\.\\.\\z/ to match \"key not found: \\\"gumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumby\\\"\"." +exclude :test_ASET_fstring_non_literal_key, "Expected \"abc_def\" (oid=40568) to be the same as \"abc_def\" (oid=40584)." +exclude :test_callcc, "RuntimeError: Continuations are unsupported on TruffleRuby" +exclude :test_AREF_fstring_key, "NoMethodError: undefined method `count_objects' for ObjectSpace:Module" +exclude :test_callcc_iter_level, "RuntimeError: Continuations are unsupported on TruffleRuby" diff --git a/test/mri/excludes/TestHash/TestSubHash.rb b/test/mri/excludes/TestHash/TestSubHash.rb index 5aed40d453d6..8081fadbb96b 100644 --- a/test/mri/excludes/TestHash/TestSubHash.rb +++ b/test/mri/excludes/TestHash/TestSubHash.rb @@ -1,34 +1,29 @@ exclude :test_ar2st, "mutating Hash during key.hash" -exclude :test_ASET_fstring_key, "needs investigation" -exclude :test_ASET_fstring_non_literal_key, "needs investigation" -exclude :test_NEWHASH_fstring_key, "needs investigation" -exclude :test_callcc_escape, "needs investigation" -exclude :test_callcc_reenter, "needs investigation" -exclude :test_create, "needs investigation" -exclude :test_dup_equality, "needs investigation" -exclude :test_dup_will_rehash, "needs investigation" -exclude :test_fetch_error, "needs investigation" -exclude :test_huge_iter_level, "needs investigation" -exclude :test_inverse_hash, "needs investigation" -exclude :test_recursive_hash_value_struct, "needs investigation" -exclude :test_rehash2, "needs investigation" -exclude :test_reject, "needs investigation" -exclude :test_ruby2_keywords_hash?, "needs investigation" -exclude :test_s_AREF, "needs investigation" -exclude :test_callcc, "needs investigation" -exclude :test_callcc_iter_level, "needs investigation" -exclude :test_bug_12706, "needs investigation" -exclude :test_to_proc, "needs investigation" -exclude :test_transform_keys_bang, "needs investigation" -exclude :test_AREF_fstring_key, "needs investigation" -exclude :test_except, "needs investigation" -exclude :test_transform_keys, "needs investigation" +exclude :test_to_proc, " expected but was" +exclude :test_huge_iter_level, "RuntimeError expected but nothing was raised." +exclude :test_merge!, "FrozenError expected but nothing was raised." +exclude :test_create, "ArgumentError expected but nothing was raised." +exclude :test_callcc_reenter, "[ruby-dev:47803] [Bug #9105]." +exclude :test_select_reject_will_not_rehash, "<1> expected but was" +exclude :test_reject_on_identhash, "<{}> expected but was" exclude :test_select_on_identhash, "<{\"str\"=>1, \"str\"=>2}> expected but was" exclude :test_slice_on_identhash, "<{\"str\"=>1, \"str\"=>2}> expected but was" -exclude :test_reject_on_identhash, "<{}> expected but was" -exclude :test_select_reject_will_not_rehash, "<1> expected but was" -exclude :test_merge!, "FrozenError expected but nothing was raised." +exclude :test_shift2, "<:foo> expected but was" +exclude :test_NEWHASH_fstring_key, "Expected \"ABC\" (oid=44424) to be the same as \"ABC\" (oid=48)." +exclude :test_inverse_hash, "[ruby-core:34334]." +exclude :test_rehash2, "RuntimeError expected but nothing was raised." +exclude :test_shift_none, "<\"FOO\"> expected but was" exclude :test_update5, "FrozenError expected but nothing was raised." exclude :test_transform_values_on_identhash, "<{1=>2, 3=>4, 5=>6, \"str\"=>1, \"str\"=>2}> expected but was" exclude :test_s_AREF_from_pairs, "ArgumentError expected but nothing was raised." exclude :test_any_hash_fixable, "too slow" +exclude :test_recursive_hash_value_struct, "[ruby-core:58567] [Bug #9151]." +exclude :test_reject, "Expected {1=>\"one\", 2=>false, true=>\"true\", \"cat\"=>99} to be an instance of Hash, not TestHash::TestSubHash::SubHash." +exclude :test_transform_keys_bang, "<{false=>1, :b=>2, :c=>3}> expected but was" +exclude :test_callcc_escape, "[ruby-dev:47803] [Bug #9105]." +exclude :test_ASET_fstring_key, "Expected \"abc\" (oid=84584) to be the same as \"abc\" (oid=84600)." +exclude :test_fetch_error, "Expected /\\.\\.\\.\\z/ to match \"key not found: \\\"gumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumbygumby\\\"\"." +exclude :test_ASET_fstring_non_literal_key, "Expected \"abc_def\" (oid=85320) to be the same as \"abc_def\" (oid=85336)." +exclude :test_callcc, "RuntimeError: Continuations are unsupported on TruffleRuby" +exclude :test_AREF_fstring_key, "NoMethodError: undefined method `count_objects' for ObjectSpace:Module" +exclude :test_callcc_iter_level, "RuntimeError: Continuations are unsupported on TruffleRuby"