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/

Unsubscribe: <mailto:ruby-core-request / ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>