-
Notifications
You must be signed in to change notification settings - Fork 44
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
Animal Sniffer is missing proper support for @PolymorphicSignature methods (such as MethodHandles.invoke) #67
Comments
Hi, pinging on this again... I can't use |
I discovered that if I change accessible.set((Boolean) isAccessible.invokeExact(field)); to accessible.set((Boolean) isAccessible.invokeExact(new Object[] {field})); I get
This explains part of the problem. This is the same issue as experienced with a change in return type of several plasma-umass/doppio#497 (comment) However, Animal Sniffer was not able to detect the covariant return type problem without the cast to There is a second problem though: in fact this is a false positive. However there is a third problem, due to Animal Sniffer not being able to recognize that a single parameter of type accessible.set((Boolean) isAccessible.invokeExact(new Object[] {field})); then I still get
The only way to fix this error completely is to change the code to: accessible.set((Boolean) (Object) isAccessible.invokeExact(new Object[] {field})); which is (I assume) less efficient than calling the varargs method the normal way. Why does Animal Sniffer require this |
Furthermore when trying to call a static method from a
I think the real issue here is that Animal Sniffer is missing explicit support for |
Hi, I ran into this again. Can somebody from Mojohaus please take a look at this? |
This is the first use of a Java 9 API in Guava, but we use the API only when it's available, so we maintain compatibility with Java 8. Use of Java 9 APIs is relevant to #6549 and #3990 (and also mojohaus/animal-sniffer#67). I didn't make the same change for `guava-android`, which [will add `java.util.zip.CRC32C` in API Level 34](https://developer.android.com/reference/java/util/zip/CRC32C). I don't know if Android is providing similar performance improvements, so it might not even matter. But even if I wanted to do it, I can't with my current approach, which relies on `MethodHandle`—unless I want to make even the usage of `MethodHandle` conditional on a reflective check :) RELNOTES=`hash`: Enhanced `crc32c()` to use Java's hardware-accelerated implementation where available. PiperOrigin-RevId: 539722059
This is the first use of a Java 9 API in Guava, but we use the API only when it's available, so we maintain compatibility with Java 8. Use of Java 9 APIs is relevant to #6549 and #3990 (and also mojohaus/animal-sniffer#67). I didn't make the same change for `guava-android`, which [will add `java.util.zip.CRC32C` in API Level 34](https://developer.android.com/reference/java/util/zip/CRC32C). I don't know if Android is providing similar performance improvements, so it might not even matter. But even if I wanted to do it, I can't with my current approach, which relies on `MethodHandle`—unless I want to make even the usage of `MethodHandle` conditional on a reflective check :) RELNOTES=`hash`: Enhanced `crc32c()` to use Java's hardware-accelerated implementation where available. PiperOrigin-RevId: 539722059
This is the first use of a Java 9 API in Guava, but we use the API only when it's available, so we maintain compatibility with Java 8. Use of Java 9 APIs is relevant to #6549 and #3990 (and also mojohaus/animal-sniffer#67). I didn't make the same change for `guava-android`, which [will add `java.util.zip.CRC32C` in API Level 34](https://developer.android.com/reference/java/util/zip/CRC32C). I don't know if Android is providing similar performance improvements, so it might not even matter. But even if I wanted to do it, I can't with my current approach, which relies on `MethodHandle`—unless I want to make even the usage of `MethodHandle` conditional on a reflective check :) RELNOTES=`hash`: Enhanced `crc32c()` to use Java's hardware-accelerated implementation where available. PiperOrigin-RevId: 539722059
This is the first use of a Java 9 API in Guava, but we use the API only when it's available, so we maintain compatibility with Java 8. Use of Java 9 APIs is relevant to #6549 and #3990 (and also mojohaus/animal-sniffer#67). I didn't make the same change for `guava-android`, which [will add `java.util.zip.CRC32C` in API Level 34](https://developer.android.com/reference/java/util/zip/CRC32C). I don't know if Android is providing similar performance improvements, so it might not even matter. But even if I wanted to do it, I can't with my current approach, which relies on `MethodHandle`—unless I want to make even the usage of `MethodHandle` conditional on a reflective check :) RELNOTES=`hash`: Enhanced `crc32c()` to use Java's hardware-accelerated implementation where available. PiperOrigin-RevId: 539983316
Animal Sniffer inspects bytecode call-sites to look for calls to methods not defined by Java 8. However, it doesn't know about signature polymorphic methods, like MethodHandle.invokeExact(), so we will always get warnings about those calls. Since we can now natively build on Java 8 (this was previously not possible with Java 6), we can instead rely on the test suite to make sure we don't make calls to methods not available in Java 8. The associated animal sniffer issue is mojohaus/animal-sniffer#67
Motivation: We've been using reflection as a way to abstract over Java API versions. Now that we're based on Java 8, some of this reflection usage is no longer necessary. In some cases, the APIs we were reflecting on are now directly available. In other cases, we'd like a little more performance and can call through method handles instead. Modification: Remove some reflection usage that was necessary when running on Java 6 or 7, and replace it with direct calls if the code is still needed. Replace the more performance sensitive reflection usage with MethodHandles. Also remove the animal sniffer maven plug-in, because animal sniffer does not support signature polymorphic method calls. See mojohaus/animal-sniffer#67 Result: Cleaner, and perhaps slightly faster, code. Fixes #14009
For the code
I get the following for
org.codehaus.mojo.signature:java17:1.0
:However this method is defined in JDK 1.7:
https://docs.oracle.com/javase/7/docs/api/java/lang/invoke/MethodHandle.html#invokeExact(java.lang.Object...)
MethodHandle::invoke
is also missing from thejava17
signatures (and maybe other things).The text was updated successfully, but these errors were encountered: