diff --git a/NEWS.md b/NEWS.md index 6de6d97e8b..cf79b059e5 100644 --- a/NEWS.md +++ b/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 diff --git a/compile.c b/compile.c index 83389947c0..8fd1fd65f8 100644 --- a/compile.c +++ b/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; diff --git a/spec/ruby/language/constants_spec.rb b/spec/ruby/language/constants_spec.rb index bc76c60d20..8586e46158 100644 --- a/spec/ruby/language/constants_spec.rb +++ b/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 diff --git a/test/ruby/test_assignment.rb b/test/ruby/test_assignment.rb index 41e8bffe82..6164d532c0 100644 --- a/test/ruby/test_assignment.rb +++ b/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'