From: "Eregon (Benoit Daloze)" Date: 2013-05-05T01:24:02+09:00 Subject: [ruby-core:54782] [ruby-trunk - Feature #7688] Error hiding with rb_rescue() on Comparable#==, #coerce and others Issue #7688 has been updated by Eregon (Benoit Daloze). Hello, matz (Yukihiro Matsumoto) wrote: > I agree with most of your changes in the patch, especially using rb_check_funcall instead of rb_rescue. > But I personally dislike the equal operator (==) to raise error, since equal comparison is so fundamental, and most of us write code that do not expect exceptions from == operator. > > Matz. Many classes including Comparable already define #== themselves and most definitions of #== are made by the user or libraries (not using Comparable#==), so I think this rescue clause protects only few users and I believe #== methods should in any case be written to support comparison with other objects without raising an exception (unless explicitly intended). On the other hand, if #<=> method does raise an exception, I would find it useful as it tells me I am probably comparing things I did not intend to (e.g.: objects of different classes/hierarchies). And it is more consistent with other uses of #<=> (and other definitions of #==). Finding why my objects are never #== because I made a typo or some sensible error in #<=> might require a lot more time to debug than just seeing the exception reported in the #<=> of #==, which is straightforward to fix. About writing code not expecting exceptions, I think most code is in this case and the fail-fast principle is great in Ruby. As #== is a method, I think it should not be treated specially even if from a mathematical or logical point of view it should never raise any exception (that being left to the programmer). ---------------------------------------- Feature #7688: Error hiding with rb_rescue() on Comparable#==, #coerce and others https://bugs.ruby-lang.org/issues/7688#change-39125 Author: Eregon (Benoit Daloze) Status: Open Priority: Normal Assignee: matz (Yukihiro Matsumoto) Category: core Target version: next minor Hello, I believe error hiding is harmful because very dangerous (it forgets errors which is likely unexpected) and hard to debug. But I guess the compatibility is the main reason to keep these cases. In the cases of Comparable#== and #coerce, I believe it is not worth to be compatible with this dangerous behavior as it will at worse help to discover bugs in #<=> and #coerce methods which would raise an exception. I doubt anyone rely on this and the #coerce spec (see #7645) itself makes me think this is unexpected behavior. It would also simplify the spec, and no specific #coerce behavior would be needed to be defined as it would behave as a simple method call without edge cases. So I think rb_rescue() or rb_rescue2(..., GenericErrorClass, ...) should be avoided if possible. I analyzed these in the code base and it is used in a couple places: * compar.c in cmp_equal(): this is the main offender in this regard with #coerce * numeric.c in rb_num_coerce_{cmp,relop}() which call do_coerce(,,FALSE): This is the current subject of #7645. * io.c in io_close(): to avoid raising if #close fails, which is likely less problematic, although it would be nicer to rescue only IO-related errors and warn when an exception is raised. * range.c in range_init(): this is to provide a clearer error. I think it would be nice to show the original error as well. Removing the general rescue in cmp_equal() revealed a couple bugs in RDoc due to this problem. I guess there are many others in the wild. Can we please remove this anti-pattern? I believe impact is only positive and that it should be done as soon as possible. What do you think? -- http://bugs.ruby-lang.org/