Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[3.2 compatibility] Make an empty hash shift return nil #2980

Merged
merged 5 commits into from
Apr 11, 2023
Merged

[3.2 compatibility] Make an empty hash shift return nil #2980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4cb3c44
Make an empty hash shift return nil.
itarato Apr 3, 2023
3a40486
Tag Hash#shift specs for old behaviour as failed
andrykonchin Apr 6, 2023
6b9156b
Add existing Hash#shift specs for the new behaviour to the :next spec…
andrykonchin Apr 6, 2023
7056cba
Mention the change in the changelog
andrykonchin Apr 6, 2023
a397394
Exclude failing MRI Array tests
andrykonchin Apr 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
2 changes: 2 additions & 0 deletions spec/tags/core/hash/shift_tags.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions spec/truffleruby.mspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, [
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/truffleruby/core/hash/HashNodes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()")
Expand Down
49 changes: 22 additions & 27 deletions test/mri/excludes/TestHash.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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, "<true> 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"
49 changes: 22 additions & 27 deletions test/mri/excludes/TestHash/TestSubHash.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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, "<true> 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"