On Feb 10, 2020, at 02:56, somewhat-functional-programmer somewhat-functional-programmer@protonmail.com wrote:
Thank you for reviewing the patch.
[…]
So read on, with our common understanding that I have no objection at all to the reversal of the patch :-)…
Cool.
I like and also thought about looking at the argument values dynamically to help determine a proper call site. It certainly makes sense, especially for literal values, to not require extra coercion or annotation, to find the proper call sites. However, here are a couple of inconsistencies that arise from the current implementation (git master branch 702405fe97a1778f5d3683a6a9cc4294071061e6):
;; the following signals the "no such method" error, ;; consistent with my understanding of the intent of the new patch (jstatic "reverseBytes" "java.lang.Short" 6300000)
;; however, this version does not throw and error, and silently narrows ;; 63000 the value to a short (let ((method (jmethod "java.lang.Short" "reverseBytes" "short"))) (jstatic method nil 6300000)
Confirmed. Mea Maximus Stupidius Culpa in my patch: I didn’t think about/test an explicitly supplied JMETHOD instance.
;; this version has a different error, IllegalArgumentException ;; as the runtime won't automatically convert 32.3 to a short (let ((method (jmethod "java.lang.Short" "reverseBytes" "short"))) (jstatic method nil 32.3))
Yes, this should probably be caught in the find call site machinery, transformed into a Lisp side TYPE_ERROR.
;; this gives the more familiar no such method error (jstatic "reverseBytes" "java.lang.Short" 32.3)
One annoying thing about Java is the "byte" type is signed -- so these value checks will still potentially cause trouble for methods taking a "byte”.
[…]
That brings me to my last question -- What is (java:jcoerce) supposed to do?
I had incorrectly assumed it was much like a Java cast -- in Java one can cast from any primitive type to any other primitive type (with the exception of boolean). One can also cast within the inheritance hierarchy for references etc.
The docstring for jcoerce says: Attempts to coerce OBJECT into a JavaObject of class INTENDED-CLASS. Raises a TYPE-ERROR if no conversion is possible.
Tentatively, I would suggest that JCOERCE tries as hard as possible to DWIM, signalling a TYPE-ERROR (or possibly a subclass) if any coercian would lose information. Then, JCOERCE is defined as a transformation to values whose inverse round-trips. As such, we would extend JCOERCE to cover as many types as possible under this definition, bringing in all Java primtitive types (float, double, char are what we need I think). For completeness, we should cover all descendents of java.lang.Number. But *not* try to cover java.lang.Character. My thinking here is that “char” has a meaning as an unsigned two bytes value, but not allow coercian of java.lang.Character values as it can potentially wrap both int and char. I could be talked out of the last point if someone can defend the interpolation of such values, but I think users would be too confused.
I agree that we should add a JCAST that potentially strips information according to the JLS widening and narrowing rules for all its floating port horrors.
Here are some examples of the inconsistencies:
[…]
Studying and incorporating it into tests. Thanks for bagging parts of the shaggy monster…
—
Currently, I am for fixing JCOERCE for abcl-1.6.1, and pushing JCAST off to either abcl-1.7 or abcl-2.0, whichever I branch first.
yers in CONS, Mark