GROOVY-11286: optimize void static method return in expression statement

apache · Oct 13, 2024 · 7b070d8 · 7b070d8
1 parent 898cedb
commit 7b070d8
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 22 deletions.
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
@@ -30,6 +30,7 @@
 import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
 import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.MethodCall;
 import org.codehaus.groovy.ast.expr.MethodCallExpression;
 import org.codehaus.groovy.ast.expr.PropertyExpression;
 import org.codehaus.groovy.ast.expr.SpreadExpression;
@@ -95,28 +96,32 @@ public class InvocationWriter {
     public static final MethodCallerMultiAdapter invokeStaticMethod = MethodCallerMultiAdapter.newStatic(ScriptBytecodeAdapter.class, "invokeStaticMethod", true, true);
     @Deprecated(since = "5.0.0")
     public static final MethodCaller invokeClosureMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "invokeClosure");
-    public static final MethodCaller castToVargsArray = MethodCaller.newStatic(DefaultTypeTransformation.class, "castToVargsArray");
-    private static final MethodNode CLASS_FOR_NAME_STRING = ClassHelper.CLASS_Type.getDeclaredMethod("forName", new Parameter[]{new Parameter(ClassHelper.STRING_TYPE, "name")});
+    public static final MethodCaller castToVargsArray    = MethodCaller.newStatic(DefaultTypeTransformation.class, "castToVargsArray");
 
-    // type conversions
-    private static final MethodCaller asTypeMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "asType");
-    private static final MethodCaller castToTypeMethod = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "castToType");
-    private static final MethodCaller castToClassMethod = MethodCaller.newStatic(ShortTypeHandling.class, "castToClass");
+    // type conversion
+    private static final MethodCaller asTypeMethod       = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "asType");
+    private static final MethodCaller castToTypeMethod   = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "castToType");
+    private static final MethodCaller castToClassMethod  = MethodCaller.newStatic(ShortTypeHandling.class, "castToClass");
     private static final MethodCaller castToStringMethod = MethodCaller.newStatic(ShortTypeHandling.class, "castToString");
-    private static final MethodCaller castToEnumMethod = MethodCaller.newStatic(ShortTypeHandling.class, "castToEnum");
+    private static final MethodCaller castToEnumMethod   = MethodCaller.newStatic(ShortTypeHandling.class, "castToEnum");
 
     // constructor calls with this() and super()
     private static final MethodCaller selectConstructorAndTransformArguments = MethodCaller.newStatic(ScriptBytecodeAdapter.class, "selectConstructorAndTransformArguments");
 
+    private static final MethodNode CLASS_FOR_NAME_STRING = ClassHelper.CLASS_Type.getDeclaredMethod("forName", new Parameter[]{new Parameter(ClassHelper.STRING_TYPE, "")});
+
     protected final WriterController controller;
 
-    protected MethodCallExpression currentCall;
+    protected Expression currentCall;
 
     public InvocationWriter(final WriterController controller) {
         this.controller = controller;
     }
 
     public void makeCall(final Expression origin, final Expression receiver, final Expression message, final Expression arguments, final MethodCallerMultiAdapter adapter, boolean safe, final boolean spreadSafe, boolean implicitThis) {
+        var oldCall = currentCall;
+        currentCall = origin instanceof MethodCall ? origin : null;
+
         ClassNode sender;
         if (isSuperExpression(receiver) || (isThisExpression(receiver) && !implicitThis)) {
             // GROOVY-6045, GROOVY-8693, et al.
@@ -127,6 +132,8 @@ public void makeCall(final Expression origin, final Expression receiver, final E
             sender = controller.getClassNode();
         }
         makeCall(origin, new ClassExpression(sender), receiver, message, arguments, adapter, safe, spreadSafe, implicitThis);
+
+        currentCall = oldCall;
     }
 
     protected void makeCall(final Expression origin, final ClassExpression sender, final Expression receiver, final Expression message, final Expression arguments, final MethodCallerMultiAdapter adapter, final boolean safe, final boolean spreadSafe, final boolean implicitThis) {
@@ -425,6 +432,14 @@ protected boolean makeClassForNameCall(final Expression origin, final Expression
         return writeDirectMethodCall(CLASS_FOR_NAME_STRING, false, receiver, ae);
     }
 
+    /**
+     * Converts an expression to an argument list.
+     *
+     * @return {@code arguments} if already an argument list or an argument list
+     *         of the expression or expressions (in case of a tuple expression).
+     *
+     * @since 2.0.0
+     */
     public static ArgumentListExpression makeArgumentList(final Expression arguments) {
         ArgumentListExpression ae;
         if (arguments instanceof ArgumentListExpression) {
@@ -459,9 +474,6 @@ protected String getMethodName(final Expression message) {
     }
 
     public void writeInvokeMethod(MethodCallExpression call) {
-        var oldCall = currentCall;
-        currentCall = call;
-
         Expression receiver = call.getObjectExpression();
         // GROOVY-8466: replace "rcvr.call(args)" with "rcvr.abstractMethod(args)"
         if (!isThisExpression(receiver) && "call".equals(call.getMethodAsString())) {
@@ -479,8 +491,6 @@ public void writeInvokeMethod(MethodCallExpression call) {
         }
         Expression messageName = new CastExpression(ClassHelper.STRING_TYPE, call.getMethod());
         makeCall(call, receiver, messageName, call.getArguments(), adapter, call.isSafe(), call.isSpreadSafe(), call.isImplicitThis());
-
-        currentCall = oldCall;
     }
 
     private static MethodCallExpression transformToRealMethodCall(final MethodCallExpression call, final ClassNode type) {

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -26,6 +26,7 @@
 import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.EmptyExpression;
 import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.MethodCall;
 import org.codehaus.groovy.ast.expr.MethodCallExpression;
 import org.codehaus.groovy.ast.expr.NotExpression;
 import org.codehaus.groovy.ast.stmt.AssertStatement;
@@ -608,11 +609,11 @@ public void writeExpressionStatement(final ExpressionStatement statement) {
         controller.getAcg().onLineNumber(statement, "visitExpressionStatement: " + expression.getClass().getName());
         writeStatementLabel(statement);
 
-        // TODO: replace with better meta data key
+        // TODO: replace with better metadata
+        if (expression instanceof MethodCall)
+            expression.putNodeMetaData("GROOVY-11286", Boolean.TRUE);
         if (expression instanceof BinaryExpression)
             expression.putNodeMetaData("GROOVY-11288", Boolean.TRUE);
-        if (expression instanceof MethodCallExpression)
-            expression.putNodeMetaData("GROOVY-11286", Boolean.TRUE);
 
         var operandStack = controller.getOperandStack();
         int mark = operandStack.getStackLength();

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -111,7 +111,7 @@ public StaticInvocationWriter(final WriterController wc) {
         super(wc);
     }
 
-    MethodCallExpression getCurrentCall() {
+    Expression getCurrentCall() {
         return currentCall;
     }
 
@@ -420,9 +420,12 @@ protected void loadArguments(final List<Expression> argumentList, final Paramete
                     arguments[i] = a;
                     j += 1;
                 } else {
-                    controller.getSourceUnit().addFatalError("Binding failed" +
-                            " for arguments [" + argumentList.stream().map(arg -> typeChooser.resolveType(arg, classNode).toString(false)).collect(Collectors.joining(", ")) + "]" +
-                            " and parameters [" + Arrays.stream(parameters).map(prm -> prm.getType().toString(false)).collect(Collectors.joining(", ")) + "]", getCurrentCall());
+                    String errorMessage = "Binding failed for arguments [" +
+                            argumentList.stream().map(arg -> typeChooser.resolveType(arg, classNode).toString(false)).collect(Collectors.joining(", ")) +
+                            "] and parameters [" +
+                            Arrays.stream(parameters).map(prm -> prm.getType().toString(false)).collect(Collectors.joining(", ")) +
+                            "]";
+                    controller.getSourceUnit().addFatalError(errorMessage, currentCall);
                 }
             }
             for (int i = 0; i < nArgs; i += 1) {

diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -365,7 +365,7 @@ public void makeGroovyObjectGetPropertySite(final Expression receiver, final Str
         }
 
         if (implicitThis && controller.getInvocationWriter() instanceof StaticInvocationWriter) {
-            Expression currentCall = ((StaticInvocationWriter) controller.getInvocationWriter()).getCurrentCall();
+            var currentCall = ((StaticInvocationWriter) controller.getInvocationWriter()).getCurrentCall();
             if (currentCall != null) {
                 String implicitReceiver = currentCall.getNodeMetaData(StaticTypesMarker.IMPLICIT_RECEIVER);
                 if (implicitReceiver != null) {

diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy
@@ -279,7 +279,7 @@ final class StaticCompilationTest extends AbstractBytecodeTestCase {
     }
 
     // GROOVY-11286
-    void testVoidMethod() {
+    void testVoidMethod1() {
         assert compile(method: 'm', '''@groovy.transform.CompileStatic
             void m() {
                 print ""
@@ -297,6 +297,24 @@ final class StaticCompilationTest extends AbstractBytecodeTestCase {
             ])
     }
 
+    // GROOVY-11286
+    void testVoidMethod2() {
+        assert compile(method: 'm', '''import static java.lang.System.gc
+            @groovy.transform.CompileStatic
+            void m() {
+                gc()
+            }
+        ''').hasStrictSequence([
+                'L0',
+                'LINENUMBER 4',
+                'INVOKESTATIC java/lang/System.gc ()V',
+                // drop: ACONST_NULL, POP
+                'L1',
+                'LINENUMBER 5',
+                'RETURN'
+            ])
+    }
+
     void testIntLeftShift() {
         assert compile(method: 'm', '''@groovy.transform.CompileStatic
             void m() {