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

java_signature not honored at runtime become_java! #3366

Closed
byteit101 opened this issue Oct 2, 2015 · 17 comments
Closed

java_signature not honored at runtime become_java! #3366

byteit101 opened this issue Oct 2, 2015 · 17 comments

Comments

@byteit101
Copy link
Member

On both 1.7.19 and 9.0.1.0:

require 'jruby/core_ext'

class RubyClz
  java_signature "void hello(javafx.event.Event)"
  def goodbye(e)
    puts "I was called!"
  end
end
RubyClz.become_java!
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[])"] instead of ["public void hello(javafx.event.Event)"]
@headius
Copy link
Member

headius commented Oct 9, 2015

Note that neither the signature types nor the method name are honored because java_signature is still a no-op at runtime. There's no reason it needs to be.

@headius
Copy link
Member

headius commented Jan 20, 2016

Punt to 9.1 since it's a fairly new addition to runtime class generation.

kares added a commit that referenced this issue Apr 4, 2016
…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 added a commit that referenced this issue Apr 6, 2016
…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
Copy link
Member

kares commented Apr 6, 2016

the problem here is how currently java_signature maps to a method - simply a method name is assumed to match an existing method (does not take the following method into consideration) i.e. java_signature "void goodbye(javafx.event.Event)" already works.

@headius
Copy link
Member

headius commented Apr 28, 2016

Obviously requires more implementation work and we did not get to it. New feature, way too late in 9.1 cycle, bumping.

@enebo enebo removed this from the JRuby 9.1.2.0 milestone May 23, 2016
@pilhuhn
Copy link
Contributor

pilhuhn commented Jul 13, 2016

Is there any way for someone who is affected by this, but who does not know too much (=nothing:) about JRuby internals, to help here?

@kares
Copy link
Member

kares commented Jul 13, 2016

@pilhuhn likely yes - its not that hard but you will need to learn some of the (related) JRuby internals.

"easily" fixing this causes compatibility issues (thus an undecided part) - since currently java_signature 'foo ...' will work even when def foo happens after the java_signature call (its a simple name based mappings). so maybe it would need to be done in two steps with some deprecation warnings first before changing the behaviour to work like Ruby constructs such as test-unit's test method "annotations".

@headius
Copy link
Member

headius commented Aug 11, 2016

@kares is right...if the name in your signature matches one in the class at the time you call become_java! it does work. So in @byteit101's original example, changing "hello" to "goodbye" works.

So there's an up side and a down side to this...

On the up side, you can put these java_signature lines anywhere you want; they don't have to be immediately before the method to which they're supposed to dispatch.

On the down side, this means you can't change the Ruby method to which they're supposed to dispatch. For most methods this isn't a big deal, but there are Ruby method names that aren't valid in Java (e.g. "become_java!") and many (most?) users will want to specify the Java-facing method as camelCase.

Soooo...I could go either way on this. The old jrubyc + java_signature stuff was always half-baked and is effectively deprecated, so the behavior it follows (java_signature applies to immediately-following method) is not really a consideration. And the limitations of method naming are a bit annoying.

However, the new behavior we're talking about has limitations as well. It can't be used to provide multiple overloads for a single method, since each java_signature call would overwrite the previous one. It also requires careful consideration of the structure of code, so the java_signature and the Ruby method it applies to do not get separated. And because it always expects to bind to a specific method, there'd be no easy way to attach method signatures to a method_missing-based class.

Perhaps we should introduce a new overload of java_signature that takes a Ruby method name to call? In that case, it would unconditionally define the Java method you provide to call that named Ruby method. Perhaps that would be the best of both worlds.

class Foo
  java_signature :baz, "void bar()"
  java_signature :baz, "void bar(String)"

  def baz(name = "baz")
    puts name
  end
end

@kares
Copy link
Member

kares commented Aug 12, 2016

I think currently signatures also re-def each other (last one is used only) :

  java_signature "void bar()"
  java_signature "void bar(String)"

... does only use the later one (Java overrides are not supported).

so handling that would be a breaking change - but I guess it might be considered a bug and thus fine for a point release.

regarding the syntax - throwing in another form for consideration (~ similar to field_reader) :

  java_signature "void bar()" => :baz
  java_signature "void bar(String)" => :baz

  def baz(name = "baz")
    puts name
  end

@headius
Copy link
Member

headius commented Aug 12, 2016

I think we're going to punt this to 9.1.4.0 (or later) since there's a reasonable workaround (use the exact name of a real method) and any changes would be new and/or breaking.

The => syntax isn't bad. It would allow defining multiple signatures in a single call too.

@headius headius modified the milestones: JRuby 9.1.4.0, JRuby 9.1.3.0 Aug 12, 2016
@pilhuhn
Copy link
Contributor

pilhuhn commented Aug 12, 2016

So you think that JRubyFX#100 can be solved with this workaround?

Am 12. August 2016 19:42:52 schrieb Charles Oliver Nutter
notifications@github.com:

I think we're going to punt this to 9.1.4.0 (or later) since there's a
reasonable workaround (use the exact name of a real method) and any changes
would be new and/or breaking.

The => syntax isn't bad. It would allow defining multiple signatures in a
single call too.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#3366 (comment)

@headius
Copy link
Member

headius commented Aug 12, 2016

@pilhuhn Well I think so, but it's not clear from the description of jruby/jrubyfx#100 what needs to be done. If the signature names match the method names, it should work. If they don't (or can't) a separate method that DOES match could be added:

class Foo
  def ugly?
    true
  end

  alias isUgly ugly?
  java_signature "boolean isUgly()"
end

@byteit101
Copy link
Member Author

The original reason why I needed renaming is that the resulting object must be a valid ruby and java object. The ruby object has initialize as the ctor, and the java object ctor is ruby initialize. However, the java object must respond to a different method that is a java interface initialize, which for reasons of java reflection must be just so or else everything fails horribly. I remember getting either the java side to work, or the ruby side, but not both, which was required. I'll see if I can dig up my original code for this.

@kares
Copy link
Member

kares commented Aug 15, 2016

initialize Java method might need a "breaking" change as its currently problematic: #3807
... and has been so for a long time, maybe it will be reconsidered for next major release 9.2

@byteit101
Copy link
Member Author

Ok, I think it may have been specific to java_signatures on a ruby class derived from a java class, see https://github.com/byteit101/JRubyFX-FXMLLoader/blob/newloader/lib/jrubyfx-fxmlloader2.rb#180

I'm currently getting an interesting issue that I think is different (NPE in become_java), but that is the original source IIRC

@headius
Copy link
Member

headius commented Sep 21, 2016

Anyone want to attempt a PR for the existing java_signature logic, to make it possible to define multiple signatures mapping to arbitrary Ruby method names?

@enebo enebo modified the milestones: JRuby 9.2.0.0, JRuby 9.1.6.0 Nov 7, 2016
@headius
Copy link
Member

headius commented May 15, 2018

Detargeting general issue with become_java! that's not on roadmap right now. Help wanted.

@headius headius removed this from the JRuby 9.2.0.0 milestone May 15, 2018
@byteit101
Copy link
Member Author

This was fixed in 9.3.1 (though the syntax for renaming methods differs via configure_class)

@byteit101 byteit101 added this to the JRuby 9.3.0.0 milestone Apr 19, 2022
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

5 participants