From 338929cefcf6c3d95accdf1494c53eb6be40e63f Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Mon, 14 Oct 2024 10:04:43 -0500 Subject: [PATCH] GROOVY-11286: optimize void method return in expression statement (safe) --- .../asm/sc/StaticInvocationWriter.java | 46 ++++++++----------- .../asm/sc/StaticPropertyAccessHelper.java | 10 ++-- .../asm/sc/StaticCompilationTest.groovy | 33 +++++++++++++ 3 files changed, 59 insertions(+), 30 deletions(-) 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 index 71c6cbe6acc..543aebb3043 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java @@ -563,44 +563,36 @@ public void makeCall(final Expression origin, final Expression receiver, final E ((TemporaryVariableExpression) tmpReceiver).remove(controller); } } else if (safe && origin instanceof MethodCallExpression) { - // wrap call in an IFNULL check - MethodVisitor mv = controller.getMethodVisitor(); CompileStack compileStack = controller.getCompileStack(); OperandStack operandStack = controller.getOperandStack(); + MethodVisitor mv = controller.getMethodVisitor(); int counter = labelCounter.incrementAndGet(); - // if (receiver != null) - ExpressionAsVariableSlot slot = new ExpressionAsVariableSlot(controller, receiver); + // (receiver != null) ? receiver.name(args) : null + Label ifnull = compileStack.createLocalLabel("ifnull_" + counter); + Label nonull = compileStack.createLocalLabel("nonull_" + counter); + Label theEnd = compileStack.createLocalLabel("ending_" + counter); + var slot = new ExpressionAsVariableSlot(controller, receiver); slot.visit(controller.getAcg()); operandStack.box(); - Label ifnull = compileStack.createLocalLabel("ifnull_" + counter); mv.visitJumpInsn(IFNULL, ifnull); - operandStack.remove(1); // receiver consumed by if() - Label nonull = compileStack.createLocalLabel("nonull_" + counter); + operandStack.remove(1); // receiver consumed mv.visitLabel(nonull); - MethodCallExpression origMCE = (MethodCallExpression) origin; - MethodCallExpression newMCE = callX( - new VariableSlotLoader(slot.getType(), slot.getIndex(), controller.getOperandStack()), - origMCE.getMethodAsString(), - origMCE.getArguments() - ); - MethodNode methodTarget = origMCE.getMethodTarget(); - newMCE.setImplicitThis(origMCE.isImplicitThis()); - newMCE.setMethodTarget(methodTarget); + var newMCE = (MethodCallExpression) origin.transformExpression((expression) -> expression); + newMCE.setObjectExpression(new VariableSlotLoader(slot.getType(), slot.getIndex(), operandStack)); + newMCE.getObjectExpression().setSourcePosition(((MethodCallExpression) origin).getObjectExpression()); newMCE.setSafe(false); - newMCE.setSourcePosition(origMCE); - newMCE.getObjectExpression().setSourcePosition(origMCE.getObjectExpression()); + int osl = operandStack.getStackLength(); newMCE.visit(controller.getAcg()); compileStack.removeVar(slot.getIndex()); - ClassNode returnType = operandStack.getTopOperand(); - if (ClassHelper.isPrimitiveType(returnType) && !isPrimitiveVoid(returnType)) { - operandStack.box(); + if (operandStack.getStackLength() > osl) { + operandStack.box(); // non-void method + mv.visitJumpInsn(GOTO, theEnd); + mv.visitLabel(ifnull); + mv.visitInsn(ACONST_NULL); + mv.visitLabel(theEnd); + } else { + mv.visitLabel(ifnull); } - Label endof = compileStack.createLocalLabel("endof_" + counter); - mv.visitJumpInsn(GOTO, endof); - mv.visitLabel(ifnull); - // else { null } - mv.visitInsn(ACONST_NULL); - mv.visitLabel(endof); } else { if (origin instanceof AttributeExpression && (adapter == AsmClassGenerator.getField || adapter == AsmClassGenerator.getGroovyObjectField)) { CallSiteWriter callSiteWriter = controller.getCallSiteWriter(); diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticPropertyAccessHelper.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticPropertyAccessHelper.java index 7a4182edbf4..6cffa34b552 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticPropertyAccessHelper.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticPropertyAccessHelper.java @@ -110,15 +110,19 @@ public Expression transformExpression(final ExpressionTransformer transformer) { call.setSafe(isSafe()); call.setSpreadSafe(isSpreadSafe()); call.setImplicitThis(isImplicitThis()); + call.setGenericsTypes(getGenericsTypes()); return call; } @Override public void visit(final GroovyCodeVisitor visitor) { - super.visit(visitor); if (visitor instanceof AsmClassGenerator) { - // ignore the return of the call - ((AsmClassGenerator) visitor).getController().getOperandStack().pop(); + var os = ((AsmClassGenerator) visitor).getController().getOperandStack(); + int sl = os.getStackLength(); + super.visit(visitor); + os.popDownTo(sl); + } else { + super.visit(visitor); } } } diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy index 4ba4aa7c454..9de6415e14c 100644 --- a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy +++ b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy @@ -315,6 +315,39 @@ final class StaticCompilationTest extends AbstractBytecodeTestCase { ]) } + // GROOVY-11286 + void testVoidMethod3() { + assert compile(method: 'm', '''@groovy.transform.CompileStatic + void m() { + System.out?.print("") + } + ''').hasStrictSequence([ + 'L0', + 'LINENUMBER 3', + 'GETSTATIC java/lang/System.out : Ljava/io/PrintStream;', + 'DUP', + 'ASTORE 1', + 'IFNULL L1', + 'ALOAD 1', + 'LDC ""', + 'INVOKEVIRTUAL java/io/PrintStream.print (Ljava/lang/String;)V', + /* replaced all this with 'L1' + 'ACONST_NULL', + 'GOTO L2', + 'L1', + 'FRAME APPEND [java/io/PrintStream]', + 'ACONST_NULL', + 'L2', + 'FRAME SAME', + 'POP', + */ + 'L1', + 'LINENUMBER 4', + 'FRAME APPEND [java/io/PrintStream]', + 'RETURN' + ]) + } + void testIntLeftShift() { assert compile(method: 'm', '''@groovy.transform.CompileStatic void m() {