diff options
| -rw-r--r-- | ChangeLog | 12 | ||||
| -rw-r--r-- | numeric.c | 16 | ||||
| -rw-r--r-- | test/ruby/test_numeric.rb | 17 |
3 files changed, 44 insertions, 1 deletions
@@ -1,3 +1,15 @@ +Sat Jun 7 22:13:42 2014 Benoit Daloze <eregontp@gmail.com> + + * numeric.c (do_coerce): Add a warning when an exception is raised + or an invalid value is returned in #coerce called by + numeric comparison operators and the exception + thrown by the caller has no information on the failure. + In the next release such exception should not be rescued or + should be the cause of the caller exception. nil is accepted + as the "no possible coercion" return value. See #7688. + + * test/ruby/test_numeric.rb: Add corresponding test. + Sat Jun 7 18:15:33 2014 Benoit Daloze <eregontp@gmail.com> * numeric.c (bit_coerce): remove constant parameter `err' @@ -251,6 +251,12 @@ coerce_rescue(VALUE *x) return Qnil; /* dummy */ } +static VALUE +coerce_rescue_quiet(VALUE *x) +{ + return Qundef; +} + static int do_coerce(VALUE *x, VALUE *y, int err) { @@ -266,10 +272,18 @@ do_coerce(VALUE *x, VALUE *y, int err) return FALSE; } - ary = rb_rescue(coerce_body, (VALUE)a, err ? coerce_rescue : 0, (VALUE)a); + 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."); + 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; } diff --git a/test/ruby/test_numeric.rb b/test/ruby/test_numeric.rb index 674e869473..b42509dd1a 100644 --- a/test/ruby/test_numeric.rb +++ b/test/ruby/test_numeric.rb @@ -59,6 +59,23 @@ class TestNumeric < Test::Unit::TestCase end assert_equal(-1, -a) + bug7688 = '[ruby-core:51389] [Bug #7688]' + DummyNumeric.class_eval do + remove_method :coerce + def coerce(x); raise StandardError; end + end + 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 } } + + DummyNumeric.class_eval do + remove_method :coerce + def coerce(x); :bad_return_value; end + end + 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 } } + ensure DummyNumeric.class_eval do remove_method :coerce |