[ruby-core:98486] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[ruby-core:98486] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[samuel]

Issue #16908 has been reported by ioquatix (Samuel Williams).

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:98487] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[merch-redmine]

Issue #16908 has been updated by jeremyevans0 (Jeremy Evans).


While your particular example is non-intuitive, there is a simple explanation for it. The first time `hash.shift` is called, `hash` is empty, so it returns the default value (`0`).  It gets the default value by calling the default_proc for `hash` with a `nil` key.  There is no better option since `hash.shift` isn't provided a key.  The second time `hash.shift` is called, the hash is not empty, so it returns the first entry as a key value pair.  I agree `Hash#shift` semantics with a default_proc are questionable, but I'm not sure if it could be improved.

I don't think we should change this behavior.  It is expected that `Hash.new.shift` should return nil, as should `Hash.new(nil).shift` and `Hash.new{}.shift`.

`hash.shift` is used in conditionals:

```ruby
hash = {a: 1, b: 2}
while (k,v = hash.shift)
  p [k, v]
end
```

If you change `Hash#shift` to return an array when the hash is empty, you've turned this into an infinite loop.

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-85763

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:98488] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[samuel]

Issue #16908 has been updated by ioquatix (Samuel Williams).


@jeremyevans0 I agree with your assessment, however that hash does not have default_proc so I assume that it would return `nil` after all key-value pairs are `shift`ed out. That's different from invoking `default_proc` and returning `[nil, 0]`.

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-85764

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:98489] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[merch-redmine]

Issue #16908 has been updated by jeremyevans0 (Jeremy Evans).


ioquatix (Samuel Williams) wrote in #note-2:
> @jeremyevans0 I agree with your assessment, however that hash does not have default_proc so I assume that it would return `nil` after all key-value pairs are `shift`ed out. That's different from invoking `default_proc` and returning `[nil, 0]`.

Same principle applies when using a default_proc:

```ruby
# simple hash with indifferent access
hash = Hash.new{|h,k| h[k.to_s] unless k.is_a? String}
hash['a'] = 1
hash['b'] = 2
while (k,v = hash.shift)
  p [k, v]
end
```

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-85765

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:98491] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[eregontp]

Issue #16908 has been updated by Eregon (Benoit Daloze).


Maybe `Hash#shift` should not call the `default_proc` or use `Hash#default`?
I.e., it would always return `nil` if `Hash#empty?`.
I think that would be more intuitive and probably compatible enough.

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-85767

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:98492] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[eregontp]

Issue #16908 has been updated by Eregon (Benoit Daloze).


In other words, the current semantics of `return hash.default(nil) if hash.empty?` feel hacky and actually harmful to me.
The user probably never expects to have the default_proc called with a `nil` key in many cases.

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-85768

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:98493] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[daniel]

Issue #16908 has been updated by Dan0042 (Daniel DeLorme).


I would expect Hash#shift to return either a key-value tuple or nil. Returning the default value is, honestly, incomprehensible.

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-85770

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:102900] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[matz]

Issue #16908 has been updated by matz (Yukihiro Matsumoto).


I don't remember why I made this behavior. Now I think `#shift` should return `nil` for an empty hash, without calling its default value, in the long run. @naruse claims the change should be postponed to `3.2` or later if we make the change.

Matz.

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-90961

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:105439] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[ioquatix (Samuel Williams)]

Issue #16908 has been updated by ioquatix (Samuel Williams).


Should we introduce some kind of deprecation or warning in 3.1?

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-93886

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:106430] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[naruse (Yui NARUSE)]

Issue #16908 has been updated by naruse (Yui NARUSE).


ioquatix (Samuel Williams) wrote in #note-8:
> Should we introduce some kind of deprecation or warning in 3.1?

Not allowed.
Ruby 3.1 shouldn't introduce anything which requests application developers to change something in their application code.
Deprecation warning is just a way of communications to request application developers to fix their application code.

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-95072

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:106855] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[jeremyevans0 (Jeremy Evans)]

Issue #16908 has been updated by jeremyevans0 (Jeremy Evans).


I've submitted a pull request to make `Hash#shift` return `nil` if the hash is empty: https://github.com/ruby/ruby/pull/5360

Not sure if we want that behavior in 3.2, or if we want to issue a deprecation warning in 3.2 and change in 3.3.  Considering there would be no way to avoid the deprecation warning if the hash has a default value, I think it's best to just change the behavior without deprecation.

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-95665

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:106938] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[ioquatix (Samuel Williams)]

Issue #16908 has been updated by ioquatix (Samuel Williams).


Pretty much anything will be better than the current behaviour. I think your proposal makes sense.

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-95761

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:107113] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[mame (Yusuke Endoh)]

Issue #16908 has been updated by mame (Yusuke Endoh).

Assignee set to jeremyevans0 (Jeremy Evans)
Status changed from Open to Assigned

Jeremy's approach (make Hash#shift return nil if the hash is empty) was approved by @matz.

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-95959

* Author: ioquatix (Samuel Williams)
* Status: Assigned
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/

[ruby-core:107135] [Ruby master Bug#16908] Strange behaviour of Hash#shift when used with `default_proc`.

[ioquatix (Samuel Williams)]

Issue #16908 has been updated by ioquatix (Samuel Williams).


Thank you @jeremyevans

----------------------------------------
Bug #16908: Strange behaviour of Hash#shift when used with `default_proc`.
https://bugs.ruby-lang.org/issues/16908#change-95982

* Author: ioquatix (Samuel Williams)
* Status: Closed
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* ruby -v: 2.7.0
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
I don't have any strong opinion about this, but I observed the following behaviour which I thought was confusing. Maybe it's okay, or maybe we should change it to be more consistent.

```
hash = Hash.new{|k,v| k[v] = 0}

hash.shift # => 0
hash.shift # => [nil, 0]
```

My feeling was, both cases should return `[nil, 0]`.




-- 
https://bugs.ruby-lang.org/