Project

General

Profile

Actions

Bug #17757

closed

Hash#slice does not keep compare_by_identity on the results

Added by kachick (Kenichi Kamiya) about 3 years ago. Updated over 2 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin20]
[ruby-core:103071]

Description

GH-PR: https://github.com/ruby/ruby/pull/4330

$ ruby -v
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin20]
str1 = +'str'
str2 = +'str'
hash = {a: :a, b: :b, c: :c}.compare_by_identity
hash[str1] = 1
hash[str2] = 2
p hash.values_at(str1, str2) #=> [1, 2]
p hash.except.compare_by_identity? #=> true
p hash.slice.compare_by_identity? #=> false
p hash.except(str1, str2) #=> {:a=>:a, :b=>:b, :c=>:c}
p hash.slice(str1, str2) #=> {"str"=>2}

Is this an intentional behavior?
I would expect Hash#slice keeps compare_by_identity behaviors.

Updated by marcandre (Marc-Andre Lafortune) about 3 years ago

I should have looked into this when I checked #16996.

Looks like we have behavior that is all over the place, and the same method sometimes returns a hash with different comparability depending if the receiver is empty or not...

ruby 3.1.0dev (2021-03-28T18:18:08Z master 1cdecb4349) [x86_64-darwin18]
slice with arg: false
slice without arg: false
except with arg: true but false for {}
except without arg: true but false for {}
dup: true
select: true but false for {} # in 3.0: false
reject: true but false for {} # in 3.0: false
merge: true but false for {}
transform_values: true but false for {}
transform_keys: false
to_h: false
to_h without block: true

Script:

def check(method, label = method, *args, &block)
  ci1, ci2 = [{a: 1}, {}].map do |h|
    h.compare_by_identity
    h2 = h.send(method, &block)
    h2.compare_by_identity?
  end
  s = "#{label}: #{ci1}"
  s << " but #{ci2} for {}" if ci2 != ci1
  puts s
end

check(:slice, 'slice with arg', :a)
check(:slice, 'slice without arg')
check(:except, 'except with arg', :a)
check(:except, 'except without arg')
%i[dup select reject merge transform_values transform_keys to_h].each do |method|
  check(method) { |k, v| [k, v] }
end
check(:to_h, 'to_h without block')

It's probably clear that the result should always be consistent for empty hash or not.

I feel the flag should be conserved for all these examples, except to_h {...} which should create an array without comparison by identity (as it does currently).

Updated by jeremyevans0 (Jeremy Evans) over 2 years ago

I've submitted a pull request that builds on @kachick's PR and fixes the remaining cases found by @marcandre (Marc-Andre Lafortune): https://github.com/ruby/ruby/pull/4616

Updated by marcandre (Marc-Andre Lafortune) over 2 years ago

Thanks for the PR @jeremyevans0 (Jeremy Evans), looks pretty good.

On the other hand, I'd like to revise what I wrote and suggest that transform_keys remain untouched. It's the only one of these methods (with to_h{...}) that changes the keys so has a reason to interpret them differently.

Matz, do you agree?

Updated by matz (Yukihiro Matsumoto) over 2 years ago

Yes, we must keep compare_by_identity status for those methods. Accepted.

Matz.

Actions #5

Updated by jeremyevans (Jeremy Evans) over 2 years ago

  • Status changed from Open to Closed

Applied in changeset git|95f8ffa5f6c70aa9383e1f6db02b22707c183402.


Copy hash compare_by_identity setting in more cases

This makes the compare_by_identity setting always copied
for the following methods:

  • except
  • merge
  • reject
  • select
  • slice
  • transform_values

Some of these methods did not copy the setting, or only
copied the setting if the receiver was not empty.

Fixes [Bug #17757]

Co-authored-by: Kenichi Kamiya

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0