Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File.directory? calls to_i in RC15, but not in RC14 #1665

Closed
rbotafogo opened this issue Apr 16, 2019 · 12 comments
Closed

File.directory? calls to_i in RC15, but not in RC14 #1665

rbotafogo opened this issue Apr 16, 2019 · 12 comments
Assignees

Comments

@rbotafogo
Copy link
Contributor

The following code

Rscript --jvm --polyglot -e 'eval.polyglot("ruby", "\
$LOAD_PATH.unshift %q(/home/rbotafogo/desenv/galaaz/lib)\
require(%q(galaaz))\
class RbChunk\
end\
RbChunk.new\
RbChunk.instance_eval(%q(File.directory?(%q(.))))
")'

When executed in RC14 returns the expected result '[1] TRUE', since '.' is a directory and Ruby 'true' becomes R vector with 1 element 'TRUE'.

Attaching package: ‘rlang’

The following object is masked from ‘package:formula.tools’:

    env


Attaching package: ‘purrr’

The following objects are masked from ‘package:rlang’:

    %@%, as_function, flatten, flatten_chr, flatten_dbl, flatten_int,
    flatten_lgl, flatten_raw, invoke, list_along, modify, prepend,
    splice

[1] TRUE

Now, when ran against RC15 results in:

Attaching package: ‘rlang’

The following object is masked from ‘package:formula.tools’:

    env


Attaching package: ‘purrr’

The following objects are masked from ‘package:rlang’:

    %@%, as_function, flatten, flatten_chr, flatten_dbl, flatten_int,
    flatten_lgl, flatten_raw, invoke, list_along, modify, prepend,
    splice

Error in polyglot evaluation : Method to_i not found in R environment (NoMethodError)
    at <ruby> 
R::Support.eval(/home/rbotafogo/desenv/galaaz/lib/R_interface/rsupport.rb:89:3503-3580)
    at <ruby> 

R::Support.exec_function_name(/home/rbotafogo/desenv/galaaz/lib/R_interface/
      rsupport.rb:269:10237-10276)
	    at <ruby> 

 R::Object#method_missing(/home/rbotafogo/desenv/galaaz/lib/R_interface/
      robject.rb:183:7683-7741)
 at <ruby> <top (required)>((eval):1:0-21)
 at <ruby> null(Unknown)

Note that although Galaaz was required, there is no call to anything in Galaaz. Class RbChunk was locally defined and is an empty class. Then I evaluate File.directory?('.') in the scope of RbChunk. I can't see why 'method_missing' is called with R::Object, since I'm giving a String '.' to File.directory?

I might have done something strange, but I really can't see it.

Adding the following code at the beginning of method_missing:

def method_missing(symbol, *args, &block)
      puts "******* method_missing"
      puts caller
      puts self
      puts symbol
      puts args
 ...
end

shows:

******* method_missing
["(eval):1:in `attach_function_eagerly'", "(eval):1:in `truffleposix_stat_mode'", "(eval):1:in 
`directory?'", "(eval):1:in `<top (required)>'", "<eval wrapper>:7:in `instance_eval'", "<eval 
wrapper>:7:in `<top (required)>'", "<REPL>:1:in `<repl wrapper>'", "<REPL>:1"]
uint32[[-2:-1]]
to_i
[]

I couldn't reproduce the error without loading Galaaz.

Thanks

@eregon eregon self-assigned this Apr 16, 2019
@eregon
Copy link
Member

eregon commented Apr 16, 2019

Thank you for the report.

I'm confused by the following lines in galaaz master:
https://github.com/rbotafogo/galaaz/blob/2e73b3019af705a2a00df0c507bfbabcee877c4a/lib/R_interface/rsupport.rb#L88-L94

    def self.eval(string)
      begin
        Polyglot.eval("R", string)
      rescue RuntimeError
        raise NoMethodError.new("Method #{string} not found in R environment")
      end
    end

Throwing a NoMethodError manually is already unusual, but then assuming it's a NoMethodError without checking what was the RuntimeError seems incorrect in general.

From the error above, it looks like galaaz is trying to Polyglot.eval("R", "to_i")?

@rbotafogo
Copy link
Contributor Author

rbotafogo commented Apr 17, 2019 via email

@eregon
Copy link
Member

eregon commented Apr 28, 2019

I'm trying to debug this in RC16, the first thing I noticed is the same issue as #1653:

Exception: `TypeError' /home/eregon/.rubies/graalvm-1.0.0-rc16/jre/languages/ruby/lib/mri/rubygems/bundler_version_finder.rb:51:in `coerce_to_failed' - Coercion error: nil.to_str => String failed

But then once I worked around that (with $0=%q(test.R)\ as the first line of the eval), I get the expected:

...
Attaching package: ‘rlang’

The following object is masked from ‘package:formula.tools’:

    env


Attaching package: ‘purrr’

The following objects are masked from ‘package:rlang’:

    %@%, as_function, flatten, flatten_chr, flatten_dbl, flatten_int,
    flatten_lgl, flatten_raw, invoke, list_along, modify, prepend,
    splice

[1] TRUE

That's using the latest Galaaz gem release (0.4.8).

@eregon
Copy link
Member

eregon commented Apr 28, 2019

I can reproduce with Galaaz master:

print("Hi")

res <- eval.polyglot("ruby", "\
$0=%q(test.R)\
$LOAD_PATH.unshift %q(/home/eregon/code/galaaz/lib)\
require(%q(galaaz))\
class RbChunk\
end\
RbChunk.new\
RbChunk.instance_eval(%q(File.directory?(%q(.))))
")

print(res)
$ Rscript --jvm --polyglot --ruby.debug --experimental-options --ruby.did-you-mean=false test.R
[1] "Hi"

Attaching package: ‘rlang’

The following object is masked from ‘package:formula.tools’:

    env


Attaching package: ‘purrr’

The following objects are masked from ‘package:rlang’:

    %@%, as_function, flatten, flatten_chr, flatten_dbl, flatten_int,
    flatten_lgl, flatten_raw, invoke, list_along, modify, prepend,
    splice

Exception `RuntimeError' at /home/eregon/code/galaaz/lib/R_interface/rsupport.rb:88:in `eval' - Error: object 'to_i' not found (RError)
Translated to internal error
Exception: `NoMethodError' /home/eregon/code/galaaz/lib/R_interface/rsupport.rb:90:in `eval' - Method to_i not found in R environment
Error in polyglot evaluation : Method to_i not found in R environment (NoMethodError)
	at <ruby> R::Support.eval(/home/eregon/code/galaaz/lib/R_interface/rsupport.rb:90:3566-3643)
	at <ruby> R::Support.exec_function_name(/home/eregon/code/galaaz/lib/R_interface/rsupport.rb:270:10300-10339)
	at <ruby> R::Object#method_missing(/home/eregon/code/galaaz/lib/R_interface/robject.rb:176:7561-7619)
	at <ruby> <top (required)>((eval):1:0-21)
	at <ruby> null(Unknown)

@eregon
Copy link
Member

eregon commented Apr 28, 2019

Kernel#puts is redefined in https://github.com/rbotafogo/galaaz/blob/a670116b0251a3c55ad4caac437ebed4e6be04fd/lib/R_interface/ruby_extensions.rb#L24-L31, that's a bit confusing and that's the reason we see the unexpected output for puts caller.

The actual issue is that https://github.com/rbotafogo/galaaz/blob/a670116b0251a3c55ad4caac437ebed4e6be04fd/lib/R_interface/ruby_extensions.rb#L178-L180 redefines Symbol#[] and that's used by our internal implementation of FFI to call truffleposix_stat_mode for File.directory? (nfi_return_type is a Symbol):

unsigned_return_type = 1 << nfi_return_type[-2..-1].to_i

Symbol#[] is a standard Ruby method: http://ruby-doc.org/core-2.6.3/Symbol.html#method-i-5B-5D

I can workaround in that code to use String#[] instead like nfi_return_type.to_s[-2..-1].to_i, and that fixes this example. I'll do that change in TruffleRuby.
But this shows overriding Symbol#[] might have non-trivial consequences.

@eregon
Copy link
Member

eregon commented Apr 28, 2019

Other notes:

  • R::Object#method_missing can use just super or super(symbol, *args, &block) to trigger a NoMethodError, this will gives better information.
  • To find out which file/line called to_i, the easiest for now is with:
    --ruby.core-load-path=/path/to/truffleruby-checkout-of-the-same-version/src/main/ruby and that gives in this case:
/home/eregon/code/galaaz/lib/R_interface/rsupport.rb:272:in `exec_function_name'
/home/eregon/code/galaaz/lib/R_interface/robject.rb:176:in `method_missing'
/home/eregon/code/truffleruby-ws/truffleruby/src/main/ruby/core/posix.rb:121:in `attach_function_eagerly'
/home/eregon/code/truffleruby-ws/truffleruby/src/main/ruby/core/posix.rb:90:in `truffleposix_stat_mode'
/home/eregon/code/truffleruby-ws/truffleruby/src/main/ruby/core/file.rb:372:in `directory?'
(eval):1:in `<top (required)>'
<eval wrapper>:8:in `instance_eval'
<eval wrapper>:8:in `<top (required)>'
/home/eregon/tmp/test.R:3:in `<repl wrapper>'
/home/eregon/tmp/test.R:1

(truffleruby/src/main/ruby/core/posix.rb:121 being the one calling to_i)

It's unfortunate that caller does not show core library files, but this is required for Ruby compatibility.

@eregon eregon added the bug label Apr 28, 2019
@rbotafogo
Copy link
Contributor Author

rbotafogo commented Apr 28, 2019 via email

@eregon
Copy link
Member

eregon commented Apr 28, 2019

  • To find out which file/line called to_i

Another and probably simpler way is using Truffle::Debug.print_backtrace along with --ruby.backtraces-hide-core-files=false on the command line:

/home/eregon/code/galaaz/lib/R_interface/rsupport.rb:90:in `print_backtrace'
	from /home/eregon/code/galaaz/lib/R_interface/rsupport.rb:90:in `eval'
	from /home/eregon/code/galaaz/lib/R_interface/rsupport.rb:273:in `exec_function_name'
	from /home/eregon/code/galaaz/lib/R_interface/robject.rb:176:in `method_missing'
	from resource:/truffleruby/core/posix.rb:121:in `attach_function_eagerly'
	from resource:/truffleruby/core/posix.rb:90:in `truffleposix_stat_mode'
	from resource:/truffleruby/core/file.rb:372:in `directory?'
        ...

@rbotafogo
Copy link
Contributor Author

rbotafogo commented Apr 28, 2019 via email

@eregon
Copy link
Member

eregon commented Apr 28, 2019

I redefine Kernel#puts because for some weird reason if I don't do it, then nothing gets printed. Can you try running: puts R.c(1, 2, 3) with both puts redefined and not? You'll see that when puts is redefined it does print fine, without it nothing gets printed on the output.

You are right, I can reproduce, that must be a bug. I'll file a bug.

I'm not sure I understand that R::Object#method_missing can just call super. This method has a lot of code to call R when the method is missing, for example, R.c(1, 2, 3) will go through method missing.

I meant for the to_ary case, so raise NoMethodError if name == "to_ary" can become super(symbol, *args, &block) if symbol == :to_ary which is better for debugging information (the exception knows the method name and the receiver and has a proper message).

@eregon
Copy link
Member

eregon commented Apr 28, 2019

@rbotafogo I filed #1679.

graalvmbot pushed a commit that referenced this issue May 23, 2019
* Symbol#[] might be overridden, for instance see
  #1665
@eregon
Copy link
Member

eregon commented May 23, 2019

I merged a fix to use String#[] instead of Symbol#[] in Truffle::POSIX in 70c7144.
It will be in the next release.
@rbotafogo Thank you for the report!

@eregon eregon closed this as completed May 23, 2019
@eregon eregon added this to the 20.0-beta1 milestone May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants