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

ruby-changes:57563

From: Jeremy <ko1@a...>
Date: Fri, 6 Sep 2019 01:58:02 +0900 (JST)
Subject: [ruby-changes:57563] 1d5066efb0 (master): Make m(**{}) mean call without keywords

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

From 1d5066efb08cbb328ba528a5f8be1708584b659f Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@j...>
Date: Wed, 4 Sep 2019 10:54:12 -0700
Subject: Make m(**{}) mean call without keywords

Previously, **{} was removed by the parser:

```
$ ruby --dump=parse -e '{**{}}'
 @ NODE_SCOPE (line: 1, location: (1,0)-(1,6))
 +- nd_tbl: (empty)
 +- nd_args:
 |   (null node)
 +- nd_body:
     @ NODE_HASH (line: 1, location: (1,0)-(1,6))*
     +- nd_brace: 1 (hash literal)
     +- nd_head:
         (null node)
```

Since it was removed by the parser, the compiler did not know
about it, and `m(**{})` was therefore treated as `m()`.

This modifies the parser to not remove the `**{}`.  A simple
approach for this is fairly simple by just removing a few
lines from the parser, but that would cause two hash
allocations every time it was used.  The approach taken here
modifies both the parser and the compiler, and results in `**{}`
not allocating any hashes in the usual case.

The basic idea is we use a literal node in the parser containing
a frozen empty hash literal.  In the compiler, we recognize when
that is used, and if it is the only keyword present, we just
push it onto the VM stack (no creation of a new hash or merging
of keywords).  If it is the first keyword present, we push a
new empty hash onto the VM stack, so that later keywords can
merge into it.  If it is not the first keyword present, we can
ignore it, since the there is no reason to merge an empty hash
into the existing hash.

Example instructions for `m(**{})`

Before (note ARGS_SIMPLE):

```
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,7)> (catch: FALSE)
0000 putself                                                          (   1)[Li]
0001 opt_send_without_block       <callinfo!mid:m, argc:0, FCALL|ARGS_SIMPLE>, <callcache>
0004 leave
```

After (note putobject and KW_SPLAT):

```
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,7)> (catch: FALSE)
0000 putself                                                          (   1)[Li]
0001 putobject                    {}
0003 opt_send_without_block       <callinfo!mid:m, argc:1, FCALL|KW_SPLAT>, <callcache>
0006 leave
```

Example instructions for `m(**h, **{})`

Before and After (no change):

```
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,12)> (catch: FALSE)
0000 putself                                                          (   1)[Li]
0001 putspecialobject             1
0003 newhash                      0
0005 putself
0006 opt_send_without_block       <callinfo!mid:h, argc:0, FCALL|VCALL|ARGS_SIMPLE>, <callcache>
0009 opt_send_without_block       <callinfo!mid:core#hash_merge_kwd, argc:2, ARGS_SIMPLE>, <callcache>
0012 opt_send_without_block       <callinfo!mid:m, argc:1, FCALL|KW_SPLAT>, <callcache>
0015 leave
```

Example instructions for `m(**{}, **h)`

Before:

```
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,12)> (catch: FALSE)
0000 putself                                                          (   1)[Li]
0001 putspecialobject             1
0003 newhash                      0
0005 putself
0006 opt_send_without_block       <callinfo!mid:h, argc:0, FCALL|VCALL|ARGS_SIMPLE>, <callcache>
0009 opt_send_without_block       <callinfo!mid:core#hash_merge_kwd, argc:2, ARGS_SIMPLE>, <callcache>
0012 opt_send_without_block       <callinfo!mid:m, argc:1, FCALL|KW_SPLAT>, <callcache>
0015 leave
```

After (basically the same except for the addition of swap):

```
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,12)> (catch: FALSE)
0000 putself                                                          (   1)[Li]
0001 newhash                      0
0003 putspecialobject             1
0005 swap
0006 putself
0007 opt_send_without_block       <callinfo!mid:h, argc:0, FCALL|VCALL|ARGS_SIMPLE>, <callcache>
0010 opt_send_without_block       <callinfo!mid:core#hash_merge_kwd, argc:2, ARGS_SIMPLE>, <callcache>
0013 opt_send_without_block       <callinfo!mid:m, argc:1, FCALL|KW_SPLAT>, <callcache>
0016 leave
```

diff --git a/compile.c b/compile.c
index 3315716..aca710e 100644
--- a/compile.c
+++ b/compile.c
@@ -3940,6 +3940,8 @@ compile_array(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node_ro https://github.com/ruby/ruby/blob/trunk/compile.c#L3940
     else {
 	int opt_p = 1;
 	int first = 1, i;
+        int single_kw = 0;
+        int num_kw = 0;
 
 	while (node) {
 	    const NODE *start_node = node, *end_node;
@@ -3955,11 +3957,15 @@ compile_array(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node_ro https://github.com/ruby/ruby/blob/trunk/compile.c#L3957
 
                 if (type == COMPILE_ARRAY_TYPE_HASH && !node->nd_head) {
 		    kw = node->nd_next;
+                    num_kw++;
 		    node = 0;
 		    if (kw) {
 			opt_p = 0;
 			node = kw->nd_next;
 			kw = kw->nd_head;
+                        if (!single_kw && !node) {
+                            single_kw = 1;
+                        }
 		    }
 		    break;
 		}
@@ -4053,6 +4059,7 @@ compile_array(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node_ro https://github.com/ruby/ruby/blob/trunk/compile.c#L4059
                       }
 		      case COMPILE_ARRAY_TYPE_HASH:
 			if (i > 0) {
+                            num_kw++;
 			    if (first) {
 				if (!popped) {
 				    ADD_INSN1(anchor, line, newhash, INT2FIX(i));
@@ -4071,16 +4078,27 @@ compile_array(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node_ro https://github.com/ruby/ruby/blob/trunk/compile.c#L4078
 			    }
 			}
 			if (kw) {
-			    if (!popped) {
-				ADD_INSN1(ret, line, putspecialobject, INT2FIX(VM_SPECIAL_OBJECT_VMCORE));
-				if (i > 0 || !first) ADD_INSN(ret, line, swap);
-				else ADD_INSN1(ret, line, newhash, INT2FIX(0));
+                            int empty_kw = nd_type(kw) == NODE_LIT;
+                            int first_kw = num_kw == 1;
+                            int only_kw = single_kw && first_kw;
+
+                            if (!popped && !empty_kw) {
+                                ADD_INSN1(ret, line, putspecialobject, INT2FIX(VM_SPECIAL_OBJECT_VMCORE));
+                                if (i > 0 || !first) ADD_INSN(ret, line, swap);
+                                else ADD_INSN1(ret, line, newhash, INT2FIX(0));
 			    }
-                            NO_CHECK(COMPILE(ret, "keyword splat", kw));
+
+                            if (empty_kw && first_kw && !only_kw) {
+                                ADD_INSN1(ret, line, newhash, INT2FIX(0));
+                            }
+                            else if (!empty_kw || only_kw) {
+                                NO_CHECK(COMPILE(ret, "keyword splat", kw));
+                            }
+
 			    if (popped) {
 				ADD_INSN(ret, line, pop);
 			    }
-			    else {
+                            else if (!empty_kw) {
 				ADD_SEND(ret, line, id_core_hash_merge_kwd, INT2FIX(2));
 			    }
 			}
diff --git a/parse.y b/parse.y
index 6729e10..9003f80 100644
--- a/parse.y
+++ b/parse.y
@@ -5209,8 +5209,14 @@ assoc		: arg_value tASSOC arg_value https://github.com/ruby/ruby/blob/trunk/parse.y#L5209
 		    {
 		    /*%%%*/
                         if (nd_type($2) == NODE_HASH &&
-                            !($2->nd_head && $2->nd_head->nd_alen))
-                            $$ = 0;
+                            !($2->nd_head && $2->nd_head->nd_alen)) {
+                            static VALUE empty_hash;
+                            if (!empty_hash) {
+                                empty_hash = rb_obj_freeze(rb_hash_new());
+                                rb_gc_register_mark_object(empty_hash);
+                            }
+                            $$ = list_append(p, NEW_LIST(0, &@$), NEW_LIT(empty_hash, &@$));
+                        }
                         else
                             $$ = list_append(p, NEW_LIST(0, &@$), $2);
 		    /*% %*/
diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb
index 1cf905f..c3ad7b9 100644
--- a/test/ruby/test_keyword.rb
+++ b/test/ruby/test_keyword.rb
@@ -205,7 +205,9 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L205
     assert_equal(h3, f[**h3])
 
     f = ->(a, **x) { [a,x] }
-    assert_raise(ArgumentError) { f[**{}] }
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `\[\]'/m) do
+      assert_equal([{}, {}], f[**{}])
+    end
     assert_warn(/The keyword argument is passed as the last hash parameter.* for `\[\]'/m) do
       assert_equal([{}, {}], f[**kw])
     end
@@ -418,7 +420,9 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L420
     def c.m(arg, **args)
       [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, **{}))
+    end
     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
@@ -491,7 +495,9 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L495
     def c.method_missing(_, arg, **args)
       [arg, args]
     end
-    assert_raise(ArgumentError) { c.send(:m, **{}) }
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([kw, kw], c.send(:m, **{}))
+    end
     assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
       assert_equal([kw, kw], c.send(:m, **kw))
     end
-- 
cgit v0.10.2


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

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