Thank you for reviewing the patch.
I completely agree that my patch should not be in a minor point release, and I hope I didn't come across as thinking it should. I definitely think that any relatively major incompatible change would at least require a smooth upgrade plan (maybe something new in *features*, and updates pushed to swank/cffi with fixes that would work for the newest ABCL as well as historical ABCLs). So that said, I have absolutely no objection to any 1.6.1 plans (not that I presume to have a say in that anyhow :-)).
So read on, with our common understanding that I have no objection at all to the reversal of the patch :-)...
--
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))
;; 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))
;; 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". Consider the following:
(require :abcl-contrib) (require :jss)
(defvar *bb* (#"allocate" 'java.nio.ByteBuffer 1024))
;; Going to use ByteBuffer.put(byte)
;; this signals a NoSuchMethod JAVA-EXCEPTION ;; since the value is > 127, too much for our signed Java byte type (#"put" *bb* #xf2)
;; but this succeeds since it is in range (-127 - 127) (#"put" *bb* #x2f)
Now this is also annoying in Java, often one has to write code like byte[] myBytes = {(byte) 0xfa, (byte) 0xaa}; However, at least there is a cast operator, ugly as it makes Java look :-). (Or one could write byte[] myBytes = {-6, -86}; )
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.
Here are some examples of the inconsistencies:
;; these all succeed, and produce a java.lang.Byte, narrowing and losing information ;; with no error (like a java cast) (jcoerce #xff "byte") (jcoerce 3.22 "byte") (jcoerce 293847918751987980174873249801273 "byte") (jcoerce 23.49879427598475074398572 "byte")
;; succeeds and returns a java.lang.Float (jcoerce 1.23 "float")
;; these fail
;; value #\c is not of type byte (jcoerce #\c "byte")
;; value 20 is not of type int (jcoerce 20 "int")
;; value 20 is not of type long (jcoerce 20 "long")
;; value 20 is not of type float (jcoerce 20 "float")
;; value 1.23 is not of type double (jcoerce 1.23 "double")
;; these return '(1 2 3) with no error (jcoerce '(1 2 3) "int") (jcoerce '(1 2 3) "javax.swing.JTextArea")
I think the interface with Java must necessarily be a little bit messy, and I think narrowing primitives is very common within Java, and if jcoerce is not the way to do that, it would be nice to define a jcast or some other similar function. One of course could always do something like the following to arrive at a valid Java signed byte (credit stassats [1] for mask-signed):
(defun mask-signed (x size) (logior x (- (mask-field (byte 1 (1- size)) x))))
;; "cast" / narrow our #xfa to the bitwise-equivalent 8 bit signed value ;; this allows our put to succeed, as our value now falls in the ;; valid range for java's signed byte type (#"put" *bb* (mask-signed #xfa 8))
Anyhow, I appreciate you looking at my patch,
-Mark
[1] https://old.reddit.com/r/Common_Lisp/comments/ecl5on/convert_unsignedbyte_to... (credit stassats)
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Sunday, February 9, 2020 1:45 PM, Mark Evenson evenson@panix.com wrote:
On Dec 13, 2019, at 20:14, somewhat-functional-programmer somewhat-functional-programmer@protonmail.com wrote: I've looked into this issue at length now and I think I have something worth discussing and potentially trying[0] . The root issue here lies in how ABCL converts its CL representation of integers to Java, as well as its conversion of Java primitives to other Java primitives.
[…]
What are your thoughts? I dislike backwards incompatible changes like this, but I think the impact is probably low due to the fact that method resolution used rules to only find widening conversions, and the fact that short and byte are not as commonly used in method calls.
After much study, reflection and experimentation, I have decided that I didn't like the incompatible change introduced by this patch in requiring Lisp code to explictly JCOERCE arguments to find Java call sites.
Instead, I [managed to solve the original poster's problems of not being able to call Java methods whose parameters where short or byte][1] by modifying the Java.findMethod() machinery to use the actual arguments that will be used in the call for a decision whether to narrow types for calling a given function makes sense for short or int types. I was unable to find other problems with converting primitive types.
With a little additional elbow grease , I was able to also code a fix for finding call-sites that take a varargs parameter such as
java.util.Arrays.asList(T... arg)
Given the major holes pointed out by the someone-functional-programmers examination of of the narrowing/widening convention, I am surprised that there are not more Java calls that can't be made with our current FFI machinery, but after scouring our bug reports and trying various methods I can't find any examples. If anyone knows of a case of the Java FFI not finding call sites with these patches, please pipe up.
While reviewing this patch, I noticed that since one can't use CL:COERCE to perform a conversion that loses information, as
(coerce 256 '(unsigned-byte 8))
signals some sort of error for all existing Lisp implementations (SBCL, CCL, ECL, CLISP, Allegro, LispWorks), therefore I would argue that something like
(jcoerce 256 "byte")
should also signal an error. Therefore, JCOERCE should be reworked to signal some sort of error if the conversion would lose information. I continue to work through what we would be needed to both incoporate somewhat-functional-programmer's patch, which allows things like
(jcoerce 2.4 "float")
to work, but that would signal TYPE-ERROR conditions when JCOERCE call that lose information are attempted.
My current inclination would be to release the current state as abcl-1.6.1 without incorporating this patch, as I think the need to [patch ABCL-ASDF's inablity to download Maven artifacts][4] is much more pressing than untangling the incomplete inconsistencies in JCOERCE.
"A screaming comes across the sky. It has happened before but there is nothing to compare to it now."