Bug #3420
closed
Module#method calling <=> causes SystemStackError
Description
=begin
It seems the SystemStackError will be raised when compar.c calls the spaceship:
static VALUE cmp_eq(VALUE *a) { VALUE c = rb_funcall(a[0], cmp, 1, a[1]); ... ruby-1.9.2-head > BrokenComparable = Module.new { def ==(other) self <=> other end } BrokenClass = Class.new { include BrokenComparable } BrokenClass.new == nil SystemStackError: stack level too deep The current implementation of the spaceship operator ...
static VALUE rb_mod_cmp(VALUE mod, VALUE arg) { *snip* cmp = rb_class_inherited_p(mod, arg); ... ... imho tries to lookup the method in the parent classes but fails because
it's actually implemented in the Kernel module:
ruby-1.9.2-head > Object.instance_methods(false) => [] ruby-1.9.2-head > Object.superclass.instance_methods(false) => [:==, :equal?, :!, :!=, :instance_eval, :instance_exec, :__send__] ruby-1.9.2-head > Object.superclass.superclass.methods(false) => [] ruby-1.9.2-head > Object.superclass.superclass.class.instance_methods(false) => [:to_i, :to_f, :to_s, :to_a, :inspect, :&, :|, :^, :nil?, :to_r, :rationalize, :to_c] ruby-1.9.2-head > Object.superclass.superclass.class.superclass.instance_methods(false) => [] ruby-1.9.2-head > Object.superclass.superclass.class.superclass.superclass.instance_methods(false) => [:==, :equal?, :!, :!=, :instance_eval, :instance_exec, :__send__] ... ruby-1.9.2-head > Kernel.instance_methods(false) => [:nil?, :===, :=~, :!~, :eql?, :hash, :<=>, :class, :singleton_class, :clone, :dup, :initialize_dup, :initialize_clone, :taint, :tainted?, :untaint, :untrust, :untrusted?, :trust, :freeze, :frozen?, :to_s, :inspect, :methods, :singleton_methods, :protected_methods, :private_methods, :public_methods, :instance_variables, :instance_variable_get, :instance_variable_set, :instance_variable_defined?, :instance_of?, :kind_of?, :is_a?, :tap, :send, :public_send, :respond_to?, :respond_to_missing?, :extend, :display, :method, :public_method, :define_singleton_method, :__id__, :object_id, :to_enum, :enum_for] But I'm neither familiar with C nor the ruby source so this is just a guess.
For reference http://github.com/rails/rails/commit/0042f4166f783085eb909d69d542b5323d8af5d6#commitcomment-91341.
Regards
Florian Aßmann
=end
Updated by boof (Florian Aßmann) over 15 years ago
=begin
Sorry, ruby -v: ruby 1.9.2dev (2010-06-01 revision 28116) [x86_64-darwin10.3.0]
=end
Updated by boof (Florian Aßmann) over 15 years ago
=begin
Sorry again, this should be in ruby-1.9... :/
=end
Updated by judofyr (Magnus Holm) over 15 years ago
=begin
The "problem" is that Object#<=> calls #== internally:
rb_obj_cmp(VALUE obj1, VALUE obj2)
{
if (obj1 == obj2 || rb_equal(obj1, obj2)) // <- Calls rb_equal
return INT2FIX(0);
return Qnil;
}
VALUE
rb_equal(VALUE obj1, VALUE obj2)
{
VALUE result;
if (obj1 == obj2) return Qtrue; result = rb_funcall(obj1, id_eq, 1, obj2); // <- Calls #== if (RTEST(result)) return Qtrue; return Qfalse; }
=end
Updated by nobu (Nobuyoshi Nakada) over 15 years ago
- Status changed from Open to Rejected
=begin
You can feel free to shoot your foot. Ruby doesn't prohibit it.
=end
Updated by boof (Florian Aßmann) over 15 years ago
=begin
Ok, but then imho the Exception is inappropriate and most people don't expect a SystemStackError when they found a method implementation missing. For most people it isn't obvious (see commit message in the ref link) why including Comparable and not implementing <=> (because they want to delegate the call through the method_missing hook) should cause a SystemStackError.
It should respect the method_missing hook. But thanks for the answer (Magnus).
=end
Updated by matz (Yukihiro Matsumoto) over 15 years ago
=begin
Hi,
In message "Re: [ruby-core:30727] [Bug #3420] Module#method calling <=> causes SystemStackError"
on Fri, 11 Jun 2010 08:30:26 +0900, Florian Aßmann redmine@ruby-lang.org writes:
|Ok, but then imho the Exception is inappropriate and most people don't expect a SystemStackError when they found a method implementation missing. For most people it isn't obvious
Could you elaborate your opinion bit more? Most people don't redefine
comparison operator, I expect.
matz. =end
Updated by boof (Florian Aßmann) over 15 years ago
=begin
Please let me ask, why as from 1.9.2:
Sat Oct 24 13:38:45 2009 Yukihiro Matsumoto
* object.c (rb_obj_cmp): defines Object#<=>. [ruby-core:24063] Before that all ruby implementations did:
ruby-1.8.7-p249 >
BrokenComparable = Module.new { def ==(other) self <=> other end }
BrokenClass = Class.new { include BrokenComparable }
BrokenClass.new == nil
NoMethodError: undefined method <=>' for #<BrokenClass:0x10043ef08> from (irb):1:in =='
from (irb):3
Which is a lot more verbose on what happened and allowed people to hook into method missing and delegate it to a wrapped object as a SystemStackError. Now, if there is a global defined <=> for all classes which mix in the kernel module (all but BasicObject) I'd suspect it should either just work (not calling rb_equal from rb_obj_cmp) as long people are used to 1.8.x/1.9.1 or be more verbose on what happened.
Removing the call of rb_equal in rb_obj_cmp would not just save the extra test of obj identity but also work for all objects; which seems to me like the intended behavior when adding a method like <=> to almost all objects.
Otherwise it should be stated in the docs of Comparable what happens if one does not implement the <=> method and what the currently undocumented Object#<=> actually does since a SystemStackError only adds confusion and isn't obvious at all.
Regards
Florian
=end