[ruby-core:96687] [Ruby master Feature#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[ruby-core:96687] [Ruby master Feature#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[mame]

Issue #16486 has been reported by mame (Yusuke Endoh).

----------------------------------------
Feature #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486

* Author: mame (Yusuke Endoh)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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

[ruby-core:96689] [Ruby master Bug#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[merch-redmine]

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


I'm fine with adding these methods, and I agree that making them class methods instead of instance methods discourages casual use.

----------------------------------------
Bug #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486#change-83673

* Author: mame (Yusuke Endoh)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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

[ruby-core:96691] [Ruby master Bug#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[eregontp]

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


+1, sounds good to me as class methods.

Could also be under ExperimentalFeatures, but not a great fit for this.

----------------------------------------
Bug #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486#change-83676

* Author: mame (Yusuke Endoh)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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

[ruby-core:96696] [Ruby master Bug#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[mame]

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

Assignee set to matz (Yukihiro Matsumoto)
Status changed from Open to Assigned

@jeremyevans0 @eregon Thanks, I put this on the next dev meeting.

----------------------------------------
Bug #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486#change-83681

* Author: mame (Yusuke Endoh)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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

[ruby-core:96703] [Ruby master Bug#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[daniel]

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


I would like to recommend this be a non-mutating method. Instead of `Hash.ruby2_keywords!(hash)` it should be something like `hash = Hash.as_ruby2_keywords(hash)` to prevent misuse.

----------------------------------------
Bug #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486#change-83689

* Author: mame (Yusuke Endoh)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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

[ruby-core:96710] [Ruby master Bug#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[eregontp]

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


@Dan0042 Very good point, I agree there should be no way to change that flag inplace, since there isn't currently (`**h` will copy Hash).
Knowing that the flag can never change for a given Hash instance is a useful guarantee, e.g., when debugging.

----------------------------------------
Bug #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486#change-83698

* Author: mame (Yusuke Endoh)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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

[ruby-core:96722] [Ruby master Bug#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[mame]

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


A memo for the dev-meeting discussion

* What we should provide as `Hash.ruby2_keywords!`? mutating version, non-mutating version, or both?
* Should it raise a FrozenError if a hash is frozen?

----------------------------------------
Bug #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486#change-83712

* Author: mame (Yusuke Endoh)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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

[ruby-core:96733] [Ruby master Bug#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[eregontp]

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


Copying from the discussion of the PR:

> @eregon IMO, the mutating version is enough here because it is not for a casual use; in the ActiveJob case, mutating version is slightly preferable (as @kamipo said), and currently there is no known use case where non-mutating version is preferable. But I'll discuss the behavior (and method name) at the next dev-meeting.

The problem is this tiny decision completely prevents other implementations or designs for ruby2_keywords. For instance, if we wanted to try using a KwHash subclass instead of a flag, as suggested in https://bugs.ruby-lang.org/issues/16463#note-20, the sole existence of `Hash.ruby2_keywords!` prevents it entirely, or would require to change the class of an existing object which is extremely ugly (for semantics at least).
It also makes debugging significantly harder IMHO.

So I think making it mutating here is needlessly restricting, and I oppose it.
Making a copy of the Hash is not a big cost when it's deserialized before.

Maybe we should not even introduce a new way to flag, as it's so easy to do it manually:
```ruby
ruby2_keywords def flag_kwhash(*args)
  args.last
end

kw = flag_kwhash(**h)
```

----------------------------------------
Bug #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486#change-83719

* Author: mame (Yusuke Endoh)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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

[ruby-core:96835] [Ruby master Bug#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[mame]

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


Hi, I talked about this ticket with ko1, nobu, and znz before the dev-meeting.  After the discussion, I am still against this.

We premise that the ruby2_keywords flag is never a wonderful thing.  We want to delete it in the future.  `Module#ruby2_keywords` will serve as a mark showing "we need to change this method definition so to accept not only positional rest arguments but also keyword ones explicitly if ruby2_keywords flag is deleted."

So, assuming the flag is removed at Ruby 4.0, we'd like users to change their code in the following style:

```
# 2.6
def foo(*args)
  bar(*args)
end
```

```
# 2.7 .. 4.0 (until ruby2_keywords flag is deprecated)
ruby2_keywords def foo(*args)
  bar(*args)
end
```

```
# 4.0 ..
def foo(*args, **kwargs)
  bar(*args, **kwargs)
end

# or, if possible
def foo(...)
  bar(...)
end
```

If we set `ruby2_keywords` by default, users cannot identify where to fix, and their code will break suddenly after the flag is removed.  It would be unacceptable and we will be unable to deprecate `Module#ruby2_keywords` forever.  This is not what we want.

----------------------------------------
Bug #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486#change-83838

* Author: mame (Yusuke Endoh)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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

[ruby-core:96838] [Ruby master Bug#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[mame]

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


Please ignore the previous comment. I took the wrong ticket.  It should have been written in #16463.

----------------------------------------
Bug #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486#change-83841

* Author: mame (Yusuke Endoh)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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

[ruby-core:96877] [Ruby master Bug#16486] Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)

[matz]

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


I propose

* Hash.ruby_keywords_hash(hash) to turn the flag on (non mutating)
* Hash.ruby_keywords_hash?(hash) to check the flag

Matz.

----------------------------------------
Bug #16486: Hash.ruby2_keywords?(hash) and Hash.ruby2_keywords!(hash)
https://bugs.ruby-lang.org/issues/16486#change-83890

* Author: mame (Yusuke Endoh)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED, 2.7: REQUIRED
----------------------------------------
Hash's ruby2_keywords flag is designed to be as implicit as possible, but unfortunately, sometimes we need to handle the flag explicitly.  In ActiveJob, the whole arguments are serialized and deserialized, and the process removes the flag; in this case, we need to check and save the flag when serializing, and restore the flag when deserializing.

https://github.com/rails/rails/pull/38105#discussion_r361863767

It is theoretically possible by [a very hacky code](https://gist.github.com/mame/18cd2dfc7d965d0bad1216f2fdd008ee#file-bouncing_test-patch-L117-L134), but it would be good to provide them officially.

https://github.com/ruby/ruby/pull/2818

* `Hash.ruby2_keywords?(hash)` checks if hash is flagged or not.
* `Hash.ruby2_keywords!(hash)` flags a given hash.

The reason why I don't add them as instance methods (`Hash#ruby2_keywords?`) is that they are never for casual use.  The ruby2_keywords flag will be removed after enough migration time.  Casual use of them will make it difficult to remove, so I'd like to keep them for non-casual use.

(I thought `RubyVM.ruby2_keywords?(hash)` is good but @eregon will be against it :-)

@jeremyevans0 Could you tell me your opinion?



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