[前][次][番号順一覧][スレッド一覧]

ruby-changes:57595

From: Jeremy <ko1@a...>
Date: Fri, 6 Sep 2019 12:55:12 +0900 (JST)
Subject: [ruby-changes:57595] d1ef73b59c (master): Always remove empty keyword hashes when calling methods

https://git.ruby-lang.org/ruby.git/commit/?id=d1ef73b59c

From d1ef73b59cede58f2173fa0f4ff7480a820f25d6 Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@j...>
Date: Thu, 5 Sep 2019 10:36:28 -0700
Subject: Always remove empty keyword hashes when calling methods

While doing so is not backwards compatible with Ruby 2.6, it is
necessary for generic argument forwarding to work for all methods:

```ruby
def foo(*args, **kw, &block)
  bar(*args, **kw, &block)
end
```

If you do not remove empty keyword hashes, and bar does not accept
keyword arguments, then a call to foo without keyword arguments
calls bar with an extra positional empty hash argument.

diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb
index 71b72bd..b337343 100644
--- a/test/ruby/test_keyword.rb
+++ b/test/ruby/test_keyword.rb
@@ -185,9 +185,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L185
 
     f = -> { true }
     assert_equal(true, f[**{}])
-    assert_warn(/The keyword argument is passed as the last hash parameter/m) do
-      assert_raise(ArgumentError) { f[**kw] }
-    end
+    assert_equal(true, f[**kw])
     assert_warn(/The keyword argument is passed as the last hash parameter/m) do
       assert_raise(ArgumentError) { f[**h] }
     end
@@ -203,9 +201,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L201
 
     f = ->(a) { a }
     assert_raise(ArgumentError) { f[**{}] }
-    assert_warn(/The keyword argument is passed as the last hash parameter/m) do
-      assert_equal(kw, f[**kw])
-    end
+    assert_raise(ArgumentError) { f[**kw] }
     assert_warn(/The keyword argument is passed as the last hash parameter/m) do
       assert_equal(h, f[**h])
     end
@@ -283,7 +279,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L279
       end
     end
     assert_equal([], c[**{}].args)
-    assert_equal([{}], c[**kw].args)
+    assert_equal([], c[**kw].args)
     assert_equal([h], c[**h].args)
     assert_equal([h], c[a: 1].args)
     assert_equal([h2], c[**h2].args)
@@ -294,7 +290,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L290
       def initialize; end
     end
     assert_nil(c[**{}].args)
-    assert_raise(ArgumentError) { c[**kw] }
+    assert_nil(c[**kw].args)
     assert_raise(ArgumentError) { c[**h] }
     assert_raise(ArgumentError) { c[a: 1] }
     assert_raise(ArgumentError) { c[**h2] }
@@ -307,7 +303,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L303
       end
     end
     assert_raise(ArgumentError) { c[**{}] }
-    assert_equal(kw, c[**kw].args)
+    assert_raise(ArgumentError) { c[**kw] }
     assert_equal(h, c[**h].args)
     assert_equal(h, c[a: 1].args)
     assert_equal(h2, c[**h2].args)
@@ -333,7 +329,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L329
       end
     end
     assert_raise(ArgumentError) { c[**{}] }
-    assert_equal([kw, kw], c[**kw].args)
+    assert_raise(ArgumentError) { c[**kw] }
     assert_equal([h, kw], c[**h].args)
     assert_equal([h, kw], c[a: 1].args)
     assert_equal([h2, kw], c[**h2].args)
@@ -365,7 +361,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L361
       args
     end
     assert_equal([], c.method(:m)[**{}])
-    assert_equal([{}], c.method(:m)[**kw])
+    assert_equal([], c.method(:m)[**kw])
     assert_equal([h], c.method(:m)[**h])
     assert_equal([h], c.method(:m)[a: 1])
     assert_equal([h2], c.method(:m)[**h2])
@@ -375,7 +371,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L371
     c.singleton_class.remove_method(:m)
     def c.m; end
     assert_nil(c.method(:m)[**{}])
-    assert_raise(ArgumentError) { c.method(:m)[**kw] }
+    assert_nil(c.method(:m)[**kw])
     assert_raise(ArgumentError) { c.method(:m)[**h] }
     assert_raise(ArgumentError) { c.method(:m)[a: 1] }
     assert_raise(ArgumentError) { c.method(:m)[**h2] }
@@ -387,7 +383,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L383
       args
     end
     assert_raise(ArgumentError) { c.method(:m)[**{}] }
-    assert_equal(kw, c.method(:m)[**kw])
+    assert_raise(ArgumentError) { c.method(:m)[**kw] }
     assert_equal(h, c.method(:m)[**h])
     assert_equal(h, c.method(:m)[a: 1])
     assert_equal(h2, c.method(:m)[**h2])
@@ -411,7 +407,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L407
       [arg, args]
     end
     assert_raise(ArgumentError) { c.method(:m)[**{}] }
-    assert_equal([kw, kw], c.method(:m)[**kw])
+    assert_raise(ArgumentError) { c.method(:m)[**kw] }
     assert_equal([h, kw], c.method(:m)[**h])
     assert_equal([h, kw], c.method(:m)[a: 1])
     assert_equal([h2, kw], c.method(:m)[**h2])
@@ -487,9 +483,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L483
       [arg, args]
     end
     assert_raise(ArgumentError) { c.send(:m, **{}) }
-    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
-      assert_equal([kw, kw], c.send(:m, **kw))
-    end
+    assert_raise(ArgumentError) { c.send(:m, **kw) }
     assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
       assert_equal([h, kw], c.send(:m, **h))
     end
@@ -576,9 +570,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L570
       [arg, args]
     end
     assert_raise(ArgumentError) { :m.to_proc.call(c, **{}) }
-    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
-      assert_equal([kw, kw], :m.to_proc.call(c, **kw))
-    end
+    assert_raise(ArgumentError) { :m.to_proc.call(c, **kw) }
     assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
       assert_equal([h, kw], :m.to_proc.call(c, **h))
     end
@@ -664,9 +656,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L656
       [arg, args]
     end
     assert_raise(ArgumentError) { c.m(**{}) }
-    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
-      assert_equal([kw, kw], c.m(**kw))
-    end
+    assert_raise(ArgumentError) { c.m(**kw) }
     assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
       assert_equal([h, kw], c.m(**h))
     end
@@ -707,9 +697,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L697
       define_method(:m) { }
     end
     assert_nil(c.m(**{}))
-    assert_warn(/The keyword argument is passed as the last hash parameter/m) do
-      assert_raise(ArgumentError) { c.m(**kw) }
-    end
+    assert_nil(c.m(**kw))
     assert_warn(/The keyword argument is passed as the last hash parameter/m) do
       assert_raise(ArgumentError) { c.m(**h) }
     end
@@ -731,9 +719,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L719
       define_method(:m) {|arg| arg }
     end
     assert_raise(ArgumentError) { c.m(**{}) }
-    assert_warn(/The keyword argument is passed as the last hash parameter/m) do
-      assert_equal(kw, c.m(**kw))
-    end
+    assert_raise(ArgumentError) { c.m(**kw) }
     assert_warn(/The keyword argument is passed as the last hash parameter/m) do
       assert_equal(h, c.m(**h))
     end
@@ -779,9 +765,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L765
       define_method(:m) {|arg, **opt| [arg, opt] }
     end
     assert_raise(ArgumentError) { c.m(**{}) }
-    assert_warn(/The keyword argument is passed as the last hash parameter/m) do
-      assert_equal([kw, kw], c.m(**kw))
-    end
+    assert_raise(ArgumentError) { c.m(**kw) }
     assert_warn(/The keyword argument is passed as the last hash parameter/m) do
       assert_equal([h, kw], c.m(**h))
     end
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index baf93de..4ae94e3 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1740,7 +1740,7 @@ rb_iseq_only_kwparam_p(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1740
 static inline void
 CALLER_SETUP_ARG(struct rb_control_frame_struct *restrict cfp,
                  struct rb_calling_info *restrict calling,
-                 const struct rb_call_info *restrict ci, int remove_empty_keyword_hash)
+                 const struct rb_call_info *restrict ci)
 {
     if (UNLIKELY(IS_ARGS_SPLAT(ci))) {
         /* This expands the rest argument to the stack.
@@ -1755,9 +1755,14 @@ CALLER_SETUP_ARG(struct rb_control_frame_struct *restrict cfp, https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1755
          */
         vm_caller_setup_arg_kw(cfp, calling, ci);
     }
-    if (UNLIKELY(calling->kw_splat && remove_empty_keyword_hash)) {
+    if (UNLIKELY(calling->kw_splat)) {
         /* This removes the last Hash object if it is empty.
          * So, ci->flag & VM_CALL_KW_SPLAT is now inconsistent.
+         * However, you can use ci->flag & VM_CALL_KW_SPLAT to
+         * determine whether a hash should be added back with
+         * warning (for backwards compatibility in cases where
+         * the method does not have the number of required
+         * arguments.
          */
         if (RHASH_EMPTY_P(cfp->sp[-1])) {
             cfp-> (... truncated)

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]