JRuby

Java method selection is subject to reflection ordering

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: JRuby 1.4
  • Fix Version/s: JRuby 1.7.0.pre2
  • Component/s: Java Integration
  • Labels:
    None
  • Number of attachments :
    0

Description

Currently method selection uses only a few coarse-grained facts to determine a best match for incoming Ruby values:

  • Exact type matches
  • Assignability of coercion product
  • Implementability of target types

When passing a Fixnum to a method with two overrides, one that takes Object and one that takes Integer, because Fixnum's "natural" Java type is Long, both of these options currently appear to be equally valid. Neither is more exactly matching, and neither is a better assignment target. Because of this, when the Object version comes first in method selection, it may be chosen rather than the Integer version.

This is currently causing two failures on IBM's Java 5 JDK:

1)
'A Java object's java_method method produces Method for static methods against an instance' FAILED
expected: #<Java::JavaLang::Integer:0x473e473e>,
     got: 1 (using ==)
...
./spec/java_integration/methods/java_method_spec.rb:22:
...

2)
'A Java object's java_method method produces Method for static methods against a class' FAILED
expected: #<Java::JavaLang::Integer:0x40a640a6>,
     got: 1 (using ==)
...
./spec/java_integration/methods/java_method_spec.rb:33:
...

Fixing this will require implementing a more complete Java method selection protocol, which should come in JRuby 1.5.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Charles Oliver Nutter added a comment -

I patched the failing specs in 6bbcf1e. Ideally this change should be reverted once this is fixed.

Show
Charles Oliver Nutter added a comment - I patched the failing specs in 6bbcf1e. Ideally this change should be reverted once this is fixed.
Hide
Permalink
Charles Oliver Nutter added a comment -

A bit more explanation of the spec patch...

In the specs, we ultimately tried to call Fixnum#== with a java.lang.Integer argument. In this case, the == logic sees that "other" is not a Fixnum and flips the comparison, calling == on java.lang.Integer. The == for java.lang.Integer comes from a java.lang.Comparable monkey-patch that makes it Comparable (in the Ruby sense) using compareTo. So we ultimately try to dynamically call java.lang.Integer.compareTo with a Fixnum argument. compareTo has two overloads, one with Object and one with Integer (due to it specializing the generic Comparable<T> interface. When the Integer version comes first in searching, we choose it. When the Object version comes first, as on IBM Java 5, we choose it. However compareTo(Object) in java.lang.Integer is the one generated by generic logic, and just casts its input to Integer. Since calling an Object method with a Fixnum coerces that Fixnum to a java.lang.Long, the cast fails and an error is raised.

See RubyFixnum.op_equal, RubyNumeric.op_num_equal, and the compareto/Comparable patching in java.lang.rb.

Show
Charles Oliver Nutter added a comment - A bit more explanation of the spec patch... In the specs, we ultimately tried to call Fixnum#== with a java.lang.Integer argument. In this case, the == logic sees that "other" is not a Fixnum and flips the comparison, calling == on java.lang.Integer. The == for java.lang.Integer comes from a java.lang.Comparable monkey-patch that makes it Comparable (in the Ruby sense) using compareTo. So we ultimately try to dynamically call java.lang.Integer.compareTo with a Fixnum argument. compareTo has two overloads, one with Object and one with Integer (due to it specializing the generic Comparable<T> interface. When the Integer version comes first in searching, we choose it. When the Object version comes first, as on IBM Java 5, we choose it. However compareTo(Object) in java.lang.Integer is the one generated by generic logic, and just casts its input to Integer. Since calling an Object method with a Fixnum coerces that Fixnum to a java.lang.Long, the cast fails and an error is raised. See RubyFixnum.op_equal, RubyNumeric.op_num_equal, and the compareto/Comparable patching in java.lang.rb.
Hide
Permalink
Charles Oliver Nutter added a comment -

I'm punting this off 1.5. It needs a proper implementation of JLS method dispatch to be completely fixed, which isn't going to happen for 1.5. Since this was only a case in our own specs and has never been reported as a problem in the wild. It's not a high priority to fix right now.

Show
Charles Oliver Nutter added a comment - I'm punting this off 1.5. It needs a proper implementation of JLS method dispatch to be completely fixed, which isn't going to happen for 1.5. Since this was only a case in our own specs and has never been reported as a problem in the wild. It's not a high priority to fix right now.
Hide
Permalink
Charles Oliver Nutter added a comment -

In addition, I'm downgrading to minor, since it has an easy workaround (using java_send and friends or pre-coercing to exact target method types).

Show
Charles Oliver Nutter added a comment - In addition, I'm downgrading to minor, since it has an easy workaround (using java_send and friends or pre-coercing to exact target method types).
Hide
Permalink
Charles Oliver Nutter added a comment -

As of 1.7pre2, method selection has been rewritten to eliminate reflection-ordering as an issue. If there's ambiguity, we report that and take the most specific method.

Show
Charles Oliver Nutter added a comment - As of 1.7pre2, method selection has been rewritten to eliminate reflection-ordering as an issue. If there's ambiguity, we report that and take the most specific method.

People

  • Assignee:
    Charles Oliver Nutter
    Reporter:
    Charles Oliver Nutter
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: