oracle / truffleruby

A high performance implementation of the Ruby programming language, built on GraalVM.
https://www.graalvm.org/ruby/
Other
3.02k stars 185 forks source link

`Kernel#inspect` raises exception if `self.class` is undefined #2324

Closed kachick closed 3 years ago

kachick commented 3 years ago

Is this an intentional behavior?

obj = Object.new
class << obj
  undef_method :class
end
print obj.inspect

ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin20]

#<Object:0x00007fe84f0a8c48>%

truffleruby 21.1.0-dev-972a0521, like ruby 2.7.2, GraalVM CE Native [x86_64-darwin]

<internal:core> core/kernel.rb:375:in `inspect': undefined method `class' for #<Object:0x18> (NoMethodError)
    from -:5:in `<main>'

The cause looks depending self.class in inspect as https://github.com/oracle/truffleruby/blob/310f0550b8cada1c89e2172182c7ab63e5014d58/src/main/ruby/truffleruby/core/kernel.rb#L375

I think it might be an intentional behavior. But I found this in testing my old tiny gem as https://github.com/kachick/validation/pull/14/checks?check_run_id=2317029150.

I would hope Kernel#inspect basically does not raise errors even if the code is not unsafe 😄

chrisseaton commented 3 years ago

Much of TruffleRuby is implemented in Ruby. This is inherited from Rubinius, and it's also increasingly what MRI and JRuby are doing, and what Topaz did.

This means that monkey patching parts of the core library may break other parts of the core library.

It wouldn't say it's intentional behaviour, but it is an intentional trade-off. I think JRuby and MRI agree that the trade-off is worth-it since they're doing it more themselves.

I would say that this is the first time ever that anyone has complained with a non-contrived example. I would not be surprised if this breaks on MRI or JRuby in the future.

Can you talk us through your test? I can see what it does but I'm missing the context on what you're really testing here?

https://github.com/kachick/validation/blob/db53feae8e081ea328240ccbd1c908b36dc84842/test/test_validation.rb#L109-L125

eregon commented 3 years ago

We should use Truffle::Type.object_class(self) instead of self.class there to fix this issue. Truffle::Type.object_class(self) should probably be a Primitive like Primitive.object_class(self).

kachick commented 2 years ago

Thank you for fix! 🙏