Require Java 11 or newer #635
Conversation
| @@ -90,7 +90,7 @@ | |||
| import java.util.Set; | |||
| import java.util.TreeMap; | |||
| import java.util.stream.Collectors; | |||
| import javax.annotation.Generated; | |||
| import javax.annotation.processing.Generated; | |||
| @@ -525,7 +527,7 @@ public void regexpOperator() throws Throwable { | |||
| @Test | |||
| public void arrayPassedToMethod() throws Throwable { | |||
| assertEvaluate(4, "def m(x) {x.size()}; def a = [1, 2]; a.size() + m(a)"); // control case | |||
| assertEvaluate(4, "def m(x) {x.size()}; def a = [1, 2].toArray(); a.length + m(Arrays.asList(a))"); // workaround #1 | |||
| assertEvaluate(4, "def m(x) {x.size()}; def a = [1, 2].toArray(); a.length + m(List.of(a))"); // workaround #1 | |||
There was a problem hiding this comment.
For some reason which is not immediately apparent to me, this causes a VerifyError when running this test using Java 17. The only thing that seems special about this case compared to all of the other replacements is that this replacement is inside of a Groovy script.
Stack trace
java.lang.VerifyError: (class: java_util_List$of$4, method: callStatic signature: (Ljava/lang/Class;[Ljava/lang/Object;)Ljava/lang/Object;) Illegal type in constant pool
at java.base/java.lang.Class.getDeclaredConstructors0(Native Method)
at java.base/java.lang.Class.privateGetDeclaredConstructors(Class.java:3373)
at java.base/java.lang.Class.getConstructor0(Class.java:3578)
at java.base/java.lang.Class.getConstructor(Class.java:2271)
at org.codehaus.groovy.reflection.ClassLoaderForClassArtifacts.defineClassAndGetConstructor(ClassLoaderForClassArtifacts.java:82)
at org.codehaus.groovy.runtime.callsite.CallSiteGenerator.compileStaticMethod(CallSiteGenerator.java:262)
at org.codehaus.groovy.reflection.CachedMethod.createStaticMetaMethodSite(CachedMethod.java:295)
at org.codehaus.groovy.runtime.callsite.StaticMetaMethodSite.createStaticMetaMethodSite(StaticMetaMethodSite.java:114)
at groovy.lang.MetaClassImpl.createStaticSite(MetaClassImpl.java:3421)
at org.codehaus.groovy.runtime.callsite.CallSiteArray.createCallStaticSite(CallSiteArray.java:76)
at org.codehaus.groovy.runtime.callsite.CallSiteArray.createCallSite(CallSiteArray.java:161)
at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)
at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:128)
at Script2.run(Script2.groovy:1)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:574)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:612)
at groovy.lang.GroovyShell.evaluate(GroovyShell.java:583)
at com.cloudbees.groovy.cps.AbstractGroovyCpsTest.eval(AbstractGroovyCpsTest.java:93)
... 46 more
There was a problem hiding this comment.
Possibly some issue with varargs. Just revert it.
There was a problem hiding this comment.
Whoops, sorry about that. Let me know if you want me to open a PR reverting this hunk.
There was a problem hiding this comment.
No worries, I rolled it into #640. I only mentioned it here since it seems somewhat unusual.
|
Looking at https://stackoverflow.com/a/27107189/4715872 and #627 (comment) it seems the change all over the place to |
| @@ -102,7 +102,7 @@ public static <E> Iterator<E> iterator(Set<E> set) { | |||
|
|
|||
| public static <E> Iterator<E> iterator(E[] array) { | |||
| // TODO as above | |||
| return new Itr<>(Arrays.asList(array)); | |||
| return new Itr<>(List.of(array)); | |||
There was a problem hiding this comment.
This evidently caused a regression.
|
@basil are you using some automated refactoring tool for these PRs? If so, please disable the |
|
Sorry for the inconvenience and thank you for fixing this. (Actually I would have jumped on this right away, but you tend to wake up a few hours earlier than me on the East Coast.) No I am not using an automated refactoring tool. The automated tools don't make this recommendation because it is not provably safe. However it is safe most of the time, and I tend to rely on a combination of quick code inspection and automated test coverage to ascertain with relatively high probability whether the change is safe or not. Lack of test coverage means that I've been wrong a handful of times, like in this case and in core, but altogether this heuristic works more often than not. |
Oh no problem, I just got alerted to it my morning time via Gitter. Anyway it is ultimately up to maintainers to determine whether a given PR is safe; in the future we will inspect |
No description provided.