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 integration should call toJava or to_int for array index and value #2658

Closed
InfraRuby opened this Issue Mar 6, 2015 · 1 comment

Comments

Projects
None yet
2 participants
@InfraRuby
Copy link

InfraRuby commented Mar 6, 2015

Java integration does not support int-like objects:

class I
    def initialize(value)
        @value = value
        return
    end
    def to_int
        return @value
    end
end

n = I.new(2)
a = Java::byte[n].new   # works: JRuby calls n.to_int

value = I.new(1)
a[1] = value            # fails with TypeError: JRuby calls value.toJava(Byte.class)

index = I.new(1)
a[index]                # fails with TypeError: JRuby expects RubyInteger or RubyRange

index = I.new(1)
a[index] = 1            # fails with ClassCastException: JRuby casts index to RubyInteger

It's also a problem for InfraRuby compatibility:

require "infraruby-java"

n = Int32[2]
a = Java::byte[n].new   # works: JRuby calls n.to_int

value = Byte[1]
a[1] = value            # works: JRuby calls value.toJava(Byte.class)

index = Int32[1]
a[index]                # fails with TypeError: JRuby expects RubyInteger or RubyRange

index = Int32[1]
a[index] = 1            # fails with ClassCastException: JRuby casts index to RubyInteger

A patch for core/src/main/java/org/jruby/RubyBasicObject.java:

@@ -833,6 +833,10 @@
             return this;
         }

+        RubyConverter converter = JavaUtil.RUBY_CONVERTERS.get(target);
+        if (converter != null) {
+            return converter.convert(getRuntime().getCurrentContext(), this);
+        }
         throw getRuntime().newTypeError("cannot convert instance of " + getClass() + " to " + target);
     }

A patch for core/src/main/java/org/jruby/java/proxies/ArrayJavaProxy.java:

@@ -73,19 +73,19 @@

     @JRubyMethod(name = "[]")
     public IRubyObject op_aref(ThreadContext context, IRubyObject arg) {
-        if (arg instanceof RubyInteger) {
-            int index = (int)((RubyInteger)arg).getLongValue();
-            return ArrayUtils.arefDirect(context.runtime, getObject(), converter, index);
-        } else {
+        if (arg instanceof RubyRange) {
             return getRange(context, arg);
+        } else {
+            int i = arg.toJava(Integer.class);
+            return ArrayUtils.arefDirect(context.runtime, getObject(), converter, i);
         }
     }

     @JRubyMethod(name = "[]", required = 1, rest = true)
     public IRubyObject op_aref(ThreadContext context, IRubyObject[] args) {
-        if (args.length == 1 && args[0] instanceof RubyInteger) {
-            int index = (int)((RubyInteger)args[0]).getLongValue();
-            return ArrayUtils.arefDirect(context.runtime, getObject(), converter, index);
+        if (args.length == 1) {
+            IRubyObject arg = args[0];
+            return op_aref(context, arg);
         } else {
             return getRange(context, args);
         }
@@ -93,7 +93,8 @@

     @JRubyMethod(name = "[]=")
     public IRubyObject op_aset(ThreadContext context, IRubyObject index, IRubyObject value) {
-        ArrayUtils.asetDirect(context.runtime, getObject(), converter, (int)((RubyInteger)index).getLongValue(), value);
+        int i = index.toJava(Integer.class);
+        ArrayUtils.asetDirect(context.runtime, getObject(), converter, i, value);
         return value;
     }

@InfraRuby InfraRuby changed the title Java integration should call toJava or to_int for array index Java integration should call toJava or to_int for array index and value Mar 6, 2015

kares added a commit to kares/jruby that referenced this issue Mar 11, 2015

tune ArrayProxy - allow indexing by Java Integer wrappers (inspired by
…jruby#2658)

avoids "ugly-sh" failures such as :

TypeError: wrong argument type Java::JavaLang::Integer (expected Range)
    org/jruby/java/proxies/ArrayJavaProxy.java:81:in `[]'

kares added a commit to kares/jruby that referenced this issue Mar 11, 2015

share ArrayProxy's array index conversion code (between [] and []=) +…
… review internals

... also added a test for new `[]` and `[]=` index conversion (inspired by jruby#2658)

@kares kares added this to the JRuby 1.7.20 milestone Mar 11, 2015

@kares

This comment has been minimized.

Copy link
Member

kares commented Mar 11, 2015

@InfraRuby similar issue (java.lang.Integer indexes not-working) bothered me as well ...
taken some of your patches and put it on jruby-1_7 (including some asserts) d45a1c8 thank you!

@kares kares closed this Mar 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.