summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--NEWS.md26
-rw-r--r--compile.c113
-rw-r--r--spec/ruby/language/constants_spec.rb34
-rw-r--r--test/ruby/test_assignment.rb109
4 files changed, 237 insertions, 45 deletions
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;
- }
- 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;
+ if (!add_masgn_lhs_node(state, lhs_pos, line_node, argc, (INSN *)LAST_ELEMENT(lhs))) {
+ return COMPILE_NG;
}
- 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
+
+ ruby_version_is ""..."3.2" do
+ it "evaluates the right hand side before evaluating a constant path" do
+ mod = Module.new
- mod::ConstantSpecsRHS::B.should == 'hello'
+ 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'
generated by cgit v1.2.3 (git 2.25.1) at 2024-05-12 09:02:29 +0000