[armedbear-devel] [PATCH] Problems with new JavaObject.javaInstance(Class c) and JSS
Unfortunately Alessio's (and Tobias?) work on [adding richness to inspection of Java objects][1] contains a bug that manifests itself in stopping [JSS][2] from working. The attached patch gets JSS to run again, but I am not sure the right way to fix the problem in the trunk, mainly as I don't understand all of our conversion rules, and how they interact with Java's autoboxing. The problem with JSS occurs when JavaObject.javaInstance(Class c) is invoked with a Class from a primitive boolean on an JavaObject that wraps a java.lang.Boolean. The call to java.lang.Class.isAssignableFrom() has an oddly (to me) specific contract that "[i]if [the] Class object represents a primitive type, this method returns true if the specified Class parameter is exactly this Class object; otherwise it returns false." This makes JavaObject<java.lang.Boolean>.javaInstance(boolean) fail, when everything seems to work fine if we simply return the wrapped object. What I don't understand mainly is if boolean/java.lang.Boolean objects are somehow special in our treatment of them, or should we do this for all of the other primitive types? [1]: http://trac.common-lisp.net/armedbear/changeset/12081 [2]: http://mumble.net:8080/svn/lsw/trunk/jss/ -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
On Wed, Aug 5, 2009 at 5:17 PM, Mark Evenson<evenson@panix.com> wrote:
Unfortunately Alessio's (and Tobias?) work on [adding richness to inspection of Java objects][1] contains a bug that manifests itself in stopping [JSS][2] from working.
The attached patch gets JSS to run again, but I am not sure the right way to fix the problem in the trunk, mainly as I don't understand all of our conversion rules, and how they interact with Java's autoboxing.
The problem with JSS occurs when JavaObject.javaInstance(Class c) is invoked with a Class from a primitive boolean on an JavaObject that wraps a java.lang.Boolean. The call to java.lang.Class.isAssignableFrom() has an oddly (to me) specific contract that "[i]if [the] Class object represents a primitive type, this method returns true if the specified Class parameter is exactly this Class object; otherwise it returns false." This makes JavaObject<java.lang.Boolean>.javaInstance(boolean) fail, when everything seems to work fine if we simply return the wrapped object.
Right, that's an oversight on my part. Unfortunately Java's reflection does not know about automatic boxing/unboxing, you have to do it by yourself.
What I don't understand mainly is if boolean/java.lang.Boolean objects are somehow special in our treatment of them, or should we do this for all of the other primitive types?
The latter. I believe there's already code written for this somewhere (in jmethod perhaps?), so maybe we could generalize & reuse that code. Also, I have a question about your patch: do you have a specific reason to handle null by always signaling an error? In principle, null is assignable to any non-primitive type, so the type error ought to be signaled only for primitive types, imho. Bye, Alessio
[1]: http://trac.common-lisp.net/armedbear/changeset/12081
[2]: http://mumble.net:8080/svn/lsw/trunk/jss/
-- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
_______________________________________________ armedbear-devel mailing list armedbear-devel@common-lisp.net http://common-lisp.net/cgi-bin/mailman/listinfo/armedbear-devel
On 8/5/09 5:33 PM, Alessio Stalla wrote: […]
Also, I have a question about your patch: do you have a specific reason to handle null by always signaling an error? In principle, null is assignable to any non-primitive type, so the type error ought to be signaled only for primitive types, imho.
No good reason: I was just trying to refactor the logic, but misunderstood your intent (that "null is assignable to any non-primitive type") so it is incorrect as it stands. -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
On Wed, Aug 5, 2009 at 6:25 PM, Mark Evenson<evenson@panix.com> wrote:
On 8/5/09 5:33 PM, Alessio Stalla wrote: […]
Also, I have a question about your patch: do you have a specific reason to handle null by always signaling an error? In principle, null is assignable to any non-primitive type, so the type error ought to be signaled only for primitive types, imho.
No good reason: I was just trying to refactor the logic, but misunderstood your intent (that "null is assignable to any non-primitive type") so it is incorrect as it stands.
Ok. Looking at my commit again, I see that actually the change to javaInstance() is not particularly useful for the inspection of Java objects; it just makes code a little bit shorter in getInspectedFields(), where a type check is not done explicitly, relying on javaInstance() to do it. So the safest thing to do is to revert my change and do the type check, until we sort out precisely how to correctly implement javaInstance(Class). I also believe this method should be generified, in order to avoid an unnecessary cast of its return value from Object to the desired type. Cheers, Alessio
On Wed, Aug 5, 2009 at 7:39 PM, Alessio Stalla<alessiostalla@gmail.com> wrote:
On Wed, Aug 5, 2009 at 6:25 PM, Mark Evenson<evenson@panix.com> wrote:
On 8/5/09 5:33 PM, Alessio Stalla wrote: […]
Also, I have a question about your patch: do you have a specific reason to handle null by always signaling an error? In principle, null is assignable to any non-primitive type, so the type error ought to be signaled only for primitive types, imho.
No good reason: I was just trying to refactor the logic, but misunderstood your intent (that "null is assignable to any non-primitive type") so it is incorrect as it stands.
Ok. Looking at my commit again, I see that actually the change to javaInstance() is not particularly useful for the inspection of Java objects; it just makes code a little bit shorter in getInspectedFields(), where a type check is not done explicitly, relying on javaInstance() to do it. So the safest thing to do is to revert my change and do the type check, until we sort out precisely how to correctly implement javaInstance(Class). I also believe this method should be generified, in order to avoid an unnecessary cast of its return value from Object to the desired type.
Hopefully it's fixed on trunk now, sorry for the inconvenience. In passing I also discovered & fixed a minor bug in inspection of Java arrays - Lisp objects inside the array were incorrectly wrapped in a JavaObject. Bye, Ale
participants (3)
-
Alessio Stalla -
Mark Evenson -
Tobias C. Rittweiler