Type coercion issue when `to_str` raises `NoMethodError`

[nirvdrum]

While working on getting the Sorbet test suite passing on TruffleRuby, @paracycle discovered an issue with some of our type coercion logic. In this case, an object is attempted to be coerced to a String via to_str. If to_str raises a NoMethodError then some of our type coercion logic assumes the type cannot be converted and raises a TypeError rather than the NoMethodError.

A brief search indicates ToStringOrSymbolNode needs to check if the method exists. Our logic in Truffle::Type.try_convert does perform the extra respond_to? check, but it looks like that didn't get carried forward to type coercion performed in Java.

class Foo
  def to_str
    raise NoMethodError, "foo"
  end
end

"bar " + Foo.new

MRI 3.1.3 (warnings removed):

)> ruby -v type_error.rb
ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [arm64-darwin21]
type_error.rb:3:in `to_str': foo (NoMethodError)

    raise NoMethodError, "foo"
    ^^^^^
	from type_error.rb:7:in `+'
	from type_error.rb:7:in `<main>'

TruffleRuby 23.0.0-dev (a6717812, warnings removed):

> jt ruby -v type_error.rb
truffleruby 23.0.0-dev-a6717812, like ruby 3.1.3, GraalVM CE JVM [aarch64-darwin]
type_error.rb:7:in `+': no implicit conversion of Foo into String (TypeError)
	from type_error.rb:7:in `<main>'

[eregon]

Right, it seems this is a problem for most To*Node, they catch NoMethodError and they can't easily know if that's from that method call or some other method call.

I think unfortunately it's semantically incorrect in those cases to use a @Cached(parameters = "PRIVATE_RETURN_MISSING") DispatchNode (which behaves a bit like rb_check_funcall()), because e.g. that wouldn't call method_missing, but IIRC the semantics on such conversion is to call method_missing.

A possibility is to do it like ToRubyIntegerNode does it by calling a Ruby helper (rb_to_int_fallback).

[eregon]

Indeed it calls method_missing in that case on CRuby (even if respond_to? is false):

$ ruby -e 'o=Object.new; def o.method_missing(name, *); p name; "foo"; end; p "a " + o'
:to_str
"a foo"

[eregon]

Oh yeah it's really weird and full of corner cases:

ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [x86_64-linux]

$ ruby -e 'o=Object.new; def o.respond_to?(m,*); p [:resp, m]; end; def o.method_missing(name, *); p name; "foo"; end; p "a " + o'
[:resp, :to_str]
:to_str
"a foo"

$ ruby -e 'o=Object.new; def o.respond_to?(m,*); p [:resp, m]; false; end; def o.method_missing(name, *); p name; "foo"; end; p "a " + o'
[:resp, :to_str]
-e:1:in `+': no implicit conversion of Object into String (TypeError)
	from -e:1:in `<main>'

$ ruby -e 'o=Object.new; def o.method_missing(name, *); p name; "foo"; end; p o.respond_to?(:to_str); p "a " + o'
false
:to_str
"a foo"

So it does call respond_to? (or at least it calls the custom one), and if a custom respond_to? returns false it believes it and doesn't call method_missing. But if it's the default respond_to? it calls method_missing anyway :sweat_smile:

[eregon]

Right, I think this is simply the "full" semantics of rb_check_funcall in CRuby. We have them implemented in Truffle::Type.check_funcall and so I think using Truffle::Type.rb_convert_type should implement the correct semantics here.