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

ruby-changes:68524

From: Jeremy <ko1@a...>
Date: Tue, 19 Oct 2021 01:09:25 +0900 (JST)
Subject: [ruby-changes:68524] fac2c0f73c (master): Fix evaluation order of hash values for duplicate keys

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

From fac2c0f73cafb5d65bfbba7aa8018fa427972d71 Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@j...>
Date: Mon, 18 Oct 2021 07:09:07 -0900
Subject: Fix evaluation order of hash values for duplicate keys

Fixes [Bug #17719]

Co-authored-by: Nobuyoshi Nakada <nobu@r...>
Co-authored-by: Ivo Anjo <ivo@i...>
---
 parse.y                   | 20 +++++++++++++-------
 test/ruby/test_literal.rb | 11 +++++++++++
 test/ruby/test_syntax.rb  | 22 +++++++++++++++++-----
 3 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/parse.y b/parse.y
index bf0d33a491..9f44ff6235 100644
--- a/parse.y
+++ b/parse.y
@@ -12246,24 +12246,30 @@ remove_duplicate_keys(struct parser_params *p, NODE *hash) https://github.com/ruby/ruby/blob/trunk/parse.y#L12246
 {
     st_table *literal_keys = st_init_table_with_size(&literal_type, hash->nd_alen / 2);
     NODE *result = 0;
+    NODE *last_expr = 0;
     rb_code_location_t loc = hash->nd_loc;
     while (hash && hash->nd_head && hash->nd_next) {
 	NODE *head = hash->nd_head;
 	NODE *value = hash->nd_next;
 	NODE *next = value->nd_next;
-	VALUE key = (VALUE)head;
+	st_data_t key = (st_data_t)head;
 	st_data_t data;
+	value->nd_next = 0;
 	if (nd_type(head) == NODE_LIT &&
-	    st_lookup(literal_keys, (key = head->nd_lit), &data)) {
+	    st_delete(literal_keys, (key = (st_data_t)head->nd_lit, &key), &data)) {
+	    NODE *dup_value = ((NODE *)data)->nd_next;
 	    rb_compile_warn(p->ruby_sourcefile, nd_line((NODE *)data),
 			    "key %+"PRIsVALUE" is duplicated and overwritten on line %d",
 			    head->nd_lit, nd_line(head));
-	    head = ((NODE *)data)->nd_next;
-	    head->nd_head = block_append(p, head->nd_head, value->nd_head);
-	}
-	else {
-	    st_insert(literal_keys, (st_data_t)key, (st_data_t)hash);
+	    if (dup_value == last_expr) {
+		value->nd_head = block_append(p, dup_value->nd_head, value->nd_head);
+	    }
+	    else {
+		last_expr->nd_head = block_append(p, dup_value->nd_head, last_expr->nd_head);
+	    }
 	}
+	st_insert(literal_keys, (st_data_t)key, (st_data_t)hash);
+	last_expr = nd_type(head) == NODE_LIT ? value : head;
 	hash = next;
     }
     st_foreach(literal_keys, append_literal_keys, (st_data_t)&result);
diff --git a/test/ruby/test_literal.rb b/test/ruby/test_literal.rb
index 3a8ff7857f..8c3256c034 100644
--- a/test/ruby/test_literal.rb
+++ b/test/ruby/test_literal.rb
@@ -474,6 +474,17 @@ class TestRubyLiteral < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_literal.rb#L474
     assert_nil(h['c'])
     assert_equal(nil, h.key('300'))
 
+    a = []
+    h = EnvUtil.suppress_warning do
+      eval <<~end
+        # This is a syntax that renders warning at very early stage.
+        # eval used to delay warning, to be suppressible by EnvUtil.
+        {"a" => a.push(100).last, "b" => a.push(200).last, "a" => a.push(300).last, "a" => a.push(400).last}
+      end
+    end
+    assert_equal({'a' => 400, 'b' => 200}, h)
+    assert_equal([100, 200, 300, 400], a)
+
     assert_all_assertions_foreach(
       "duplicated literal key",
       ':foo',
diff --git a/test/ruby/test_syntax.rb b/test/ruby/test_syntax.rb
index 667eb205dc..fc40a7f21a 100644
--- a/test/ruby/test_syntax.rb
+++ b/test/ruby/test_syntax.rb
@@ -200,17 +200,29 @@ class TestSyntax < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_syntax.rb#L200
     bug10315 = '[ruby-core:65625] [Bug #10315]'
     a = []
     def a.add(x) push(x); x; end
-    def a.f(k:) k; end
+    b = a.clone
+    def a.f(k:, **) k; end
+    def b.f(k:) k; end
     a.clear
     r = nil
-    assert_warn(/duplicated/) {r = eval("a.f(k: a.add(1), k: a.add(2))")}
+    assert_warn(/duplicated/) {r = eval("b.f(k: b.add(1), k: b.add(2))")}
     assert_equal(2, r)
-    assert_equal([1, 2], a, bug10315)
+    assert_equal([1, 2], b, bug10315)
+    b.clear
+    r = nil
+    assert_warn(/duplicated/) {r = eval("a.f(k: a.add(1), j: a.add(2), k: a.add(3), k: a.add(4))")}
+    assert_equal(4, r)
+    assert_equal([1, 2, 3, 4], a)
     a.clear
     r = nil
-    assert_warn(/duplicated/) {r = eval("a.f(**{k: a.add(1), k: a.add(2)})")}
+    assert_warn(/duplicated/) {r = eval("b.f(**{k: b.add(1), k: b.add(2)})")}
     assert_equal(2, r)
-    assert_equal([1, 2], a, bug10315)
+    assert_equal([1, 2], b, bug10315)
+    b.clear
+    r = nil
+    assert_warn(/duplicated/) {r = eval("a.f(**{k: a.add(1), j: a.add(2), k: a.add(3), k: a.add(4)})")}
+    assert_equal(4, r)
+    assert_equal([1, 2, 3, 4], a)
   end
 
   def test_keyword_empty_splat
-- 
cgit v1.2.1


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

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