-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Changing type of global variables gives TypeError #4178
Comments
Must be caching the last Java type used to coerce the value, or something like that. |
Yes, that appears to be it. For whatever reason, when assigning these variables from either Java or Ruby, we cache the first-seen Java type and try to forcibly coerce to that type from then on. This seems wrong to me. The following patch removes this behavior from both forms of assignment: diff --git a/core/src/main/java/org/jruby/embed/variable/AbstractVariable.java b/core/src/main/java/org/jruby/embed/variable/AbstractVariable.java
index 809f6f4..ace98e6 100644
--- a/core/src/main/java/org/jruby/embed/variable/AbstractVariable.java
+++ b/core/src/main/java/org/jruby/embed/variable/AbstractVariable.java
@@ -94,22 +94,22 @@ abstract class AbstractVariable implements BiVariable {
return (RubyObject) receiver.getRuntime().getTopSelf();
}
protected void updateByJavaObject(final Ruby runtime, Object... values) {
assert values != null;
javaObject = values[0];
if (javaObject == null) {
javaType = null;
} else if (values.length > 1) {
javaType = (Class) values[1];
- } else {
- javaType = javaObject.getClass();
+// } else {
+// javaType = javaObject.getClass();
}
irubyObject = JavaEmbedUtils.javaToRuby(runtime, javaObject);
fromRuby = false;
}
protected void updateRubyObject(final IRubyObject rubyObject) {
if ( rubyObject == null ) return;
this.irubyObject = rubyObject;
// NOTE: quite weird - but won't pass tests otherwise !?!
//this.javaObject = null;
@@ -134,23 +134,23 @@ abstract class AbstractVariable implements BiVariable {
}
public Object getJavaObject() {
if (irubyObject == null) return javaObject;
if (javaType != null) { // Java originated variables
javaObject = javaType.cast( irubyObject.toJava(javaType) );
}
else { // Ruby originated variables
javaObject = irubyObject.toJava(Object.class);
- if (javaObject != null) {
- javaType = javaObject.getClass();
- }
+// if (javaObject != null) {
+// javaType = javaObject.getClass();
+// }
}
return javaObject;
}
public void setJavaObject(final Ruby runtime, Object javaObject) {
updateByJavaObject(runtime, javaObject);
}
public IRubyObject getRubyObject() {
return irubyObject; And with this patch in place, your original case runs (in Ruby form below): ENV_JAVA["jruby.backtrace.style"] = "full"
engine = javax.script.ScriptEngineManager.new.getEngineByName("jruby")
puts(engine.eval("$x = 10"))
puts(engine.eval("$x = 'a'")) @enebo Can you think of any reason why this type caching should be there? I assume it was done for perf reasons, but I don't think there really are any. |
I'm going to go ahead with the removal. I can't find anything in git logs as to why this caching of javaType was done. Maybe @yokolet remembers? |
@kares You've also touched this code from time to time...I'd be interested in your thoughts. |
looking good, although I wasn't changing much semantics here. |
Environment
jruby-complete-9.1.5.0.jar
Java 1.8.0_25
Linux x86_64
Expected Behavior
With:
The output should be '10' followed by 'a'.
Actual Behavior
I'm getting '10', followed by:
The text was updated successfully, but these errors were encountered: