From: "jeremyevans0 (Jeremy Evans)" <noreply@...>
Date: 2022-03-11T23:21:20+00:00
Subject: [ruby-core:107850] [Ruby master Bug#18625] ruby2_keywords does not unmark the hash if the receiving method has a *rest parameter

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


Based on @matz's comments (https://bugs.ruby-lang.org/issues/16466#note-3), I do not think this is a bug.  Splatted arguments where the final argument is a flagged hash will have the hash converted to keywords if the method accepts keywords.  If the method does not accept keywords, I don't think the behavior is specified.

I think this change is more consistent than the current code.  I cannot think of a justification why `single(*args)` would return an unflagged copy, but `splat(*args)` would return the flagged hash.  I think there are valid arguments for both returning flagged hash, with the argument that flagged hashes only affect methods that accept keywords, and other methods they are passed straight through. I also think this change returning an unflagged copy is justifiable, with the argument that `foo(*args)` where the final element of `args` is a flagged hash should always be treated as `foo(*args[0...-1], **args[-1])`, regardless of the definition of `foo`. 

I recommend that we accept this change for 3.2.  I'm not sure we should backport this to 3.0 or 3.1, since it could break existing code (unlikely, but possible).

In case this change is considered desirable (either as a bug fix or as a deliberate feature change), I have submitted a pull request for it: https://github.com/ruby/ruby/pull/5645 .  The pull request does not require any additional allocations. 

----------------------------------------
Bug #18625: ruby2_keywords does not unmark the hash if the receiving method has a *rest parameter
https://bugs.ruby-lang.org/issues/18625#change-96790

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
The code below shows the inconsistency.
In all cases the `marked` Hash is copied at call sites using `some_call(*args)`, however for the case of `splat` it keeps the ruby2_keywords flag to true, and not false as expected.
This can be observed in user code and will hurt migration from `ruby2_keywords` to other ways of delegation (`(...)` and `(*args, **kwargs)`).
I believe this is another manifestation of #16466.

```ruby
ruby2_keywords def foo(*args)
  args
end

def single(arg)
  arg
end

def splat(*args)
  args.last
end

def kwargs(**kw)
  kw
end

h = { a: 1 }
args = foo(**h)
marked = args.last
Hash.ruby2_keywords_hash?(marked) # => true

after_usage = single(*args)
after_usage == h # => true
after_usage.equal?(marked) # => false
p Hash.ruby2_keywords_hash?(after_usage) # => false

after_usage = splat(*args)
after_usage == h # => true
after_usage.equal?(marked) # => false
p Hash.ruby2_keywords_hash?(after_usage) # => true, BUG, should be false

after_usage = kwargs(*args)
after_usage == h # => true
after_usage.equal?(marked) # => false
p Hash.ruby2_keywords_hash?(after_usage) # => false

Hash.ruby2_keywords_hash?(marked) # => true
```

I'm implementing Ruby 3 kwargs in TruffleRuby and this came up as an inconsistency in specs.
In TruffleRuby it's also basically not possible to implement this behavior, because at a splat call site where we check for a last Hash argument marked as ruby2_keywords, we have no idea of which method will be called yet, and so cannot differentiate behavior based on that.

cc @jeremyevans0 @mame



-- 
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>