From fac2c0f73cafb5d65bfbba7aa8018fa427972d71 Mon Sep 17 00:00:00 2001 From: Jeremy Evans 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 Co-authored-by: Ivo Anjo --- 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) { 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 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 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.3