Make Array#min/max optimization respect refined methods

jeremyevans · web-flow · commit 1f5f8a187adb · 2021-09-30T15:18:14.000-07:00
Pass in ec to vm_opt_newarray_{max,min}. Avoids having to
call GET_EC inside the functions, for better performance.

While here, add a test for Array#min/max being redefined to
test_optimization.rb.

Fixes [Bug #18180]
diff --git a/insns.def b/insns.def
@@ -834,7 +834,7 @@ opt_newarray_max
 // attr bool leaf = false; /* has rb_funcall() */
 // attr rb_snum_t sp_inc = 1 - (rb_snum_t)num;
 {
-    val = vm_opt_newarray_max(num, STACK_ADDR_FROM_TOP(num));
+    val = vm_opt_newarray_max(ec, num, STACK_ADDR_FROM_TOP(num));
 }
 
 DEFINE_INSN
@@ -846,7 +846,7 @@ opt_newarray_min
 // attr bool leaf = false; /* has rb_funcall() */
 // attr rb_snum_t sp_inc = 1 - (rb_snum_t)num;
 {
-    val = vm_opt_newarray_min(num, STACK_ADDR_FROM_TOP(num));
+    val = vm_opt_newarray_min(ec, num, STACK_ADDR_FROM_TOP(num));
 }
 
 /* super(args) # args.size => num */
diff --git a/test/ruby/test_optimization.rb b/test/ruby/test_optimization.rb
@@ -150,6 +150,18 @@ def test_string_uminus
     assert_redefine_method('String', '-@', 'assert_nil(-"foo")')
   end
 
+  def test_array_min
+    assert_equal 1, [1, 2, 4].min
+    assert_redefine_method('Array', 'min', 'assert_nil([1, 2, 4].min)')
+    assert_redefine_method('Array', 'min', 'assert_nil([1 + 0, 2, 4].min)')
+  end
+
+  def test_array_max
+    assert_equal 4, [1, 2, 4].max
+    assert_redefine_method('Array', 'max', 'assert_nil([1, 2, 4].max)')
+    assert_redefine_method('Array', 'max', 'assert_nil([1 + 0, 2, 4].max)')
+  end
+
   def test_trace_optimized_methods
     bug14870 = "[ruby-core:87638]"
     expected = [:-@, :max, :min, :+, :-, :*, :/, :%, :==, :<, :<=, :>, :>=, :<<,
diff --git a/test/ruby/test_refinement.rb b/test/ruby/test_refinement.rb
@@ -2537,6 +2537,25 @@ def test_redefining_refined_for_prepended_class
     assert_equal(:second, klass.new.foo)
   end
 
+  class Bug18180
+    module M
+      refine Array do
+        def min; :min; end
+        def max; :max; end
+      end
+    end
+
+    using M
+
+    def t
+      [[1+0, 2, 4].min, [1, 2, 4].min, [1+0, 2, 4].max, [1, 2, 4].max]
+    end
+  end
+
+  def test_refine_array_min_max
+    assert_equal([:min, :min, :max, :max], Bug18180.new.t)
+  end
+
   class Bug17822
     module Ext
       refine(Bug17822) do
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
@@ -4655,7 +4655,7 @@ vm_opt_str_freeze(VALUE str, int bop, ID id)
 #define id_cmp idCmp
 
 static VALUE
-vm_opt_newarray_max(rb_num_t num, const VALUE *ptr)
+vm_opt_newarray_max(rb_execution_context_t *ec, rb_num_t num, const VALUE *ptr)
 {
     if (BASIC_OP_UNREDEFINED_P(BOP_MAX, ARRAY_REDEFINED_OP_FLAG)) {
 	if (num == 0) {
@@ -4676,12 +4676,19 @@ vm_opt_newarray_max(rb_num_t num, const VALUE *ptr)
     }
     else {
 	VALUE ary = rb_ary_new4(num, ptr);
-	return rb_funcall(ary, idMax, 0);
+        const rb_callable_method_entry_t *me =
+            rb_callable_method_entry_with_refinements(rb_cArray, idMax, NULL);
+        if (me) {
+            return rb_vm_call0(ec, ary, idMax, 0, NULL, me, RB_NO_KEYWORDS);
+        }
+        else {
+            return rb_funcall(ary, idMax, 0);
+        }
     }
 }
 
 static VALUE
-vm_opt_newarray_min(rb_num_t num, const VALUE *ptr)
+vm_opt_newarray_min(rb_execution_context_t *ec, rb_num_t num, const VALUE *ptr)
 {
     if (BASIC_OP_UNREDEFINED_P(BOP_MIN, ARRAY_REDEFINED_OP_FLAG)) {
 	if (num == 0) {
@@ -4702,7 +4709,14 @@ vm_opt_newarray_min(rb_num_t num, const VALUE *ptr)
     }
     else {
 	VALUE ary = rb_ary_new4(num, ptr);
-	return rb_funcall(ary, idMin, 0);
+        const rb_callable_method_entry_t *me =
+            rb_callable_method_entry_with_refinements(rb_cArray, idMin, NULL);
+        if (me) {
+            return rb_vm_call0(ec, ary, idMin, 0, NULL, me, RB_NO_KEYWORDS);
+        }
+        else {
+            return rb_funcall(ary, idMin, 0);
+        }
     }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -834,7 +834,7 @@ opt_newarray_max`
`834`	`834`	`// attr bool leaf = false; /* has rb_funcall() */`
`835`	`835`	`// attr rb_snum_t sp_inc = 1 - (rb_snum_t)num;`
`836`	`836`	`{`
`837`		`- val = vm_opt_newarray_max(num, STACK_ADDR_FROM_TOP(num));`
	`837`	`+ val = vm_opt_newarray_max(ec, num, STACK_ADDR_FROM_TOP(num));`
`838`	`838`	`}`
`839`	`839`
`840`	`840`	`DEFINE_INSN`
`@@ -846,7 +846,7 @@ opt_newarray_min`
`846`	`846`	`// attr bool leaf = false; /* has rb_funcall() */`
`847`	`847`	`// attr rb_snum_t sp_inc = 1 - (rb_snum_t)num;`
`848`	`848`	`{`
`849`		`- val = vm_opt_newarray_min(num, STACK_ADDR_FROM_TOP(num));`
	`849`	`+ val = vm_opt_newarray_min(ec, num, STACK_ADDR_FROM_TOP(num));`
`850`	`850`	`}`
`851`	`851`
`852`	`852`	`/* super(args) # args.size => num */`
Original file line number	Diff line number	Diff line change
`@@ -4655,7 +4655,7 @@ vm_opt_str_freeze(VALUE str, int bop, ID id)`
`4655`	`4655`	`#define id_cmp idCmp`
`4656`	`4656`
`4657`	`4657`	`static VALUE`
`4658`		`-vm_opt_newarray_max(rb_num_t num, const VALUE *ptr)`
	`4658`	`+vm_opt_newarray_max(rb_execution_context_t ec, rb_num_t num, const VALUE ptr)`
`4659`	`4659`	`{`
`4660`	`4660`	`if (BASIC_OP_UNREDEFINED_P(BOP_MAX, ARRAY_REDEFINED_OP_FLAG)) {`
`4661`	`4661`	`if (num == 0) {`
`@@ -4676,12 +4676,19 @@ vm_opt_newarray_max(rb_num_t num, const VALUE *ptr)`
`4676`	`4676`	`}`
`4677`	`4677`	`else {`
`4678`	`4678`	`VALUE ary = rb_ary_new4(num, ptr);`
`4679`		`- return rb_funcall(ary, idMax, 0);`
	`4679`	`+ const rb_callable_method_entry_t *me =`
	`4680`	`+ rb_callable_method_entry_with_refinements(rb_cArray, idMax, NULL);`
	`4681`	`+ if (me) {`
	`4682`	`+ return rb_vm_call0(ec, ary, idMax, 0, NULL, me, RB_NO_KEYWORDS);`
	`4683`	`+ }`
	`4684`	`+ else {`
	`4685`	`+ return rb_funcall(ary, idMax, 0);`
	`4686`	`+ }`
`4680`	`4687`	`}`
`4681`	`4688`	`}`
`4682`	`4689`
`4683`	`4690`	`static VALUE`
`4684`		`-vm_opt_newarray_min(rb_num_t num, const VALUE *ptr)`
	`4691`	`+vm_opt_newarray_min(rb_execution_context_t ec, rb_num_t num, const VALUE ptr)`
`4685`	`4692`	`{`
`4686`	`4693`	`if (BASIC_OP_UNREDEFINED_P(BOP_MIN, ARRAY_REDEFINED_OP_FLAG)) {`
`4687`	`4694`	`if (num == 0) {`
`@@ -4702,7 +4709,14 @@ vm_opt_newarray_min(rb_num_t num, const VALUE *ptr)`
`4702`	`4709`	`}`
`4703`	`4710`	`else {`
`4704`	`4711`	`VALUE ary = rb_ary_new4(num, ptr);`
`4705`		`- return rb_funcall(ary, idMin, 0);`
	`4712`	`+ const rb_callable_method_entry_t *me =`
	`4713`	`+ rb_callable_method_entry_with_refinements(rb_cArray, idMin, NULL);`
	`4714`	`+ if (me) {`
	`4715`	`+ return rb_vm_call0(ec, ary, idMin, 0, NULL, me, RB_NO_KEYWORDS);`
	`4716`	`+ }`
	`4717`	`+ else {`
	`4718`	`+ return rb_funcall(ary, idMin, 0);`
	`4719`	`+ }`
`4706`	`4720`	`}`
`4707`	`4721`	`}`
`4708`	`4722`