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


Previously, the args rest array was modified, but that was changed in commit:299a13612e54accd9d3661bafde8f67142a78d54 due to the issues it caused.  Duping the array before modifying it should fix those issues, at the cost of an extra array allocation.

I don't have strong feelings about this change.  I'm not against it, so if you feel it is helpful, please commit it.

----------------------------------------
Bug #16466: `*args -> *args` delegation should be warned when the last hash has a `ruby2_keywords` flag
https://bugs.ruby-lang.org/issues/16466#change-83545

* Author: mame (Yusuke Endoh)
* Status: Open
* Priority: Normal
* Assignee: jeremyevans0 (Jeremy Evans)
* Target version: 
* ruby -v: 
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
(This ticket is derived from https://github.com/rails/rails/pull/38105#discussion_r361798251)

Currently, the following code displays no warnings.

```ruby
def baz(**kw)
end

def bar(*args)
  baz(*args)
end

ruby2_keywords def foo(*args)
  bar(*args)
end

foo(k: 1)
```

However, I think this code should be warned.  `bar` should be marked by `ruby2_keywords`.  This is because `ruby2_keywords` will be used as a mark to indicate "we need to rewrite this method from `def foo(*args)` to `def foo(*args, **kwargs)`" after `ruby2_keywords` is deprecated in far future.  If no warning is emitted for the code above, a user will rewrite `foo` as:

```
def foo(*args, *kwargs)
  bar(*args, *kwargs)
end
```

and will not modify the definitions of `bar` and `baz`.  Actually, the resulting code is broken; `bar` must be also modified like `foo`.


@jeremyevans0 Is this intentional?  If not, what do you think the following patch?

```diff
diff --git a/vm_args.c b/vm_args.c
index 7bf61cefe7..7077c7b3c1 100644
--- a/vm_args.c
+++ b/vm_args.c
@@ -801,6 +801,8 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
             if (RB_TYPE_P(rest_last, T_HASH) &&
                 (((struct RHash *)rest_last)->basic.flags & RHASH_PASS_AS_KEYWORDS)) {
                 rest_last = rb_hash_dup(rest_last);
+                arg_rest_dup(args);
+                RARRAY_ASET(args->rest, len - 1, rest_last);
                 kw_flag |= VM_CALL_KW_SPLAT;
                 if (iseq->body->param.flags.ruby2_keywords) {
                     remove_empty_keyword_hash = 0;
```

This patch prints the following warnings for the code above.

```
test.rb:5: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
test.rb:1: warning: The called method `baz' is defined here
```



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