ruby-changes:50668
From: nobu <ko1@a...>
Date: Mon, 19 Mar 2018 13:08:23 +0900 (JST)
Subject: [ruby-changes:50668] nobu:r62833 (trunk): compile.c: arg var index
nobu 2018-03-19 13:08:19 +0900 (Mon, 19 Mar 2018) New Revision: 62833 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=62833 Log: compile.c: arg var index * compile.c (iseq_set_arguments): determine argument variable indexes by the order, not by just IDs. arguments begin with `_` can be duplicate, so by-ID index may result in a wrong value. [ruby-core:86159] [Bug #14611] Modified files: trunk/compile.c trunk/test/ruby/test_syntax.rb Index: test/ruby/test_syntax.rb =================================================================== --- test/ruby/test_syntax.rb (revision 62832) +++ test/ruby/test_syntax.rb (revision 62833) @@ -401,21 +401,30 @@ WARN https://github.com/ruby/ruby/blob/trunk/test/ruby/test_syntax.rb#L401 def test_duplicated_arg assert_syntax_error("def foo(a, a) end", /duplicated argument name/) assert_valid_syntax("def foo(_, _) end") + (obj = Object.new).instance_eval("def foo(_, x, _) x end") + assert_equal(2, obj.foo(1, 2, 3)) end def test_duplicated_rest assert_syntax_error("def foo(a, *a) end", /duplicated argument name/) assert_valid_syntax("def foo(_, *_) end") + (obj = Object.new).instance_eval("def foo(_, x, *_) x end") + assert_equal(2, obj.foo(1, 2, 3)) end def test_duplicated_opt assert_syntax_error("def foo(a, a=1) end", /duplicated argument name/) assert_valid_syntax("def foo(_, _=1) end") + (obj = Object.new).instance_eval("def foo(_, x, _=42) x end") + assert_equal(2, obj.foo(1, 2)) end def test_duplicated_opt_rest assert_syntax_error("def foo(a=1, *a) end", /duplicated argument name/) assert_valid_syntax("def foo(_=1, *_) end") + (obj = Object.new).instance_eval("def foo(_, x=42, *_) x end") + assert_equal(42, obj.foo(1)) + assert_equal(2, obj.foo(1, 2)) end def test_duplicated_rest_opt @@ -424,41 +433,80 @@ WARN https://github.com/ruby/ruby/blob/trunk/test/ruby/test_syntax.rb#L433 def test_duplicated_rest_post assert_syntax_error("def foo(*a, a) end", /duplicated argument name/) + assert_valid_syntax("def foo(*_, _) end") + (obj = Object.new).instance_eval("def foo(*_, x, _) x end") + assert_equal(2, obj.foo(1, 2, 3)) + assert_equal(2, obj.foo(2, 3)) + (obj = Object.new).instance_eval("def foo(*_, _, x) x end") + assert_equal(3, obj.foo(1, 2, 3)) + assert_equal(3, obj.foo(2, 3)) end def test_duplicated_opt_post assert_syntax_error("def foo(a=1, a) end", /duplicated argument name/) assert_valid_syntax("def foo(_=1, _) end") + (obj = Object.new).instance_eval("def foo(_=1, x, _) x end") + assert_equal(2, obj.foo(1, 2, 3)) + assert_equal(2, obj.foo(2, 3)) + (obj = Object.new).instance_eval("def foo(_=1, _, x) x end") + assert_equal(3, obj.foo(1, 2, 3)) + assert_equal(3, obj.foo(2, 3)) end def test_duplicated_kw assert_syntax_error("def foo(a, a: 1) end", /duplicated argument name/) assert_valid_syntax("def foo(_, _: 1) end") + (obj = Object.new).instance_eval("def foo(_, x, _: 1) x end") + assert_equal(3, obj.foo(2, 3)) + assert_equal(3, obj.foo(2, 3, _: 42)) + (obj = Object.new).instance_eval("def foo(x, _, _: 1) x end") + assert_equal(2, obj.foo(2, 3)) + assert_equal(2, obj.foo(2, 3, _: 42)) end def test_duplicated_rest_kw assert_syntax_error("def foo(*a, a: 1) end", /duplicated argument name/) assert_nothing_raised {def foo(*_, _: 1) end} + (obj = Object.new).instance_eval("def foo(*_, x: 42, _: 1) x end") + assert_equal(42, obj.foo(42)) + assert_equal(42, obj.foo(2, _: 0)) + assert_equal(2, obj.foo(x: 2, _: 0)) end def test_duplicated_opt_kw assert_syntax_error("def foo(a=1, a: 1) end", /duplicated argument name/) assert_valid_syntax("def foo(_=1, _: 1) end") + (obj = Object.new).instance_eval("def foo(_=42, x, _: 1) x end") + assert_equal(0, obj.foo(0)) + assert_equal(0, obj.foo(0, _: 3)) end def test_duplicated_kw_kwrest assert_syntax_error("def foo(a: 1, **a) end", /duplicated argument name/) assert_valid_syntax("def foo(_: 1, **_) end") + (obj = Object.new).instance_eval("def foo(_: 1, x: 42, **_) x end") + assert_equal(42, obj.foo()) + assert_equal(42, obj.foo(a: 0)) + assert_equal(42, obj.foo(_: 0, a: 0)) + assert_equal(1, obj.foo(_: 0, x: 1, a: 0)) end def test_duplicated_rest_kwrest assert_syntax_error("def foo(*a, **a) end", /duplicated argument name/) assert_valid_syntax("def foo(*_, **_) end") + (obj = Object.new).instance_eval("def foo(*_, x, **_) x end") + assert_equal(1, obj.foo(1)) + assert_equal(1, obj.foo(1, a: 0)) + assert_equal(2, obj.foo(1, 2, a: 0)) end def test_duplicated_opt_kwrest assert_syntax_error("def foo(a=1, **a) end", /duplicated argument name/) assert_valid_syntax("def foo(_=1, **_) end") + (obj = Object.new).instance_eval("def foo(_=42, x, **_) x end") + assert_equal(1, obj.foo(1)) + assert_equal(1, obj.foo(1, a: 0)) + assert_equal(1, obj.foo(0, 1, a: 0)) end def test_duplicated_when Index: compile.c =================================================================== --- compile.c (revision 62832) +++ compile.c (revision 62833) @@ -1534,9 +1534,9 @@ iseq_calc_param_size(rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/compile.c#L1534 } } -static void +static int iseq_set_arguments_keywords(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, - const struct rb_args_info *args) + const struct rb_args_info *args, int arg_size) { const NODE *node = args->kw_args; struct rb_iseq_param_keyword *keyword; @@ -1546,9 +1546,16 @@ iseq_set_arguments_keywords(rb_iseq_t *i https://github.com/ruby/ruby/blob/trunk/compile.c#L1546 iseq->body->param.flags.has_kw = TRUE; iseq->body->param.keyword = keyword = ZALLOC_N(struct rb_iseq_param_keyword, 1); - keyword->bits_start = get_dyna_var_idx_at_raw(iseq, args->kw_rest_arg->nd_cflag); while (node) { + kw++; + node = node->nd_next; + } + arg_size += kw; + keyword->bits_start = arg_size++; + + node = args->kw_args; + while (node) { const NODE *val_node = node->nd_body->nd_value; VALUE dv; @@ -1579,14 +1586,13 @@ iseq_set_arguments_keywords(rb_iseq_t *i https://github.com/ruby/ruby/blob/trunk/compile.c#L1586 rb_ary_push(default_values, dv); } - kw++; node = node->nd_next; } keyword->num = kw; if (args->kw_rest_arg->nd_vid != 0) { - keyword->rest_start = get_dyna_var_idx_at_raw(iseq, args->kw_rest_arg->nd_vid); + keyword->rest_start = arg_size++; iseq->body->param.flags.has_kwrest = TRUE; } keyword->required_num = rkw; @@ -1603,6 +1609,7 @@ iseq_set_arguments_keywords(rb_iseq_t *i https://github.com/ruby/ruby/blob/trunk/compile.c#L1609 keyword->default_values = dvs; } + return arg_size; } static int @@ -1615,10 +1622,11 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK https://github.com/ruby/ruby/blob/trunk/compile.c#L1622 ID rest_id = 0; int last_comma = 0; ID block_id = 0; + int arg_size; EXPECT_NODE("iseq_set_arguments", node_args, NODE_ARGS, COMPILE_NG); - iseq->body->param.lead_num = (int)args->pre_args_num; + iseq->body->param.lead_num = arg_size = (int)args->pre_args_num; if (iseq->body->param.lead_num > 0) iseq->body->param.flags.has_lead = TRUE; debugs(" - argc: %d\n", iseq->body->param.lead_num); @@ -1629,12 +1637,6 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK https://github.com/ruby/ruby/blob/trunk/compile.c#L1637 } block_id = args->block_arg; - if (args->first_post_arg) { - iseq->body->param.post_start = get_dyna_var_idx_at_raw(iseq, args->first_post_arg); - iseq->body->param.post_num = args->post_args_num; - iseq->body->param.flags.has_post = TRUE; - } - if (args->opt_args) { const NODE *node = args->opt_args; LABEL *label; @@ -1667,18 +1669,44 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK https://github.com/ruby/ruby/blob/trunk/compile.c#L1669 iseq->body->param.flags.has_opt = TRUE; iseq->body->param.opt_num = i; iseq->body->param.opt_table = opt_table; + arg_size += i; + } + + if (rest_id) { + iseq->body->param.rest_start = arg_size++; + iseq->body->param.flags.has_rest = TRUE; + assert(iseq->body->param.rest_start != -1); + } + + if (args->first_post_arg) { + iseq->body->param.post_start = arg_size; + iseq->body->param.post_num = args->post_args_num; + iseq->body->param.flags.has_post = TRUE; + arg_size += args->post_args_num; + + if (iseq->body->param.flags.has_rest) { /* TODO: why that? */ + iseq->body->param.post_start = iseq->body->param.rest_start + 1; + } } if (args->kw_args) { - iseq_set_arguments_keywords(iseq, optargs, args); + arg_size = iseq_set_arguments_keywords(iseq, optargs, args, arg_size); } else if (args->kw_rest_arg) { struct rb_iseq_param_keyword *keyword = ZALLOC_N(struct rb_iseq_param_keyword, 1); - keyword->rest_start = get_dyna_var_idx_at_raw(iseq, args->kw_rest_arg->nd_vid); + keyword->rest_start = arg_size++; iseq->body->param.keyword = keyword; iseq->body->param.flags.has_kwrest = TRUE; } + if (block_id) { + iseq->body->param.block_start = arg_size++; + iseq->body->param.flags.has_block = TRUE; + } + + iseq_calc_param_size(iseq); + iseq->body->param.size = arg_size; + if (args->pre_init) { /* m_init */ COMPILE_POPPED(optargs, "init arguments (m)", args->pre_init); } @@ -1686,23 +1714,6 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK https://github.com/ruby/ruby/blob/trunk/compile.c#L1714 COMPILE_POPPED(optargs, "init arguments (p)", args->post_init); } - if (rest_id) { - iseq->body->param.rest_start = get_dyna_var_idx_at_raw(iseq, rest_id); - iseq->body->param.flags.has_rest = TRUE; - assert(iseq->body->param.rest_start != -1); - - if (iseq->body->param.post_start == 0) { /* TODO: why that? */ - iseq->body->param.post_start = iseq->body->param.rest_start + 1; - } - } - - if (block_id) { - iseq->body->param.block_start = get_dyna_var_idx_at_raw(iseq, block_id); - iseq->body->param.flags.has_block = TRUE; - } - - iseq_calc_param_size(iseq); - if (iseq->body->type == ISEQ_TYPE_BLOCK) { if (iseq->body->param.flags.has_opt == FALSE && iseq->body->param.flags.has_post == FALSE && -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/