GROOVY-11630: optimize void groovy method return in expression statement

Author: eric-milles

src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java

@@ -264,10 +264,14 @@ public class AsmClassGenerator extends ClassGenerator {
 
     private final Map<String,ClassNode> referencedClasses = new HashMap<>();
 
+    /**
+     * Add marker in the bytecode to show source-bytecode relationship.
+     */
+    public static final boolean ASM_DEBUG = false;
     public static final boolean CREATE_DEBUG_INFO = true;
     public static final boolean CREATE_LINE_NUMBER_INFO = true;
-    public static final boolean ASM_DEBUG = false; // add marker in the bytecode to show source-bytecode relationship
-    public static final String MINIMUM_BYTECODE_VERSION = "_MINIMUM_BYTECODE_VERSION";
+    public static final String  ELIDE_EXPRESSION_VALUE = "_EXPR_VALUE_UNUSED";
+    public static final String  MINIMUM_BYTECODE_VERSION = "_MINIMUM_BYTECODE_VERSION";
 
     private WriterController controller;
     private ASTNode currentASTNode;

src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java

@@ -376,7 +376,7 @@ protected void assignToArray(final Expression parent, final Expression receiver,
         controller.getInvocationWriter().makeCall(parent, receiver, constX("putAt"), args(index, rhsValueLoader), InvocationWriter.invokeMethod, safe, false, false);
         controller.getOperandStack().pop(); // method return value
 
-        if (!Boolean.TRUE.equals(parent.getNodeMetaData("GROOVY-11288")))
+        if (!Boolean.TRUE.equals(parent.getNodeMetaData(AsmClassGenerator.ELIDE_EXPRESSION_VALUE)))
             rhsValueLoader.visit(controller.getAcg()); // assignment expression value
     }
 
@@ -401,7 +401,7 @@ public void evaluateEqual(final BinaryExpression expression, final boolean defin
         Expression rightExpression = expression.getRightExpression();
         boolean singleAssignment = !(leftExpression instanceof TupleExpression);
         boolean directAssignment = defineVariable && singleAssignment; //def x=y
-        boolean returnRightValue = !Boolean.TRUE.equals(expression.getNodeMetaData("GROOVY-11288"));
+        boolean returnRightValue = !Boolean.TRUE.equals(expression.getNodeMetaData(AsmClassGenerator.ELIDE_EXPRESSION_VALUE));
 
         // TODO: LHS has not been visited -- it could be a variable in a closure and type chooser is not aware.
         ClassNode lhsType = controller.getTypeChooser().resolveType(leftExpression, controller.getClassNode());

src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionMultiTypeDispatcher.java

@@ -380,7 +380,7 @@ protected void assignToArray(final Expression orig, final Expression receiver, f
             // update operand stack
             operandStack.remove(3);
 
-            if (!Boolean.TRUE.equals(orig.getNodeMetaData("GROOVY-11288")))
+            if (!Boolean.TRUE.equals(orig.getNodeMetaData(AsmClassGenerator.ELIDE_EXPRESSION_VALUE)))
                 rhsValueLoader.visit(asmGenerator); // re-load result value
         } else {
             super.assignToArray(orig, receiver, index, rhsValueLoader, safe);

src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java

@@ -238,7 +238,7 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i
         operandStack.remove(operandStack.getStackLength() - startDepth); // receiver plus argument(s)
 
         if (isPrimitiveVoid(returnType)) {
-            if (currentCall != null && currentCall.getNodeMetaData("GROOVY-11286") != null) {
+            if (currentCall != null && currentCall.getNodeMetaData(AsmClassGenerator.ELIDE_EXPRESSION_VALUE) != null) {
                 return true; // do not load value
             }
             returnType = ClassHelper.OBJECT_TYPE;

src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java

@@ -48,6 +48,7 @@
 import org.codehaus.groovy.ast.stmt.ThrowStatement;
 import org.codehaus.groovy.ast.stmt.TryCatchStatement;
 import org.codehaus.groovy.ast.stmt.WhileStatement;
+import org.codehaus.groovy.classgen.AsmClassGenerator;
 import org.codehaus.groovy.classgen.asm.CompileStack.BlockRecorder;
 import org.objectweb.asm.Label;
 import org.objectweb.asm.MethodVisitor;
@@ -632,11 +633,8 @@ public void writeExpressionStatement(final ExpressionStatement statement) {
         controller.getAcg().onLineNumber(statement, "visitExpressionStatement: " + expression.getClass().getName());
         writeStatementLabel(statement);
 
-        // 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 MethodCall || expression instanceof BinaryExpression)
+            expression.putNodeMetaData(AsmClassGenerator.ELIDE_EXPRESSION_VALUE, Boolean.TRUE);
 
         var operandStack = controller.getOperandStack();
         int mark = operandStack.getStackLength();

src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java

@@ -288,6 +288,9 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i
             controller.getOperandStack().remove(argumentList.size());
 
             if (isPrimitiveVoid(returnType)) {
+                if (currentCall != null && currentCall.getNodeMetaData(AsmClassGenerator.ELIDE_EXPRESSION_VALUE) != null) {
+                    return true; // do not load value
+                }
                 returnType = ClassHelper.OBJECT_TYPE;
                 mv.visitInsn(ACONST_NULL);
             }
@@ -490,7 +493,7 @@ public void makeCall(final Expression origin, final Expression receiver, final E
             Label allDone = new Label();
             MethodVisitor mv = controller.getMethodVisitor();
             OperandStack operandStack = controller.getOperandStack();
-            boolean produceResultList = origin.getNodeMetaData("GROOVY-11286") == null;
+            boolean produceResultList = origin.getNodeMetaData(AsmClassGenerator.ELIDE_EXPRESSION_VALUE) == null;
 
             // if (receiver == null)
             tmpReceiver.visit(controller.getAcg());

src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java

@@ -64,6 +64,7 @@
 import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
+import static org.codehaus.groovy.classgen.AsmClassGenerator.ELIDE_EXPRESSION_VALUE;
 import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.PRIVATE_FIELDS_MUTATORS;
 import static org.codehaus.groovy.transform.sc.StaticCompilationVisitor.ARRAYLIST_ADD_METHOD;
 import static org.codehaus.groovy.transform.sc.StaticCompilationVisitor.ARRAYLIST_CLASSNODE;
@@ -342,7 +343,7 @@ protected void assignToArray(final Expression enclosing, final Expression receiv
             call.visit(controller.getAcg());
             controller.getOperandStack().pop(); // method return value
 
-            if (!Boolean.TRUE.equals(enclosing.getNodeMetaData("GROOVY-11288")))
+            if (!Boolean.TRUE.equals(enclosing.getNodeMetaData(ELIDE_EXPRESSION_VALUE)))
                 rhsValueLoader.visit(controller.getAcg()); // assignment expression value
         }
     }

src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy

@@ -430,6 +430,25 @@ final class StaticCompilationTest extends AbstractBytecodeTestCase {
             ])
     }
 
+    // GROOVY-11630
+    void testVoidMethod5() {
+        assert compile(method: 'm', '''@groovy.transform.CompileStatic
+            void m() {
+                def list = [1,2,3]
+                list.shuffle()
+            }
+        ''').hasStrictSequence([
+                'L1',
+                'LINENUMBER 4 L1',
+                'ALOAD 1',
+                'INVOKESTATIC org/codehaus/groovy/runtime/DefaultGroovyMethods.shuffle (Ljava/util/List;)V',
+                // drop: ACONST_NULL, POP
+                'L2',
+                'LINENUMBER 5 L2',
+                'RETURN'
+            ])
+    }
+
     void testIntLeftShift() {
         assert compile(method: 'm', '''@groovy.transform.CompileStatic
             void m() {