From: "Eregon (Benoit Daloze)" <noreply@...>
Date: 2022-03-22T10:52:28+00:00
Subject: [ruby-core:108019] [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 Eregon (Benoit Daloze).


Yes, that's correct, and that code would also break when migrating to other form of delegations (if not changing `bar` too).

I don't understand why we had these semantics in the first place, I think there are hard to understand for anyone (it seems it was [unintentional](https://bugs.ruby-lang.org/issues/16466#note-1)).
They might lead people to think the `ruby2_keywords` flag is kept forever, but that is not the case.
The rule is the `ruby2_keywords` flag is "reset" (i.e., the Hash in the callee is not flagged) once used, e.g., on `foo(*flagged_args)`.
This always holds, except for the case where the callee has a rest arg (this issue).

@mame noticed as well and I said it back just after the 2.7 release that we should have fixed this (it would probably have been easier), but it seems I was not heard: #16466.
I believe this is worth fixing, for simpler semantics and for being able to migrate to other forms of delegations without major troubles.
It is easier to add some `ruby2_keywords` now than while migrating to `(*args, **kwargs)`/`(...)` figure out several `ruby2_keywords` were missing and have to review all places which do delegation.
It is likely some gems which do lots of delegation might miss a few `ruby2_keywords`, like [rspec-mocks does](https://github.com/rspec/rspec-mocks/pull/1464).
Adding missing `ruby2_keywords` is not particularly hard, adding `puts caller` at the end is enough to find all of them along a delegation chain.

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

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