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

Conversation

Projects
None yet
2 participants
@kares
Member

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

This comment has been minimized.

Show comment
Hide comment
@Lan5432

Lan5432 Apr 5, 2016

Contributor

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.

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

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 5, 2016

Member

#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!

Member

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

This comment has been minimized.

Show comment
Hide comment
@Lan5432

Lan5432 Apr 5, 2016

Contributor

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.

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 some commits Mar 19, 2016

[ji] refactor Ruby class reify to generate more accurate (arity match…
…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)
move RubyClass' reify() ASM generation into an inner Reificator class
... one synchronized reify method is enough
@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 6, 2016

Member

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

Member

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

[ji] when a Ruby Class is being coerced to a Java class - auto-reify …
…(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

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 6, 2016

Member

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

Member

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

2 of 4 checks passed

continuous-integration/appveyor/branch AppVeyor build failed
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@kares kares deleted the test-become-java branch Apr 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment