You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/08/06 18:47:36 UTC

[groovy] branch groovy5-java11 created (now 058d48544b)

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a change to branch groovy5-java11
in repository https://gitbox.apache.org/repos/asf/groovy.git


      at 058d48544b GROOVY-10278: `CompilerConfiguration`: improve target bytecode selection

This branch includes the following new commits:

     new 058d48544b GROOVY-10278: `CompilerConfiguration`: improve target bytecode selection

The 1 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.



[groovy] 01/01: GROOVY-10278: `CompilerConfiguration`: improve target bytecode selection

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch groovy5-java11
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 058d48544b1faa9ed4a7d18033e0568653e98ba0
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 2bd1573e7a..d586c7bc30 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();