You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2022/09/17 16:55:32 UTC
[groovy] branch master updated: GROOVY-10278: `CompilerConfiguration`: improve target bytecode selection
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 0d536b522a GROOVY-10278: `CompilerConfiguration`: improve target bytecode selection
0d536b522a is described below
commit 0d536b522a942c6377a2b614719f1e36d7c9d89a
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Aug 6 13:47:17 2022 -0500
GROOVY-10278: `CompilerConfiguration`: improve target bytecode selection
---
.github/workflows/groovy-build-test.yml | 2 +-
.../groovy/control/CompilerConfiguration.java | 68 +++++++++-------------
.../groovy/control/CompilerConfigurationTest.java | 24 +++++---
.../groovy/runtime/InterfaceConversionMapTest.java | 16 +----
.../main/java/org/codehaus/groovy/ant/Groovyc.java | 13 ++---
5 files changed, 53 insertions(+), 70 deletions(-)
diff --git a/.github/workflows/groovy-build-test.yml b/.github/workflows/groovy-build-test.yml
index e17cce8db7..d9d062c458 100644
--- a/.github/workflows/groovy-build-test.yml
+++ b/.github/workflows/groovy-build-test.yml
@@ -26,7 +26,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-20.04]
- java: [8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18]
+ java: [11, 12, 13, 14, 15, 16, 17, 18]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v3
diff --git a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java
index 1654281f6a..a6b121bf66 100644
--- a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java
+++ b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java
@@ -67,19 +67,19 @@ public class CompilerConfiguration {
public static final String MEM_STUB = "memStub";
/** This (<code>"1.4"</code>) is the value for targetBytecode to compile for a JDK 1.4. */
- public static final String JDK4 = "1.4";
+ @Deprecated public static final String JDK4 = "1.4";
/** This (<code>"1.5"</code>) is the value for targetBytecode to compile for a JDK 1.5. */
- public static final String JDK5 = "1.5";
+ @Deprecated public static final String JDK5 = "1.5";
/** This (<code>"1.6"</code>) is the value for targetBytecode to compile for a JDK 1.6. */
- public static final String JDK6 = "1.6";
+ @Deprecated public static final String JDK6 = "1.6";
/** This (<code>"1.7"</code>) is the value for targetBytecode to compile for a JDK 1.7. */
- public static final String JDK7 = "1.7";
+ @Deprecated public static final String JDK7 = "1.7";
/** This (<code>"1.8"</code>) is the value for targetBytecode to compile for a JDK 1.8. */
- public static final String JDK8 = "1.8";
+ @Deprecated public static final String JDK8 = "1.8";
/** This (<code>"9"</code>) is the value for targetBytecode to compile for a JDK 9. */
- public static final String JDK9 = "9";
+ @Deprecated public static final String JDK9 = "9";
/** This (<code>"10"</code>) is the value for targetBytecode to compile for a JDK 10. */
- public static final String JDK10 = "10";
+ @Deprecated public static final String JDK10 = "10";
/** This (<code>"11"</code>) is the value for targetBytecode to compile for a JDK 11. */
public static final String JDK11 = "11";
/** This (<code>"12"</code>) is the value for targetBytecode to compile for a JDK 12. */
@@ -99,31 +99,10 @@ public class CompilerConfiguration {
/** This (<code>"19"</code>) is the value for targetBytecode to compile for a JDK 19. */
public static final String JDK19 = "19";
- /**
- * This constant is for comparing targetBytecode to ensure it is set to JDK 1.5 or later.
- * @deprecated
- */
- @Deprecated
- public static final String POST_JDK5 = JDK5;
-
- /**
- * This constant is for comparing targetBytecode to ensure it is set to an earlier value than JDK 1.5.
- * @deprecated
- */
- @Deprecated
- public static final String PRE_JDK5 = JDK4;
-
/**
* JDK version to bytecode version mapping.
*/
public static final Map<String, Integer> JDK_TO_BYTECODE_VERSION_MAP = Maps.of(
- JDK4, Opcodes.V1_4,
- JDK5, Opcodes.V1_5,
- JDK6, Opcodes.V1_6,
- JDK7, Opcodes.V1_7,
- JDK8, Opcodes.V1_8,
- JDK9, Opcodes.V9,
- JDK10, Opcodes.V10,
JDK11, Opcodes.V11,
JDK12, Opcodes.V12,
JDK13, Opcodes.V13,
@@ -512,7 +491,7 @@ public class CompilerConfiguration {
public CompilerConfiguration(final CompilerConfiguration configuration) {
setWarningLevel(configuration.getWarningLevel());
setTargetDirectory(configuration.getTargetDirectory());
- setClasspathList(new LinkedList<>(configuration.getClasspath()));
+ setClasspathList(configuration.getClasspath());
setVerbose(configuration.getVerbose());
setDebug(configuration.getDebug());
setParameters(configuration.getParameters());
@@ -601,6 +580,7 @@ public class CompilerConfiguration {
* @param bytecodeVersion The parameter can take one of the values in {@link #ALLOWED_JDKS}.
* @return true if the bytecode version is JDK 1.5+
*/
+ @Deprecated
public static boolean isPostJDK5(final String bytecodeVersion) {
return isAtLeast(bytecodeVersion, JDK5);
}
@@ -611,6 +591,7 @@ public class CompilerConfiguration {
* @param bytecodeVersion The parameter can take one of the values in {@link #ALLOWED_JDKS}.
* @return true if the bytecode version is JDK 1.7+
*/
+ @Deprecated
public static boolean isPostJDK7(final String bytecodeVersion) {
return isAtLeast(bytecodeVersion, JDK7);
}
@@ -621,6 +602,7 @@ public class CompilerConfiguration {
* @param bytecodeVersion The parameter can take one of the values in {@link #ALLOWED_JDKS}.
* @return true if the bytecode version is JDK 1.8+
*/
+ @Deprecated
public static boolean isPostJDK8(final String bytecodeVersion) {
return isAtLeast(bytecodeVersion, JDK8);
}
@@ -631,6 +613,7 @@ public class CompilerConfiguration {
* @param bytecodeVersion The parameter can take one of the values in {@link #ALLOWED_JDKS}.
* @return true if the bytecode version is JDK 9+
*/
+ @Deprecated
public static boolean isPostJDK9(final String bytecodeVersion) {
return isAtLeast(bytecodeVersion, JDK9);
}
@@ -641,6 +624,7 @@ public class CompilerConfiguration {
* @param bytecodeVersion The parameter can take one of the values in {@link #ALLOWED_JDKS}.
* @return true if the bytecode version is JDK 10+
*/
+ @Deprecated
public static boolean isPostJDK10(final String bytecodeVersion) {
return isAtLeast(bytecodeVersion, JDK10);
}
@@ -832,7 +816,6 @@ public class CompilerConfiguration {
}
}
-
/**
* Gets the currently configured warning level. See {@link WarningMessage}
* for level details.
@@ -1070,8 +1053,8 @@ public class CompilerConfiguration {
}
/**
- * Sets the bytecode compatibility level. The parameter can take one of the values
- * in {@link #ALLOWED_JDKS}.
+ * Sets the bytecode compatibility level. The parameter can take one of the
+ * values in {@link #ALLOWED_JDKS}.
*
* @param version the bytecode compatibility level
*/
@@ -1080,8 +1063,12 @@ public class CompilerConfiguration {
}
private void setTargetBytecodeIfValid(final String version) {
- if (JDK_TO_BYTECODE_VERSION_MAP.containsKey(version)) {
- this.targetBytecode = version;
+ int index = Arrays.binarySearch(ALLOWED_JDKS, !version.startsWith("1") ? "1." + version : version);
+ if (index >= 0) {
+ targetBytecode = ALLOWED_JDKS[index];
+ } else {
+ index = Math.abs(index) - 2; // closest version
+ targetBytecode = ALLOWED_JDKS[Math.max(0, index)];
}
}
@@ -1096,20 +1083,19 @@ public class CompilerConfiguration {
}
/**
- * Returns the ASM bytecode version
+ * Returns the targeted bytecode (aka Java class file) version number.
*
- * @return ASM bytecode version
* @since 4.0.0
*/
- public int getBytecodeVersion() {
- Integer bytecodeVersion = CompilerConfiguration.JDK_TO_BYTECODE_VERSION_MAP.get(targetBytecode);
+ public final int getBytecodeVersion() {
+ Integer bytecodeVersion = JDK_TO_BYTECODE_VERSION_MAP.get(getTargetBytecode());
if (bytecodeVersion == null) {
- throw new GroovyBugError("Bytecode version [" + targetBytecode + "] is not supported by the compiler");
+ throw new GroovyBugError("Bytecode version '" + getTargetBytecode() + "' is not supported by the compiler");
}
if (bytecodeVersion <= Opcodes.V1_8) {
return Opcodes.V1_8;
- } else if (previewFeatures) {
+ } else if (isPreviewFeatures()) {
return bytecodeVersion | Opcodes.V_PREVIEW;
} else {
return bytecodeVersion;
@@ -1127,7 +1113,7 @@ public class CompilerConfiguration {
if (JDK_TO_BYTECODE_VERSION_MAP.containsKey(javaVersion)) {
return javaVersion;
}
- return JDK8;
+ return JDK11;
}
/**
diff --git a/src/test/org/codehaus/groovy/control/CompilerConfigurationTest.java b/src/test/org/codehaus/groovy/control/CompilerConfigurationTest.java
index 5673552407..25a5cc0fb3 100644
--- a/src/test/org/codehaus/groovy/control/CompilerConfigurationTest.java
+++ b/src/test/org/codehaus/groovy/control/CompilerConfigurationTest.java
@@ -25,9 +25,11 @@ import org.junit.Before;
import org.junit.Test;
import java.io.File;
+import java.util.Arrays;
import java.util.Collections;
import java.util.Properties;
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -113,7 +115,7 @@ public final class CompilerConfigurationTest {
init.setMinimumRecompilationInterval(234);
init.setScriptBaseClass("blarg.foo.WhatSit");
init.setSourceEncoding("LEAD-123");
- init.setTargetBytecode(CompilerConfiguration.JDK5);
+ init.setTargetBytecode(CompilerConfiguration.JDK17);
init.setRecompileGroovySource(true);
init.setClasspath("File1" + File.pathSeparator + "Somewhere");
File targetDirectory = new File("A wandering path");
@@ -134,7 +136,7 @@ public final class CompilerConfigurationTest {
assertEquals(234, init.getMinimumRecompilationInterval());
assertEquals("blarg.foo.WhatSit", init.getScriptBaseClass());
assertEquals("LEAD-123", init.getSourceEncoding());
- assertEquals(CompilerConfiguration.JDK5, init.getTargetBytecode());
+ assertEquals(CompilerConfiguration.JDK17, init.getTargetBytecode());
assertTrue(init.getRecompileGroovySource());
assertEquals("File1", init.getClasspath().get(0));
assertEquals("Somewhere", init.getClasspath().get(1));
@@ -158,7 +160,7 @@ public final class CompilerConfigurationTest {
assertEquals(234, config.getMinimumRecompilationInterval());
assertEquals("blarg.foo.WhatSit", config.getScriptBaseClass());
assertEquals("LEAD-123", config.getSourceEncoding());
- assertEquals(CompilerConfiguration.JDK5, config.getTargetBytecode());
+ assertEquals(CompilerConfiguration.JDK17, config.getTargetBytecode());
assertTrue(config.getRecompileGroovySource());
assertEquals("File1", config.getClasspath().get(0));
assertEquals("Somewhere", config.getClasspath().get(1));
@@ -184,7 +186,7 @@ public final class CompilerConfigurationTest {
init.setMinimumRecompilationInterval(975);
init.setScriptBaseClass("");
init.setSourceEncoding("Gutenberg");
- init.setTargetBytecode(CompilerConfiguration.JDK5);
+ init.setTargetBytecode(CompilerConfiguration.JDK17);
init.setRecompileGroovySource(false);
init.setClasspath("");
File targetDirectory = new File("A wandering path");
@@ -203,7 +205,7 @@ public final class CompilerConfigurationTest {
assertEquals(975, init.getMinimumRecompilationInterval());
assertEquals("", init.getScriptBaseClass());
assertEquals("Gutenberg", init.getSourceEncoding());
- assertEquals(CompilerConfiguration.JDK5, init.getTargetBytecode());
+ assertEquals(CompilerConfiguration.JDK17, init.getTargetBytecode());
assertFalse(init.getRecompileGroovySource());
assertEquals(Collections.emptyList(), init.getClasspath());
assertEquals(targetDirectory, init.getTargetDirectory());
@@ -224,7 +226,7 @@ public final class CompilerConfigurationTest {
assertEquals(975, config.getMinimumRecompilationInterval());
assertEquals("", config.getScriptBaseClass());
assertEquals("Gutenberg", config.getSourceEncoding());
- assertEquals(CompilerConfiguration.JDK5, config.getTargetBytecode());
+ assertEquals(CompilerConfiguration.JDK17, config.getTargetBytecode());
assertFalse(config.getRecompileGroovySource());
assertEquals(Collections.emptyList(), config.getClasspath());
assertEquals(targetDirectory, config.getTargetDirectory());
@@ -276,7 +278,7 @@ public final class CompilerConfigurationTest {
config.setSourceEncoding("Gutenberg");
});
assertThrows(UnsupportedOperationException.class, () -> {
- config.setTargetBytecode(CompilerConfiguration.JDK5);
+ config.setTargetBytecode("11");
});
assertThrows(UnsupportedOperationException.class, () -> {
config.setTargetDirectory(new File("path"));
@@ -291,4 +293,12 @@ public final class CompilerConfigurationTest {
config.setWarningLevel(WarningMessage.POSSIBLE_ERRORS);
});
}
+
+ @Test // GROOVY-10278
+ public void testTargetVersion() {
+ CompilerConfiguration config = new CompilerConfiguration();
+ String[] inputs = {"1.3", "1.4", "1.5", "1.6", "1.7", "1.8", "1.9", "5" , "6" , "7" , "8" , "9" , "9.0", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19"};
+ String[] expect = {"11" , "11" , "11" , "11" , "11" , "11" , "11" , "11", "11", "11", "11", "11", "11" , "11", "11", "12", "13", "14", "15", "16", "17", "18", "19"};
+ assertArrayEquals(expect, Arrays.stream(inputs).map(v -> { config.setTargetBytecode(v); return config.getTargetBytecode(); }).toArray(String[]::new));
+ }
}
diff --git a/src/test/org/codehaus/groovy/runtime/InterfaceConversionMapTest.java b/src/test/org/codehaus/groovy/runtime/InterfaceConversionMapTest.java
index 5c67bd0ff1..53bb67bbf5 100644
--- a/src/test/org/codehaus/groovy/runtime/InterfaceConversionMapTest.java
+++ b/src/test/org/codehaus/groovy/runtime/InterfaceConversionMapTest.java
@@ -19,26 +19,16 @@
package org.codehaus.groovy.runtime;
import groovy.lang.GroovyShell;
-import org.codehaus.groovy.control.CompilerConfiguration;
import org.junit.Assert;
import org.junit.Test;
import java.util.Map;
-public class InterfaceConversionMapTest {
+public final class InterfaceConversionMapTest {
- private final GroovyShell shell;
-
- public InterfaceConversionMapTest() {
- final CompilerConfiguration cc = new CompilerConfiguration();
- cc.setTargetBytecode(CompilerConfiguration.JDK8);
- shell = new GroovyShell(cc);
- }
-
- // GROOVY-7330
- @Test
+ @Test // GROOVY-7330
public void testMapToProxy() {
- final Map map = (Map) shell.evaluate("[x: {10}, y: {20}]");
+ final Map map = (Map) new GroovyShell().evaluate("[x: {10}, y: {20}]");
final SomeInterface si = DefaultGroovyMethods.asType(map, SomeInterface.class);
Assert.assertEquals(20, si.y());
Assert.assertEquals(10, si.x());
diff --git a/subprojects/groovy-ant/src/main/java/org/codehaus/groovy/ant/Groovyc.java b/subprojects/groovy-ant/src/main/java/org/codehaus/groovy/ant/Groovyc.java
index 3fcdbaf55c..c583586057 100644
--- a/subprojects/groovy-ant/src/main/java/org/codehaus/groovy/ant/Groovyc.java
+++ b/subprojects/groovy-ant/src/main/java/org/codehaus/groovy/ant/Groovyc.java
@@ -306,13 +306,8 @@ public class Groovyc extends MatchingTask {
*
* @param version the bytecode compatibility level
*/
- public void setTargetBytecode(String version) {
- for (String allowedJdk : CompilerConfiguration.ALLOWED_JDKS) {
- if (allowedJdk.equals(version)) {
- this.targetBytecode = version;
- break;
- }
- }
+ public void setTargetBytecode(final String version) {
+ this.targetBytecode = version;
}
/**
@@ -1126,7 +1121,9 @@ public class Groovyc extends MatchingTask {
commandLineList.add("-Xmx" + memoryMaximumSize);
}
if (targetBytecode != null) {
- commandLineList.add("-Dgroovy.target.bytecode=" + targetBytecode);
+ CompilerConfiguration cc = new CompilerConfiguration();
+ cc.setTargetBytecode(targetBytecode); // GROOVY-10278: nearest valid value
+ commandLineList.add("-Dgroovy.target.bytecode=" + cc.getTargetBytecode());
}
if (!"*.groovy".equals(getScriptExtension())) {
String tmpExtension = getScriptExtension();