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

improved method arities on become_java! #3779

Merged
merged 6 commits into from
Apr 12, 2016
Merged

improved method arities on become_java! #3779

merged 6 commits into from
Apr 12, 2016

Conversation

kares
Copy link
Member

@kares kares commented Apr 4, 2016

just in time for 9.1 as this might be considered a "breaking" change at the binary (Java reflective) level.

the change is pretty much explained in the commit message: 2f363d0

previously all method arities ended up as var-args : foo(IRubyObject[] args)

now for fixed arities we'll have correct number of args e.g. foo(IRubyObject arg1)
... we still generate var-args: foo(IRubyObject[] args) for all non-fixed arity sizes!

most users, of Ruby user-types entering Java land, complained about the previous behaviour - so this is a welcoming change. Ruby fixed arities will now be much more intuitive on the Java side.

// cc @Lan5432
... if you're interested you could confirm whether this resolves some/all of the related issues :

@kares kares added this to the JRuby 9.1.0.0 milestone Apr 4, 2016
@Lan5432
Copy link
Contributor

Lan5432 commented Apr 5, 2016

If you mean just applying the test cases provided by each issue's discussion, I can do that, sorry I didn't get to it earlier

So far, 449 is not fixed, reduced test case provided by Byteit:

require 'jruby/core_ext'
class App2 < javafx.application.Application; def start(stage); end; end
App2.become_java!
App2.java_class.to_java(java.lang.Class).getConstructor([].to_java(java.lang.Class)).newInstance([].to_java java.lang.Object)

Still breaks with InstantiationException:

2.2.1 :009 > require 'jruby/core_ext'
 => true 
2.2.1 :010 > class App2 < javafx.application.Application; def start(stage); end; end
 => :start 
2.2.1 :011 > App2.become_java!
 => nil 
2.2.1 :012 > App2.java_class.to_java(java.lang.Class).getConstructor([].to_java(java.lang.Class)).newInstance([].to_java java.lang.Object)
Java::JavaLang::InstantiationException: 
        from java.lang.reflect.Constructor.newInstance(java/lang/reflect/Constructor.java:423)
        from java.lang.reflect.Method.invoke(java/lang/reflect/Method.java:498)
        from RUBY.<eval>((irb):12)
        from org.jruby.RubyKernel.eval(org/jruby/RubyKernel.java:983)
        from org.jruby.RubyKernel.loop(org/jruby/RubyKernel.java:1290)
        from org.jruby.RubyKernel.catch(org/jruby/RubyKernel.java:1103)
        from org.jruby.RubyKernel.catch(org/jruby/RubyKernel.java:1103)
        from home.lan5432.Apps.JRuby.jruby.bin.jirb.<top>(/home/lan5432/Apps/JRuby/jrub/bin/jirb:13)
        from java.lang.invoke.MethodHandle.invokeWithArguments(java/lang/invoke/MethodHandle.java:627)

Would like to notice that it may be because I have the wrong files, I'm not 100% sure.
I did checkout branches and your changes of this request are in my files, so I have no reasons to doubt that I'm using your changes.

I will test the rest of the cases, but as I reported on IRC, the issue 3206 also breaks, unexpectedly:

2.2.1 :001 > class Example
2.2.1 :002?>   java_signature "java.lang.String hello()"
2.2.1 :003?>   def hello
2.2.1 :004?>     return 'hello world from JRuby'
2.2.1 :005?>     end
2.2.1 :006?>   end
 => :hello 
2.2.1 :007 > ex = Example.become_java!
NoMethodError: undefined method `become_java!' for Example:Class
        from org/jruby/RubyBasicObject.java:1603:in `method_missing'
        from (irb):7:in `<eval>'
        from org/jruby/RubyKernel.java:983:in `eval'
        from org/jruby/RubyKernel.java:1290:in `loop'
        from org/jruby/RubyKernel.java:1103:in `catch'
        from org/jruby/RubyKernel.java:1103:in `catch'
        from /home/lan5432/Apps/JRuby/jruby/bin/jirb:13:in `<top>'

Problem is, as you saw on the test for 449, it should not break, the only key difference between this test and the other is that the test for 449 inherits an actual Java class

I will test the other cases later on, but if these problems are of my build or new problems that arise with these changes, I'm worried. Of course, not to tick out the possibility that I may've done something wrong using Git, but all file checkings prove otherwise.

@kares kares force-pushed the test-become-java branch from 6df7e04 to eb7c236 Compare April 5, 2016 06:08
@kares
Copy link
Member Author

kares commented Apr 5, 2016

#3206 missed a require but you might have noticed that you did a require 'jruby/core_ext' in the previous one (also if you grep become_java you would have found that its in the jruby/core_ext.rb file).
as noted #3206 is not complete script (need to pass the ex into a Java API that does reflection) - but I do assume it's fine since there's a similar test-case already added.

#449 is definitely not fixed - so I was wrong assuming its just the var-args, thanks for confirming that one!

@Lan5432
Copy link
Contributor

Lan5432 commented Apr 5, 2016

Yeah, I knew about the require statement, I must've assumed that I already imported it (I did all the tests in the same irb session) so that's my bad, sorry.

I got a problem when instantiating the result of become_java on #3206 because it says "new" doesn't exist for that class. I know it's not from the issue itself, but it might be important for the others

2.2.1 :009 > ex = Example.become_java!
 => class rubyobj.Example 
2.2.1 :010 > ex.new
NoMethodError: undefined method `new' for class rubyobj.Example:Java::JavaLang::Class
        from (irb):10:in `<eval>'
        from org/jruby/RubyKernel.java:983:in `eval'
        from org/jruby/RubyKernel.java:1290:in `loop'
        from org/jruby/RubyKernel.java:1103:in `catch'
        from org/jruby/RubyKernel.java:1103:in `catch'
        from /home/lan5432/Apps/JRuby/jruby/bin/jirb:13:in `<top>'

In issue #1925 I cannot complete the tests because I don't know what I need to do to generate "proxied_java_object"

#3206

I can confirm it's not fixed, specified java signature is still not respected at runtime, provided test case yields same problem:

2.2.1 :017 > class RubyClz
2.2.1 :018?>   java_signature "void hello(javafx.event.Event)"
2.2.1 :019?>   def goodbye(e)
2.2.1 :020?>     puts "I was called :D"
2.2.1 :021?>     end
2.2.1 :022?>   end
 => :goodbye 
2.2.1 :023 > RubyClz.become_java!
 => class rubyobj.RubyClz 
2.2.1 :024 > p RubyClz.java_class.methods.sort{|a, b| a.name <=> b.name }.find_all{|x| x.to_s.include? "hello" or x.to_s.include? "goodby"}.map(&:to_s)
["public org.jruby.runtime.builtin.IRubyObject rubyobj.RubyClz.goodbye(org.jruby.runtime.builtin.IRubyObject[])"] => ["public org.jruby.runtime.builtin.IRubyObjectrubyobj.RubyClz.goodbye(org.jruby.runtime.builtin.IRubyObject[])"] 

As opposed to intented signature void hello(javafx.event.Event)

#3454

Also seems to remain unfixed, I used the same code provided in the example test case, exported it into a .jar, wrote the rb file and executed it (here you have the code for easy testing)
Test3454.zip

jruby/bin/jruby -S ./teste.rb 
java.lang.InstantiationException: org.jruby.RubyClass
        at java.lang.Class.newInstance(Class.java:427)
        at Teste.executaJob(Teste.java:4)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.jruby.javasupport.JavaMethod.invokeDirectWithExceptionHandling(JavaMethod.java:453)
        at org.jruby.javasupport.JavaMethod.invokeDirect(JavaMethod.java:314)
        at org.jruby.java.invokers.InstanceMethodInvoker.call(InstanceMethodInvoker.java:45)
        at org.jruby.runtime.callsite.CachingCallSite.cacheAndCall(CachingCallSite.java:313)
        at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:163)
        at $_dot_.teste.invokeOther17:executaJob(./teste.rb)
        at $_dot_.teste.RUBY$script(./teste.rb:12)
        at java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:627)
        at org.jruby.ir.Compiler$1.load(Compiler.java:111)
        at org.jruby.Ruby.runScript(Ruby.java:823)
        at org.jruby.Ruby.runScript(Ruby.java:815)
        at org.jruby.Ruby.runNormally(Ruby.java:753)
        at org.jruby.Ruby.runFromMain(Ruby.java:574)
        at org.jruby.Main.doRunFromMain(Main.java:415)
        at org.jruby.Main.internalRun(Main.java:310)
        at org.jruby.Main.run(Main.java:239)
        at org.jruby.Main.main(Main.java:201)
Caused by: java.lang.NoSuchMethodException: org.jruby.RubyClass.<init>()
        at java.lang.Class.getConstructor0(Class.java:3082)
        at java.lang.Class.newInstance(Class.java:412)
        ... 22 more

The last bit of the stack trace was not presented by the issue owner, I assume because it was not necessary to troubleshoot the problem.

kares added 3 commits April 6, 2016 09:53
…ing) signatures

previously all method arities ended up as var-args : `foo(IRubyObject[] args)`

now for fixed arities we'll have correct number of args e.g. `foo(IRubyObject arg1)`
... we still generate var-args: `foo(IRubyObject[] args)` for all non-fixed arity sizes!

resolves #3206 and likely #449 also #3366 (java_signature is now honored)
@kares kares force-pushed the test-become-java branch from eb7c236 to a06ac16 Compare April 6, 2016 07:54
@kares
Copy link
Member Author

kares commented Apr 6, 2016

OK, thanks I rebased and added some (minor) improvements.

I got a problem when instantiating the result of become_java on #3206 because it says "new" doesn't exist for that class. I know it's not from the issue itself, but it might be important for the others

2.2.1 :009 > ex = Example.become_java!
=> class rubyobj.Example
2.2.1 :010 > ex.new
NoMethodError: undefined method `new' for class rubyobj.Example:Java::JavaLang::Class

never worked, and we likely do not need a new on a java.lang.Class much (there's new_instance).

#3206 notes on issue - its fixable by re-architecting java_signature to apply for next method def

…(closing #3454)

... much less confusing than for user classes to see the nearest reified class in the inheritance hierarchy (without explicitly doing `become_java!`)
@kares
Copy link
Member Author

kares commented Apr 6, 2016

added auto-reify behavior of Ruby classes when they're being converted to a java.lang.Class

@kares kares merged commit 2876388 into master Apr 12, 2016
@kares kares deleted the test-become-java branch April 12, 2016 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants