[ruby-core:100192] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[ruby-core:100192] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[marcandre-ruby-core]

Issue #17197 has been reported by marcandre (Marc-Andre Lafortune).

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100194] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[marcandre-ruby-core]

Issue #17197 has been updated by marcandre (Marc-Andre Lafortune).


Also, as noted by @boblail, `Hash#map` accepts both arity 1 and 2; shouldn't it accept only arity 1?

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-87765

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100196] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[eregontp]

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


marcandre (Marc-Andre Lafortune) wrote in #note-1:
> Also, as noted by @boblail, `Hash#map` accepts both arity 1 and 2; shouldn't it accept only arity 1?

Agreed, especially since Hash#each changed to always yield a single argument.

I think it was just the same implementation as for Hash#each that would check the given Proc's arity.

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-87767

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100197] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[eregontp]

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


`#map` is defined on Enumerable.

So this seems an unintended interaction between
Hash#each using `rb_block_pair_yield_optimizable()`:
https://github.com/ruby/ruby/blob/245ed57ddc93901e90388cf479968392299d1067/hash.c#L3158-L3159
and Enumerable#map using `rb_block_min_max_arity` and `rb_yield_values2`.
https://github.com/ruby/ruby/blob/245ed57ddc93901e90388cf479968392299d1067/enum.c#L547-L587

So as result that optimization seems to still not be transparent and changes semantics.

`rb_block_pair_yield_optimizable()` returns false for lambda though, and `#map` uses `rb_lambda_call()` so not sure what goes wrong there.

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-87768

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100516] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[marcandre-ruby-core]

Issue #17197 has been updated by marcandre (Marc-Andre Lafortune).


@nobu would it be possible to address this before preview-2?

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-88139

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100867] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[mame]

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


This story is rather different from Hash#each in #14015.

`Hash#each` has yielded an array that has a key and a value since a long time ago:

```
# 2.7
{ a: 1 }.each {|a| p a } #=> [:a, 1]
```

BTW, `Hash#each` has an optimization to skip array generation if the arity of a given block is 2. However, this optimization had overlooked the case where the block is lambda. This is a bug of optimization. #14015 pointed out this issue, and we fixed it:

```
# 2.7
{ a: 1 }.each(&->(k, v){ p k }) #=> :a # the bug that #14015 pointed out

# the current master
{ a: 1 }.each(&->(k, v){ p k }) #=> wrong number of arguments (given 1, expected 2)
```

This is incompatible, but it changes only the case where a lambda block is passed to `Hash#each` (which is relatively rare, I think), so I (hopefully) guess it is acceptable, and we have no bug report about this since preview-1.

On the other hand, `Hash#select` has yielded two elements since Ruby 1.9.0:

```
# 2.7
{ a: 1 }.select {|k| p k } #=> :a

# this proposal
{ a: 1 }.select {|k| p k } #=> [:a, 1]
```

IMO, this is a bug since 1.9.0 because Hash#select looks like a faster version of Enumerable#select, so it should behave as possible as like Enumerable#select. However, the behavior has been longly accepted anyway, and passing a plain (non-lambda) block to Hash#select is the main use case, so fixing this may have a bigger impact than `Hash#each`.

In short: I like the proposal, but the imcompatibility would be much bigger than #14015.

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-88514

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100875] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[daniel]

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


So when you have a bug that is troublesome to fix because it may result in large incompatibility... rather than "just fix it" and wait to see if other people's code explodes, wouldn't the Industry-Standard Best Practice™ here be to just add a deprecation warning?

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-88522

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100880] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[eregontp]

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


@Dan0042 How would we warn that case? Check if the block uses arity 1 and warn that it should instead use `|k,v|` or `|k,|`?

I think we should fix `Hash#map` at least, that's also a clear optimization bug like `Hash#each`, and it forces other Ruby implementations to replicate the bug.
(e.g., https://github.com/oracle/truffleruby/issues/1944)

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-88528

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100899] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[daniel]

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


Eregon (Benoit Daloze) wrote in #note-8:
> Check if the block uses arity 1 and warn that it should instead use `|k,v|` or `|k,|`?

Yes. It seems obvious, am I missing something? I'm aware that `proc{ |k,| }.arity == 1` (imho a bug) but internally it's possible to tell the difference between `|k|` and `|k,|` otherwise they would have the same behavior.

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-88551

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100900] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[mame]

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


Ah, I was wrong.

mame (Yusuke Endoh) wrote in #note-6:
> IMO, this is a bug since 1.9.0 because Hash#select looks like a faster version of Enumerable#select, so it should behave as possible as like Enumerable#select.

Hash#select returns a hash instead of an array, so Hash#select is not a simple faster variant of Enumerable#select. So it don't necessarily have to follow the arity.


Just for the record: I found one affected use case of `Hash#select` with one-arity block in [a popular gem](https://rubygems.org/gems/docker-api):

https://github.com/swipely/docker-api/blob/1e9b9cc5f0f38dcd54c18189812328bb802d3656/lib/docker/container.rb#L327

```
  def self.create(opts = {}, conn = Docker.connection)
    query = opts.select {|key| ['name', :name].include?(key) }
```

It is very difficult to find this kind of cases by gem-codesearch, so I guess there are more cases.

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-88552

* Author: marcandre (Marc-Andre Lafortune)
* Status: Open
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100964] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[matz]

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

Status changed from Open to Rejected

It is caused by a historical reason. I don't think it's worth breaking compatibility, at least for Ruby3.0. Maybe for 3.1?

Matz.


----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-88624

* Author: marcandre (Marc-Andre Lafortune)
* Status: Rejected
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:100968] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[marcandre-ruby-core]

Issue #17197 has been updated by marcandre (Marc-Andre Lafortune).


matz (Yukihiro Matsumoto) wrote in #note-11:
> It is caused by a historical reason. I don't think it's worth breaking compatibility, at least for Ruby3.0. Maybe for 3.1?
> 
> Matz.

Thanks for the reply. Should we add a deprecation warning then?

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-88629

* Author: marcandre (Marc-Andre Lafortune)
* Status: Rejected
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:101092] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[matz]

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


@marcandre No. Issuing deprecation warnings itself is a declaration of future change. I haven't set my mind for either direction. Actually slightly leaning toward no change.

Matz.


----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-88767

* Author: marcandre (Marc-Andre Lafortune)
* Status: Rejected
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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

[ruby-core:101166] [Ruby master Bug#17197] Some Hash methods still have arity 2 instead of 1

[daniel]

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


It doesn't have to be a deprecation warning though. But I think that in the same sense as the "method redefined" warning indicates a potential problem, `hash.select{|x|...}` is likely to have a surprising result, and that's worth a warning.

----------------------------------------
Bug #17197: Some Hash methods still have arity 2 instead of 1
https://bugs.ruby-lang.org/issues/17197#change-88852

* Author: marcandre (Marc-Andre Lafortune)
* Status: Rejected
* Priority: Normal
* Assignee: nobu (Nobuyoshi Nakada)
* ruby -v: ruby 3.0.0dev (2020-09-26T17:38:39Z master 950614b088)
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
`Hash#each` was changed recently to have arity of 1.
All other methods of `Hash` should behave the same.
Much has been fixed since #14015, but some remains:

```ruby
# Some methods consistently have arity 2:
{a: 1}.select( &->(_kvp) {} ) # => ArgumentError (wrong number of arguments (given 2, expected 1))
```

All in all: `%i[select keep_if delete_if reject to_h]` have their arity still set at 2.




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