oracle / truffleruby

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

Wrong object returned from Polyglot.eval? #1534

Open rbotafogo opened 5 years ago

rbotafogo commented 5 years ago

I have the following code:

mpg = Polyglot.eval("R", <<-R)
  mtcars[1,]['mpg'] == 21
R
puts mpg

The output is (Ruby):

> true

Should this be an R Vector:

[1] TRUE

Thanks!

eregon commented 5 years ago

Thank you for the report, I can reproduce with GraalVM 1.0.0-rc10.

This is due to the implementation of to_s on foreign objects which does: https://github.com/oracle/truffleruby/blob/1340cb43b9a2e71cb46eff47642cbf99eb6c36f8/src/main/ruby/core/truffle/interop.rb#L181-L187

And this one-element logical vector in R "unboxes" to the boolean true. Then true is shown as it would be as a Ruby boolean.

A bit of debugging shows:

mpg = Polyglot.eval("R", <<-R)
  result <- mtcars[1,]['mpg'] == 21
  print(result)
  result
R
#            mpg
# Mazda RX4 TRUE
puts Truffle::Debug.java_class_of(mpg)
# RLogicalVector
puts mpg
# true
p mpg
# interop.rb:8:in `size?': Truffle doesn't have a case for the # org.truffleruby.interop.InteropNodesFactory$HasSizeNodeFactory$HasSizeNodeGen node with values of type  java.lang.Boolean=true (TypeError)
#   from test.rb:8:in `inspect_foreign'

The last one is a bug, but if it went past that check it would just show something like #<Foreign:0x123> which is not very useful, because inspect_foreign also does object = Truffle::Interop.unbox_if_needed(object). If it didn't unbox, then we could see something like <Foreign:0x123 [true]>, which is a Ruby representation of it (there is another "bug" in there because the pointer? check is done before).

In general @chumer advises to print foreign objects as if they were objects of the current language, but here it loses quite a lot of information, and it proves really difficult. Shouldn't we have a TO_STRING or INSPECT message, or just call the Java toString? Only R knows how to encode such a data type in a meaningful string.

eregon commented 4 years ago

This is probably fixed now, but we need to check. (puts will call toDisplayString() which would show the result like R does)

eregon commented 1 year ago

On 22.3.1:

mpg = Polyglot.eval("R", <<-R)
  result <- mtcars[1,]['mpg'] == 21
  print(result)
  result
R
#            mpg
# Mazda RX4 TRUE
puts Truffle::Debug.java_class_of(mpg)
# RLogicalVector
puts mpg
# true
p mpg
# #<TrueClass:0x584585f1>
p mpg.class.ancestors
# [Polyglot::ForeignArray, Polyglot::ArrayTrait, Polyglot::IterableTrait, Enumerable, Polyglot::ForeignObject, Polyglot::ObjectTrait, Object, Kernel, BasicObject]

What's complicated here is this is both an Array and it has isBoolean()=true. I think it'll be clearer after the changes in the 23.0 release, I'll try then.