github
/
ruby
зеркало из https://github.com/github/ruby.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Fix constant assignment evaluation order 

Previously, the right hand side was always evaluated before the
left hand side for constant assignments.  For the following:

```ruby
lhs::C = rhs
```

rhs was evaluated before lhs, which is inconsistant with attribute
assignment (lhs.m = rhs), and apparently also does not conform to
JIS 3017:2013 11.4.2.2.3.

Fix this by changing evaluation order.  Previously, the above
compiled to:

```
0000 putself                                                          (   1)[Li]
0001 opt_send_without_block                 <calldata!mid:rhs, argc:0, FCALL|VCALL|ARGS_SIMPLE>
0003 dup
0004 putself
0005 opt_send_without_block                 <calldata!mid:lhs, argc:0, FCALL|VCALL|ARGS_SIMPLE>
0007 setconstant                            :C
0009 leave
```

After this change:

```
0000 putself                                                          (   1)[Li]
0001 opt_send_without_block                 <calldata!mid:lhs, argc:0, FCALL|VCALL|ARGS_SIMPLE>
0003 putself
0004 opt_send_without_block                 <calldata!mid:rhs, argc:0, FCALL|VCALL|ARGS_SIMPLE>
0006 swap
0007 topn                                   1
0009 swap
0010 setconstant                            :C
0012 leave
```

Note that if expr is not a module/class, then a TypeError is not
raised until after the evaluation of rhs.  This is because that
error is raised by setconstant.  If we wanted to raise TypeError
before evaluation of rhs, we would have to add a VM instruction
for calling vm_check_if_namespace.

Changing assignment order for single assignments caused problems
in the multiple assignment code, revealing that the issue also
affected multiple assignment.  Fix the multiple assignment code
so left-to-right evaluation also works for constant assignments.

Do some refactoring of the multiple assignment code to reduce
duplication after adding support for constants. Rename struct
masgn_attrasgn to masgn_lhs_node, since it now handles both
constants and attributes. Add add_masgn_lhs_node static function
for adding data for lhs attribute and constant setting.

Fixes [Bug #15928]
This commit is contained in:
Jeremy Evans 2021-04-30 16:01:27 -07:00
Родитель 3cc82ff93c
Коммит ca3d405242
4 изменённых файлов: 237 добавлений и 45 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

26
NEWS.md
Просмотреть файл

@ -20,6 +20,31 @@ Note that each entry is kept to a minimum, see links for details.
end
```
* Constant assignment evaluation order for constants set on explicit
objects has been made consistent with single attribute assignment
evaluation order. With this code:
```ruby
foo::BAR = baz
```
`foo` is now called before `baz`. Similarly, for multiple assignment
to constants, left-to-right evaluation order is used. With this
code:
```ruby
foo1::BAR1, foo2::BAR2 = baz1, baz2
```
The following evaluation order is now used:
1. `foo1`
2. `foo2`
3. `baz1`
4. `baz2`
[[Bug #15928]]
## Command line options
## Core classes updates
@ -110,6 +135,7 @@ The following deprecated APIs are removed.
[Feature #12737]: https://bugs.ruby-lang.org/issues/12737
[Feature #14332]: https://bugs.ruby-lang.org/issues/14332
[Feature #15231]: https://bugs.ruby-lang.org/issues/15231
[Bug #15928]: https://bugs.ruby-lang.org/issues/15928
[Feature #16131]: https://bugs.ruby-lang.org/issues/16131
[Feature #17351]: https://bugs.ruby-lang.org/issues/17351
[Feature #17391]: https://bugs.ruby-lang.org/issues/17391

113
compile.c
Просмотреть файл

@ -4686,9 +4686,9 @@ when_splat_vals(rb_iseq_t *iseq, LINK_ANCHOR *const cond_seq, const NODE *vals,
*
* In order to handle evaluation of multiple assignment such that the left hand side
* is evaluated before the right hand side, we need to process the left hand side
* and see if there are any attributes that need to be assigned. If so, we add
* instructions to evaluate the receiver of any assigned attributes before we
* process the right hand side.
* and see if there are any attributes that need to be assigned, or constants set
* on explicit objects. If so, we add instructions to evaluate the receiver of
* any assigned attributes or constants before we process the right hand side.
*
* For a multiple assignment such as:
*
@ -4755,7 +4755,7 @@ when_splat_vals(rb_iseq_t *iseq, LINK_ANCHOR *const cond_seq, const NODE *vals,
* In order to handle this correctly, we need to keep track of the nesting
* level for each attribute assignment, as well as the attribute number
* (left hand side attributes are processed left to right) and number of
* arguments to pass to the setter method. struct masgn_attrasgn tracks
* arguments to pass to the setter method. struct masgn_lhs_node tracks
* this information.
*
* We also need to track information for the entire multiple assignment, such
@ -4766,9 +4766,9 @@ when_splat_vals(rb_iseq_t *iseq, LINK_ANCHOR *const cond_seq, const NODE *vals,
* tracks this information.
*/
struct masgn_attrasgn {
struct masgn_lhs_node {
INSN *before_insn;
struct masgn_attrasgn *next;
struct masgn_lhs_node *next;
const NODE *line_node;
int argn;
int num_args;
@ -4776,13 +4776,43 @@ struct masgn_attrasgn {
};
struct masgn_state {
struct masgn_attrasgn *first_memo;
struct masgn_attrasgn *last_memo;
struct masgn_lhs_node *first_memo;
struct masgn_lhs_node *last_memo;
int lhs_level;
int num_args;
bool nested;
};
static int
add_masgn_lhs_node(struct masgn_state *state, int lhs_pos, const NODE *line_node, int argc, INSN *before_insn) {
if (!state) {
rb_bug("no masgn_state");
}
struct masgn_lhs_node *memo;
memo = malloc(sizeof(struct masgn_lhs_node));
if (!memo) {
return COMPILE_NG;
}
memo->before_insn = before_insn;
memo->line_node = line_node;
memo->argn = state->num_args + 1;
memo->num_args = argc;
state->num_args += argc;
memo->lhs_pos = lhs_pos;
memo->next = NULL;
if (!state->first_memo) {
state->first_memo = memo;
}
else {
state->last_memo->next = memo;
}
state->last_memo = memo;
return COMPILE_OK;
}
static int compile_massign0(rb_iseq_t *iseq, LINK_ANCHOR *const pre, LINK_ANCHOR *const rhs, LINK_ANCHOR *const lhs, LINK_ANCHOR *const post, const NODE *const node, struct masgn_state *state, int popped);
static int
@ -4790,10 +4820,6 @@ compile_massign_lhs(rb_iseq_t *iseq, LINK_ANCHOR *const pre, LINK_ANCHOR *const
{
switch (nd_type(node)) {
case NODE_ATTRASGN: {
if (!state) {
rb_bug("no masgn_state");
}
INSN *iobj;
const NODE *line_node = node;
@ -4819,25 +4845,9 @@ compile_massign_lhs(rb_iseq_t *iseq, LINK_ANCHOR *const pre, LINK_ANCHOR *const
ADD_INSN1(lhs, line_node, topn, INT2FIX(argc));
}
struct masgn_attrasgn *memo;
memo = malloc(sizeof(struct masgn_attrasgn));
if (!memo) {
return 0;
if (!add_masgn_lhs_node(state, lhs_pos, line_node, argc, (INSN *)LAST_ELEMENT(lhs))) {
return COMPILE_NG;
}
memo->before_insn = (INSN *)LAST_ELEMENT(lhs);
memo->line_node = line_node;
memo->argn = state->num_args + 1;
memo->num_args = argc;
state->num_args += argc;
memo->lhs_pos = lhs_pos;
memo->next = NULL;
if (!state->first_memo) {
state->first_memo = memo;
}
else {
state->last_memo->next = memo;
}
state->last_memo = memo;
ADD_ELEM(lhs, (LINK_ELEMENT *)iobj);
if (vm_ci_flag(ci) & VM_CALL_ARGS_SPLAT) {
@ -4875,6 +4885,29 @@ compile_massign_lhs(rb_iseq_t *iseq, LINK_ANCHOR *const pre, LINK_ANCHOR *const
ADD_SEQ(lhs, nest_lhs);
break;
}
case NODE_CDECL:
if (!node->nd_vid) {
/* Special handling only needed for expr::C, not for C */
INSN *iobj;
CHECK(COMPILE_POPPED(pre, "masgn lhs (NODE_CDECL)", node));
LINK_ELEMENT *insn_element = LAST_ELEMENT(pre);
iobj = (INSN *)insn_element; /* setconstant insn */
ELEM_REMOVE((LINK_ELEMENT *)get_prev_insn((INSN *)get_prev_insn(iobj)));
ELEM_REMOVE((LINK_ELEMENT *)get_prev_insn(iobj));
ELEM_REMOVE(insn_element);
pre->last = iobj->link.prev;
ADD_ELEM(lhs, (LINK_ELEMENT *)iobj);
if (!add_masgn_lhs_node(state, lhs_pos, node, 1, (INSN *)LAST_ELEMENT(lhs))) {
return COMPILE_NG;
}
ADD_INSN(post, node, pop);
break;
}
/* Fallthrough */
default: {
DECL_ANCHOR(anchor);
INIT_ANCHOR(anchor);
@ -5044,7 +5077,7 @@ compile_massign(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node,
INIT_ANCHOR(post);
int ok = compile_massign0(iseq, pre, rhs, lhs, post, node, &state, popped);
struct masgn_attrasgn *memo = state.first_memo, *tmp_memo;
struct masgn_lhs_node *memo = state.first_memo, *tmp_memo;
while (memo) {
VALUE topn_arg = INT2FIX((state.num_args - memo->argn) + memo->lhs_pos);
for (int i = 0; i < memo->num_args; i++) {
@ -9236,19 +9269,27 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const no
break;
}
case NODE_CDECL:{
CHECK(COMPILE(ret, "lvalue", node->nd_value));
if (node->nd_vid) {
CHECK(COMPILE(ret, "lvalue", node->nd_value));
if (!popped) {
ADD_INSN(ret, node, dup);
}
if (!popped) {
ADD_INSN(ret, node, dup);
}
if (node->nd_vid) {
ADD_INSN1(ret, node, putspecialobject,
INT2FIX(VM_SPECIAL_OBJECT_CONST_BASE));
ADD_INSN1(ret, node, setconstant, ID2SYM(node->nd_vid));
}
else {
compile_cpath(ret, iseq, node->nd_else);
CHECK(COMPILE(ret, "lvalue", node->nd_value));
ADD_INSN(ret, node, swap);
if (!popped) {
ADD_INSN1(ret, node, topn, INT2FIX(1));
ADD_INSN(ret, node, swap);
}
ADD_INSN1(ret, node, setconstant, ID2SYM(node->nd_else->nd_mid));
}
break;

34
spec/ruby/language/constants_spec.rb
Просмотреть файл

@ -135,18 +135,34 @@ describe "Literal (A::X) constant resolution" do
ConstantSpecs::ClassB::CS_CONST109.should == :const109_2
end
it "evaluates the right hand side before evaluating a constant path" do
mod = Module.new
ruby_version_is "3.2" do
it "evaluates left-to-right" do
mod = Module.new
mod.module_eval <<-EOC
ConstantSpecsRHS::B = begin
module ConstantSpecsRHS; end
mod.module_eval <<-EOC
order = []
ConstantSpecsRHS = Module.new
(order << :lhs; ConstantSpecsRHS)::B = (order << :rhs)
EOC
"hello"
end
EOC
mod::ConstantSpecsRHS::B.should == [:lhs, :rhs]
end
end
mod::ConstantSpecsRHS::B.should == 'hello'
ruby_version_is ""..."3.2" do
it "evaluates the right hand side before evaluating a constant path" do
mod = Module.new
mod.module_eval <<-EOC
ConstantSpecsRHS::B = begin
module ConstantSpecsRHS; end
"hello"
end
EOC
mod::ConstantSpecsRHS::B.should == 'hello'
end
end
end

109
test/ruby/test_assignment.rb
Просмотреть файл

@ -139,6 +139,104 @@ class TestAssignment < Test::Unit::TestCase
order.clear
end
def test_massign_const_order
order = []
test_mod_class = Class.new(Module) do
define_method(:x1){order << :x1; self}
define_method(:y1){order << :y1; self}
define_method(:x2){order << :x2; self}
define_method(:x3){order << :x3; self}
define_method(:x4){order << :x4; self}
define_method(:[]){|*args| order << [:[], *args]; self}
define_method(:r1){order << :r1; :r1}
define_method(:r2){order << :r2; :r2}
define_method(:constant_values) do
h = {}
constants.each do |sym|
h[sym] = const_get(sym)
end
h
end
define_singleton_method(:run) do |code|
m = new
m.instance_eval(code)
ret = [order.dup, m.constant_values]
order.clear
ret
end
end
ord, constants = test_mod_class.run(
"x1.y1::A, x2[1, 2, 3]::B, self[4]::C = r1, 6, r2"
)
assert_equal([:x1, :y1, :x2, [:[], 1, 2, 3], [:[], 4], :r1, :r2], ord)
assert_equal({:A=>:r1, :B=>6, :C=>:r2}, constants)
ord, constants = test_mod_class.run(
"x1.y1::A, *x2[1, 2, 3]::B, self[4]::C = r1, 6, 7, r2"
)
assert_equal([:x1, :y1, :x2, [:[], 1, 2, 3], [:[], 4], :r1, :r2], ord)
assert_equal({:A=>:r1, :B=>[6, 7], :C=>:r2}, constants)
ord, constants = test_mod_class.run(
"x1.y1::A, *x2[1, 2, 3]::B, x3[4]::C = r1, 6, 7, r2"
)
assert_equal([:x1, :y1, :x2, [:[], 1, 2, 3], :x3, [:[], 4], :r1, :r2], ord)
assert_equal({:A=>:r1, :B=>[6, 7], :C=>:r2}, constants)
ord, constants = test_mod_class.run(
"x1.y1::A, *x2[1, 2, 3]::B, x3[4]::C, x4::D = r1, 6, 7, r2, 8"
)
assert_equal([:x1, :y1, :x2, [:[], 1, 2, 3], :x3, [:[], 4], :x4, :r1, :r2], ord)
assert_equal({:A=>:r1, :B=>[6, 7], :C=>:r2, :D=>8}, constants)
ord, constants = test_mod_class.run(
"(x1.y1::A, x2::B), _a = [r1, r2], 7"
)
assert_equal([:x1, :y1, :x2, :r1, :r2], ord)
assert_equal({:A=>:r1, :B=>:r2}, constants)
ord, constants = test_mod_class.run(
"(x1.y1::A, x1::B), *x2[1, 2, 3]::C = [r1, 5], 6, 7, r2, 8"
)
assert_equal([:x1, :y1, :x1, :x2, [:[], 1, 2, 3], :r1, :r2], ord)
assert_equal({:A=>:r1, :B=>5, :C=>[6, 7, :r2, 8]}, constants)
ord, constants = test_mod_class.run(
"*x2[1, 2, 3]::A, (x3[4]::B, x4::C) = 6, 7, [r2, 8]"
)
assert_equal([:x2, [:[], 1, 2, 3], :x3, [:[], 4], :x4, :r2], ord)
assert_equal({:A=>[6, 7], :B=>:r2, :C=>8}, constants)
ord, constants = test_mod_class.run(
"(x1.y1::A, x1::B), *x2[1, 2, 3]::C, x3[4]::D, x4::E = [r1, 5], 6, 7, r2, 8"
)
assert_equal([:x1, :y1, :x1, :x2, [:[], 1, 2, 3], :x3, [:[], 4], :x4, :r1, :r2], ord)
assert_equal({:A=>:r1, :B=>5, :C=>[6, 7], :D=>:r2, :E=>8}, constants)
ord, constants = test_mod_class.run(
"(x1.y1::A, x1::B), *x2[1, 2, 3]::C, (x3[4]::D, x4::E) = [r1, 5], 6, 7, [r2, 8]"
)
assert_equal([:x1, :y1, :x1, :x2, [:[], 1, 2, 3], :x3, [:[], 4], :x4, :r1, :r2], ord)
assert_equal({:A=>:r1, :B=>5, :C=>[6, 7], :D=>:r2, :E=>8}, constants)
ord, constants = test_mod_class.run(
"((x1.y1::A, x1::B), _a), *x2[1, 2, 3]::C, ((x3[4]::D, x4::E), _b) = [[r1, 5], 10], 6, 7, [[r2, 8], 11]"
)
assert_equal([:x1, :y1, :x1, :x2, [:[], 1, 2, 3], :x3, [:[], 4], :x4, :r1, :r2], ord)
assert_equal({:A=>:r1, :B=>5, :C=>[6, 7], :D=>:r2, :E=>8}, constants)
ord, constants = test_mod_class.run(
"((x1.y1::A, x1::B), _a), *x2[1, 2, 3]::C, ((*x3[4]::D, x4::E), _b) = [[r1, 5], 10], 6, 7, [[r2, 8], 11]"
)
assert_equal([:x1, :y1, :x1, :x2, [:[], 1, 2, 3], :x3, [:[], 4], :x4, :r1, :r2], ord)
assert_equal({:A=>:r1, :B=>5, :C=>[6, 7], :D=>[:r2], :E=>8}, constants)
end
def test_massign_splat
a,b,*c = *[]; assert_equal([nil,nil,[]], [a,b,c])
a,b,*c = *[1]; assert_equal([1,nil,[]], [a,b,c])
@ -619,6 +717,17 @@ class TestAssignment < Test::Unit::TestCase
result = eval("if (a, b = MyObj.new); [a, b]; end", nil, __FILE__, __LINE__)
assert_equal [[1,2],[3,4]], result
end
def test_const_assign_order
assert_raise(RuntimeError) do
eval('raise("recv")::C = raise(ArgumentError, "bar")')
end
assert_raise(RuntimeError) do
m = 1
eval('m::C = raise("bar")')
end
end
end
require_relative 'sentence'