From bc1827e8825c558bcda14a214280caf83dc1215b Mon Sep 17 00:00:00 2001 From: eregon Date: Tue, 25 Apr 2017 11:42:20 +0000 Subject: [PATCH] no longer rescue exceptions in numeric comparison operations * numeric.c (do_coerce): no more error hiding. * test/ruby/test_numeric.rb: follow the change. [Feature #7688] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@58474 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- NEWS | 6 ++++++ numeric.c | 38 ++++---------------------------------- test/ruby/test_numeric.rb | 10 ++++------ 3 files changed, 14 insertions(+), 40 deletions(-) diff --git a/NEWS b/NEWS index 5ef0bfb112..d7f79727a2 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,12 @@ with all sufficient information, see the ChangeLog file or Redmine * exception message "stream closed" is changed [Bug #13405] +* Numeric + + * Numerical comparison operators (<,<=,>=,>) no longer rescue exceptions + of #coerce. Return nil in #coerce if the coercion is impossible. + [Feature #7688] + * Regexp * Update Onigmo 6.1.1. * Support absent operator https://github.com/k-takata/Onigmo/issues/82 diff --git a/numeric.c b/numeric.c index ad7c984241..dda44c0a77 100644 --- a/numeric.c +++ b/numeric.c @@ -432,13 +432,6 @@ num_coerce(VALUE x, VALUE y) return rb_assoc_new(y, x); } -static VALUE -coerce_body(VALUE arg) -{ - VALUE *x = (VALUE *)arg; - return rb_funcall(x[1], id_coerce, 1, x[0]); -} - NORETURN(static void coerce_failed(VALUE x, VALUE y)); static void coerce_failed(VALUE x, VALUE y) @@ -453,14 +446,6 @@ coerce_failed(VALUE x, VALUE y) y, rb_obj_class(x)); } -static VALUE -coerce_rescue(VALUE arg, VALUE errinfo) -{ - VALUE *x = (VALUE *)arg; - coerce_failed(x[0], x[1]); - return Qnil; /* dummy */ -} - static VALUE coerce_rescue_quiet(VALUE arg, VALUE errinfo) { @@ -470,33 +455,18 @@ coerce_rescue_quiet(VALUE arg, VALUE errinfo) static int do_coerce(VALUE *x, VALUE *y, int err) { - VALUE ary; - VALUE a[2]; - - a[0] = *x; a[1] = *y; - - if (!rb_respond_to(*y, id_coerce)) { + VALUE ary = rb_check_funcall(*y, id_coerce, 1, x); + if (ary == Qundef) { if (err) { coerce_failed(*x, *y); } return FALSE; } - - ary = rb_rescue(coerce_body, (VALUE)a, err ? coerce_rescue : coerce_rescue_quiet, (VALUE)a); - if (ary == Qundef) { - rb_warn("Numerical comparison operators will no more rescue exceptions of #coerce"); - rb_warn("in the next release. Return nil in #coerce if the coercion is impossible."); + if (!err && NIL_P(ary)) { return FALSE; } if (!RB_TYPE_P(ary, T_ARRAY) || RARRAY_LEN(ary) != 2) { - if (err) { - rb_raise(rb_eTypeError, "coerce must return [x, y]"); - } - else if (!NIL_P(ary)) { - rb_warn("Bad return value for #coerce, called by numerical comparison operators."); - rb_warn("#coerce must return [x, y]. The next release will raise an error for this."); - } - return FALSE; + rb_raise(rb_eTypeError, "coerce must return [x, y]"); } *x = RARRAY_AREF(ary, 0); diff --git a/test/ruby/test_numeric.rb b/test/ruby/test_numeric.rb index 92717e9f3b..4f4fd5cc3e 100644 --- a/test/ruby/test_numeric.rb +++ b/test/ruby/test_numeric.rb @@ -54,18 +54,16 @@ class TestNumeric < Test::Unit::TestCase bug7688 = '[ruby-core:51389] [Bug #7688]' a = Class.new(Numeric) do - def coerce(x); raise StandardError; end + def coerce(x); raise StandardError, "my error"; end end.new - assert_raise_with_message(TypeError, /can't be coerced into /) { 1 + a } - warn = /will no more rescue exceptions of #coerce.+ in the next release/m - assert_warn(warn, bug7688) { assert_raise(ArgumentError) { 1 < a } } + assert_raise_with_message(StandardError, "my error") { 1 + a } + assert_raise_with_message(StandardError, "my error") { 1 < a } a = Class.new(Numeric) do def coerce(x); :bad_return_value; end end.new assert_raise_with_message(TypeError, "coerce must return [x, y]") { 1 + a } - warn = /Bad return value for #coerce.+next release will raise an error/m - assert_warn(warn, bug7688) { assert_raise(ArgumentError) { 1 < a } } + assert_raise_with_message(TypeError, "coerce must return [x, y]") { 1 < a } end def test_singleton_method