You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/02/25 02:04:12 UTC
[groovy] branch GROOVY_2_5_X updated (a3771b9 -> 78430ba)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a change to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from a3771b9 GROOVY-9409: org.codehaus.groovy.runtime.callsite.GroovySunClassLoader is unusable (port to 2_5_X)
new 98ae51c GROOVY-9211: BUG! UNCAUGHT EXCEPTION on OpenJDK14-ea+8
new 78430ba GROOVY-9211: BUG! UNCAUGHT EXCEPTION on OpenJDK14-ea+8 (adjust for better JDK8 lookup)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
build.gradle | 2 +-
.../groovy/control/CompilerConfiguration.java | 5 +-
.../org/codehaus/groovy/vmplugin/v7/Java7.java | 71 +++++++++++----------
.../org/codehaus/groovy/vmplugin/v8/Java8.java | 72 ++++++++++++++++++++++
src/spec/test/SyntaxTest.groovy | 3 +-
src/test/gls/annotations/AnnotationTest.groovy | 9 ++-
.../lang/ScriptSourcePositionInAstTest.groovy | 15 +++--
src/test/groovy/lang/StripMarginTest.groovy | 16 +++--
.../gls/annotations/vm8/AnnotationTest.groovy | 5 +-
9 files changed, 149 insertions(+), 49 deletions(-)
[groovy] 01/02: GROOVY-9211: BUG! UNCAUGHT EXCEPTION on
OpenJDK14-ea+8
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 98ae51c6039c09b82a6ec30408d0d8cf2e441aef
Author: Paul King <pa...@asert.com.au>
AuthorDate: Sun Feb 23 23:09:40 2020 +1000
GROOVY-9211: BUG! UNCAUGHT EXCEPTION on OpenJDK14-ea+8
---
build.gradle | 2 +-
.../groovy/control/CompilerConfiguration.java | 5 +-
.../org/codehaus/groovy/vmplugin/v7/Java7.java | 71 ++++++++++++----------
src/spec/test/SyntaxTest.groovy | 3 +-
src/test/gls/annotations/AnnotationTest.groovy | 9 ++-
.../lang/ScriptSourcePositionInAstTest.groovy | 15 +++--
src/test/groovy/lang/StripMarginTest.groovy | 16 +++--
.../groovy/runtime/InterfaceConversionTest.groovy | 2 +
.../gls/annotations/vm8/AnnotationTest.groovy | 5 +-
9 files changed, 79 insertions(+), 49 deletions(-)
diff --git a/build.gradle b/build.gradle
index df6f98c..d298b2d 100644
--- a/build.gradle
+++ b/build.gradle
@@ -142,7 +142,7 @@ configurations {
ext {
antVersion = '1.9.13'
- asmVersion = '7.2'
+ asmVersion = '7.3.1'
antlrVersion = '2.7.7'
bridgerVersion = '1.5.Final'
coberturaVersion = '1.9.4.1'
diff --git a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java
index 2ce176c..89f6ae0 100644
--- a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java
+++ b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java
@@ -69,6 +69,8 @@ public class CompilerConfiguration {
public static final String JDK12 = "12";
/** This (<code>"13"</code>) is the value for targetBytecode to compile for a JDK 13. **/
public static final String JDK13 = "13";
+ /** This (<code>"14"</code>) is the value for targetBytecode to compile for a JDK 14. **/
+ public static final String JDK14 = "14";
/**
* This constant is for comparing targetBytecode to ensure it is set to JDK 1.5 or later.
@@ -97,7 +99,8 @@ public class CompilerConfiguration {
JDK10, Opcodes.V10,
JDK11, Opcodes.V11,
JDK12, Opcodes.V12,
- JDK13, Opcodes.V13
+ JDK13, Opcodes.V13,
+ JDK14, Opcodes.V14
);
private static final String[] EMPTY_STRING_ARRAY = new String[0];
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v7/Java7.java b/src/main/java/org/codehaus/groovy/vmplugin/v7/Java7.java
index 3b33607..792ff14 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v7/Java7.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v7/Java7.java
@@ -32,39 +32,8 @@ import java.security.PrivilegedAction;
* Java 7 based functions. Currently just a stub but you can
* add your own methods to your own version and place it on the classpath
* ahead of this one.
- *
- * @author Jochen Theodorou
*/
public class Java7 extends Java6 {
- private static final Constructor<MethodHandles.Lookup> LOOKUP_Constructor;
- static {
- Constructor<MethodHandles.Lookup> con = null;
- try {
- con = MethodHandles.Lookup.class.getDeclaredConstructor(Class.class, int.class);
- } catch (NoSuchMethodException e) {
- throw new GroovyBugError(e);
- }
- try {
- if (!con.isAccessible()) {
- final Constructor tmp = con;
- AccessController.doPrivileged(new PrivilegedAction() {
- @Override
- public Object run() {
- tmp.setAccessible(true);
- return null;
- }
- });
- }
- } catch (SecurityException se) {
- con = null;
- } catch (RuntimeException re) {
- // test for JDK9 JIGSAW
- if (!"java.lang.reflect.InaccessibleObjectException".equals(re.getClass().getName())) throw re;
- con = null;
- }
- LOOKUP_Constructor = con;
- }
-
@Override
public void invalidateCallSites() {
IndyInterface.invalidateSwitchPoints();
@@ -77,7 +46,7 @@ public class Java7 extends Java6 {
@Override
public Object getInvokeSpecialHandle(final Method method, final Object receiver) {
- if (LOOKUP_Constructor==null) {
+ if (getLookupConstructor() == null) {
return super.getInvokeSpecialHandle(method, receiver);
}
if (!method.isAccessible()) {
@@ -91,7 +60,7 @@ public class Java7 extends Java6 {
}
Class declaringClass = method.getDeclaringClass();
try {
- return LOOKUP_Constructor.
+ return getLookupConstructor().
newInstance(declaringClass, -1).
unreflectSpecial(method, declaringClass).
bindTo(receiver);
@@ -105,4 +74,40 @@ public class Java7 extends Java6 {
MethodHandle mh = (MethodHandle) handle;
return mh.invokeWithArguments(args);
}
+
+ private static Constructor<MethodHandles.Lookup> getLookupConstructor() {
+ return LookupHolder.LOOKUP_Constructor;
+ }
+
+ private static class LookupHolder {
+ private static final Constructor<MethodHandles.Lookup> LOOKUP_Constructor;
+
+ static {
+ Constructor<MethodHandles.Lookup> con = null;
+ try {
+ con = MethodHandles.Lookup.class.getDeclaredConstructor(Class.class, int.class);
+ } catch (NoSuchMethodException e) {
+ throw new GroovyBugError(e);
+ }
+ try {
+ if (!con.isAccessible()) {
+ final Constructor tmp = con;
+ AccessController.doPrivileged(new PrivilegedAction() {
+ @Override
+ public Object run() {
+ tmp.setAccessible(true);
+ return null;
+ }
+ });
+ }
+ } catch (SecurityException se) {
+ con = null;
+ } catch (RuntimeException re) {
+ // test for JDK9 JIGSAW
+ if (!"java.lang.reflect.InaccessibleObjectException".equals(re.getClass().getName())) throw re;
+ con = null;
+ }
+ LOOKUP_Constructor = con;
+ }
+ }
}
diff --git a/src/spec/test/SyntaxTest.groovy b/src/spec/test/SyntaxTest.groovy
index 8f3d0b6..bd1de6b 100644
--- a/src/spec/test/SyntaxTest.groovy
+++ b/src/spec/test/SyntaxTest.groovy
@@ -17,6 +17,7 @@
* under the License.
*/
import gls.CompilableTestSupport
+import org.codehaus.groovy.runtime.StringGroovyMethods
class SyntaxTest extends CompilableTestSupport {
@@ -206,7 +207,7 @@ class SyntaxTest extends CompilableTestSupport {
// end::shebang_comment_line[]
'''
- shouldCompile script.stripIndent().split('\n')[1..2].join('\n')
+ shouldCompile StringGroovyMethods.stripIndent((CharSequence)script).split('\n')[1..2].join('\n')
}
void testSingleLineComment() {
diff --git a/src/test/gls/annotations/AnnotationTest.groovy b/src/test/gls/annotations/AnnotationTest.groovy
index 6ec88be..796e8f1 100644
--- a/src/test/gls/annotations/AnnotationTest.groovy
+++ b/src/test/gls/annotations/AnnotationTest.groovy
@@ -641,7 +641,9 @@ class AnnotationTest extends CompilableTestSupport {
def string = getClass().getMethod('method').getAnnotation(Foo).toString()[5..-2].tokenize(', ').sort().join('|')
assert string == 'b=6|c1=A|c2=B|c3=C|c4=D|d1=2.0|d2=2.1|d3=2.2|d4=2.3|f1=1.0|f2=1.1|f3=1.2|f4=1.3|i1=0|i2=1|s1=2|s2=3|s3=4|s4=5' ||
// changed in some jdk9 versions
- string == "b=6|c1='A'|c2='B'|c3='C'|c4='D'|d1=2.0|d2=2.1|d3=2.2|d4=2.3|f1=1.0f|f2=1.1f|f3=1.2f|f4=1.3f|i1=0|i2=1|s1=2|s2=3|s3=4|s4=5"
+ string == "b=6|c1='A'|c2='B'|c3='C'|c4='D'|d1=2.0|d2=2.1|d3=2.2|d4=2.3|f1=1.0f|f2=1.1f|f3=1.2f|f4=1.3f|i1=0|i2=1|s1=2|s2=3|s3=4|s4=5" ||
+ // changed in some jdk14 versions
+ string == "b=(byte)0x06|c1='A'|c2='B'|c3='C'|c4='D'|d1=2.0|d2=2.1|d3=2.2|d4=2.3|f1=1.0f|f2=1.1f|f3=1.2f|f4=1.3f|i1=0|i2=1|s1=2|s2=3|s3=4|s4=5"
'''
}
@@ -714,8 +716,9 @@ class AnnotationTest extends CompilableTestSupport {
class MyClass {
// TODO confirm the JDK9 behavior is what we expect
private static final List<String> expected = [
- '@MyAnnotationArray(value=[@MyAnnotation(value=val1), @MyAnnotation(value=val2)])', // JDK5-8
- '@MyAnnotationArray(value={@MyAnnotation(value="val1"), @MyAnnotation(value="val2")})' // JDK9
+ '@MyAnnotationArray(value=[@MyAnnotation(value=val1), @MyAnnotation(value=val2)])', // JDK5-8
+ '@MyAnnotationArray(value={@MyAnnotation(value="val1"), @MyAnnotation(value="val2")})', // JDK9
+ '@MyAnnotationArray({@MyAnnotation("val1"), @MyAnnotation("val2")})' // JDK14
]
// control
diff --git a/src/test/groovy/lang/ScriptSourcePositionInAstTest.groovy b/src/test/groovy/lang/ScriptSourcePositionInAstTest.groovy
index d550ddb..48f8711 100644
--- a/src/test/groovy/lang/ScriptSourcePositionInAstTest.groovy
+++ b/src/test/groovy/lang/ScriptSourcePositionInAstTest.groovy
@@ -20,6 +20,7 @@ package groovy.lang
import org.codehaus.groovy.control.CompilationUnit
import org.codehaus.groovy.control.Phases
+import org.codehaus.groovy.runtime.StringGroovyMethods
/**
* Check that scripts have proper source position in the AST
@@ -45,18 +46,24 @@ class ScriptSourcePositionInAstTest extends GroovyTestCase {
}
void testDoubleStatementScript() {
- assert positionsForScript("""\
+ def raw = """\
println 'hello'
println 'bye'
- """.stripIndent()) == [[1, 1], [2, 14]]
+ """
+ // avoid stripIndent issues on JDK13+ by calling Groovy's DGM stripIndent explicitly
+ def script = StringGroovyMethods.stripIndent((CharSequence)raw)
+ assert positionsForScript(script) == [[1, 1], [2, 14]]
}
void testScriptWithClasses() {
- assert positionsForScript("""\
+ def raw = """\
class Bar {}
println 'hello'
println 'bye'
class Baz{}
- """.stripIndent()) == [[2, 1], [3, 14]]
+ """
+ // avoid stripIndent issues on JDK13+ by calling Groovy's DGM stripIndent explicitly
+ def script = StringGroovyMethods.stripIndent((CharSequence)raw)
+ assert positionsForScript(script) == [[2, 1], [3, 14]]
}
}
diff --git a/src/test/groovy/lang/StripMarginTest.groovy b/src/test/groovy/lang/StripMarginTest.groovy
index 2215fbc..dfdad15 100644
--- a/src/test/groovy/lang/StripMarginTest.groovy
+++ b/src/test/groovy/lang/StripMarginTest.groovy
@@ -18,6 +18,10 @@
*/
package groovy.lang
+import org.codehaus.groovy.runtime.StringGroovyMethods
+
+import static groovy.test.GroovyAssert.isAtLeastJdk
+
class StripMarginTest extends GroovyTestCase {
void testStripMarginOnSingleLineString() {
def expected = "the quick brown fox jumps over the lazy dog"
@@ -48,14 +52,16 @@ class StripMarginTest extends GroovyTestCase {
}
void testStripIndent() {
- def actual = """
+ def raw = """
return 'foo'
}
def method() {
return 'bar'
}
- """.stripIndent()
+ """
+ // hack to avoid conflicts with stripIndent from JDK13+ (resolution in Groovy 3)
+ def actual = isAtLeastJdk('13.0') ? StringGroovyMethods.stripIndent((CharSequence)raw) : raw.stripIndent()
def expected = """
return 'foo'
@@ -70,14 +76,16 @@ def method() {
}
void testStripIndentWithFirstLineBackslash() {
- def actual = """\
+ def raw = """\
return 'foo'
}
def method() {
return 'bar'
}
- """.stripIndent()
+ """
+ // hack to avoid conflicts with stripIndent from JDK13+ (resolution in Groovy 3)
+ def actual = isAtLeastJdk('13.0') ? StringGroovyMethods.stripIndent((CharSequence)raw) : raw.stripIndent()
def expected = """\
return 'foo'
diff --git a/src/test/org/codehaus/groovy/runtime/InterfaceConversionTest.groovy b/src/test/org/codehaus/groovy/runtime/InterfaceConversionTest.groovy
index 30bcc93..be6a385 100644
--- a/src/test/org/codehaus/groovy/runtime/InterfaceConversionTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/InterfaceConversionTest.groovy
@@ -45,6 +45,8 @@ class InterfaceConversionTest extends GroovyTestCase {
void testDefaultInterfaceMethodCallOnProxy() {
// reversed is a default method within the Comparator interface for 1.8+
if (!isAtLeastJdk("1.8")) return
+ // broken on JDK14, TODO: FIX
+ if (isAtLeastJdk("14.0")) return
Comparator c1 = { a, b -> a <=> b }
assert c1.compare("a", "b") == -1
def c2 = c1.reversed()
diff --git a/subprojects/tests-vm8/src/test/groovy/gls/annotations/vm8/AnnotationTest.groovy b/subprojects/tests-vm8/src/test/groovy/gls/annotations/vm8/AnnotationTest.groovy
index 436b4a0..7161f57 100644
--- a/subprojects/tests-vm8/src/test/groovy/gls/annotations/vm8/AnnotationTest.groovy
+++ b/subprojects/tests-vm8/src/test/groovy/gls/annotations/vm8/AnnotationTest.groovy
@@ -31,8 +31,9 @@ class AnnotationTest extends GroovyTestCase {
class MyClass {
// TODO confirm the JDK9 behavior is what we expect
private static final List<String> expected = [
- '@vm8.Requires(value=[@vm8.Require(value=val1), @vm8.Require(value=val2)])', // JDK5-8
- '@vm8.Requires(value={@vm8.Require(value="val1"), @vm8.Require(value="val2")})' // JDK9
+ '@vm8.Requires(value=[@vm8.Require(value=val1), @vm8.Require(value=val2)])', // JDK5-8
+ '@vm8.Requires(value={@vm8.Require(value="val1"), @vm8.Require(value="val2")})', // JDK9
+ '@vm8.Requires({@vm8.Require("val1"), @vm8.Require("val2")})' // JDK14
]
// control
[groovy] 02/02: GROOVY-9211: BUG! UNCAUGHT EXCEPTION on
OpenJDK14-ea+8 (adjust for better JDK8 lookup)
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 78430bae4ece202d764a18547669a6f9bf740a24
Author: Paul King <pa...@asert.com.au>
AuthorDate: Mon Feb 24 10:09:14 2020 +1000
GROOVY-9211: BUG! UNCAUGHT EXCEPTION on OpenJDK14-ea+8 (adjust for better JDK8 lookup)
---
.../org/codehaus/groovy/vmplugin/v8/Java8.java | 72 ++++++++++++++++++++++
.../groovy/runtime/InterfaceConversionTest.groovy | 2 -
2 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Java8.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/Java8.java
index f4bd201..3c1bc32 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Java8.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Java8.java
@@ -18,12 +18,17 @@
*/
package org.codehaus.groovy.vmplugin.v8;
+import groovy.lang.GroovyRuntimeException;
import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.CompileUnit;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.vmplugin.v7.Java7;
import java.lang.annotation.ElementType;
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.AccessibleObject;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Type;
import java.util.ArrayList;
@@ -83,4 +88,71 @@ public class Java8 extends Java7 {
}
return params;
}
+
+ private static class LookupHolder {
+ private static final Method PRIVATE_LOOKUP;
+ private static final Constructor<MethodHandles.Lookup> LOOKUP_Constructor;
+
+ static {
+ Constructor<MethodHandles.Lookup> lookup = null;
+ Method privateLookup = null;
+ try { // java 9+ friendly
+ privateLookup = MethodHandles.class.getMethod("privateLookupIn", Class.class, MethodHandles.Lookup.class);
+ } catch (final NoSuchMethodException | RuntimeException e) { // java 8 or fallback if anything else goes wrong
+ try {
+ lookup = MethodHandles.Lookup.class.getDeclaredConstructor(Class.class, Integer.TYPE);
+ if (!lookup.isAccessible()) {
+ trySetAccessible(lookup);
+ }
+ } catch (final NoSuchMethodException ex) {
+ throw new IllegalStateException("Incompatible JVM", e);
+ }
+ }
+ PRIVATE_LOOKUP = privateLookup;
+ LOOKUP_Constructor = lookup;
+ }
+ }
+
+ private static boolean trySetAccessible(AccessibleObject ao) {
+ try {
+ ao.setAccessible(true);
+ return true;
+ } catch (SecurityException e) {
+ throw e;
+ } catch (Throwable t) {
+ return false;
+ }
+ }
+
+ private static Constructor<MethodHandles.Lookup> getLookupConstructor() {
+ return LookupHolder.LOOKUP_Constructor;
+ }
+
+ private static Method getPrivateLookup() {
+ return LookupHolder.PRIVATE_LOOKUP;
+ }
+
+ public static MethodHandles.Lookup of(final Class<?> declaringClass) {
+ try {
+ final Method privateLookup = getPrivateLookup();
+ if (privateLookup != null) {
+ return (MethodHandles.Lookup) privateLookup.invoke(null, declaringClass, MethodHandles.lookup());
+ }
+ return getLookupConstructor().newInstance(declaringClass, MethodHandles.Lookup.PRIVATE).in(declaringClass);
+ } catch (final IllegalAccessException | InstantiationException e) {
+ throw new IllegalArgumentException(e);
+ } catch (final InvocationTargetException e) {
+ throw new GroovyRuntimeException(e);
+ }
+ }
+
+ @Override
+ public Object getInvokeSpecialHandle(Method method, Object receiver) {
+ final Class<?> receiverType = receiver.getClass();
+ try {
+ return of(receiverType).unreflectSpecial(method, receiverType).bindTo(receiver);
+ } catch (ReflectiveOperationException e) {
+ return super.getInvokeSpecialHandle(method, receiver);
+ }
+ }
}
diff --git a/src/test/org/codehaus/groovy/runtime/InterfaceConversionTest.groovy b/src/test/org/codehaus/groovy/runtime/InterfaceConversionTest.groovy
index be6a385..30bcc93 100644
--- a/src/test/org/codehaus/groovy/runtime/InterfaceConversionTest.groovy
+++ b/src/test/org/codehaus/groovy/runtime/InterfaceConversionTest.groovy
@@ -45,8 +45,6 @@ class InterfaceConversionTest extends GroovyTestCase {
void testDefaultInterfaceMethodCallOnProxy() {
// reversed is a default method within the Comparator interface for 1.8+
if (!isAtLeastJdk("1.8")) return
- // broken on JDK14, TODO: FIX
- if (isAtLeastJdk("14.0")) return
Comparator c1 = { a, b -> a <=> b }
assert c1.compare("a", "b") == -1
def c2 = c1.reversed()