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 2024/10/14 15:55:07 UTC
(groovy) branch master updated: GROOVY-11286: optimize void method return in expression statement (safe)
This is an automated email from the ASF dual-hosted git repository.
emilles 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 338929cefc GROOVY-11286: optimize void method return in expression statement (safe)
338929cefc is described below
commit 338929cefcf6c3d95accdf1494c53eb6be40e63f
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Oct 14 10:04:43 2024 -0500
GROOVY-11286: optimize void method return in expression statement (safe)
---
.../classgen/asm/sc/StaticInvocationWriter.java | 46 +++++++++-------------
.../asm/sc/StaticPropertyAccessHelper.java | 10 +++--
.../classgen/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 71c6cbe6ac..543aebb304 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 class StaticInvocationWriter extends InvocationWriter {
((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 7a4182edbf..6cffa34b55 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 abstract class StaticPropertyAccessHelper {
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 4ba4aa7c45..9de6415e14 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() {