This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-11360
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 31eb3d942cde7d5425b9ef86fd3afe1f849ca5bb
Author: Eric Milles <eric.mil...@thomsonreuters.com>
AuthorDate: Mon Apr 21 10:37:34 2025 -0500
GROOVY-11630: optimize void groovy method return in expression statement
---
.../codehaus/groovy/classgen/AsmClassGenerator.java | 8 ++++++--
.../groovy/classgen/asm/BinaryExpressionHelper.java | 4 ++--
.../asm/BinaryExpressionMultiTypeDispatcher.java | 2 +-
.../groovy/classgen/asm/InvocationWriter.java | 2 +-
.../codehaus/groovy/classgen/asm/StatementWriter.java | 8 +++-----
.../classgen/asm/sc/StaticInvocationWriter.java | 5 ++++-
...taticTypesBinaryExpressionMultiTypeDispatcher.java | 3 ++-
.../classgen/asm/sc/StaticCompilationTest.groovy | 19 +++++++++++++++++++
8 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 10564ecfcc..3dfb93b596 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/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;
diff --git
a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
index 341938b2fd..041f257d37 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionHelper.java
@@ -376,7 +376,7 @@ public class BinaryExpressionHelper {
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 class BinaryExpressionHelper {
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());
diff --git
a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionMultiTypeDispatcher.java
b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionMultiTypeDispatcher.java
index e57392abe5..f7a9c3906f 100644
---
a/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionMultiTypeDispatcher.java
+++
b/src/main/java/org/codehaus/groovy/classgen/asm/BinaryExpressionMultiTypeDispatcher.java
@@ -380,7 +380,7 @@ public class BinaryExpressionMultiTypeDispatcher extends
BinaryExpressionHelper
// 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);
diff --git
a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
index 32d1981085..8e576c4e2e 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
@@ -238,7 +238,7 @@ public class InvocationWriter {
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;
diff --git
a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
index eb9f08000a..0749aad049 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java
@@ -48,6 +48,7 @@ import org.codehaus.groovy.ast.stmt.SynchronizedStatement;
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 class StatementWriter {
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();
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 7d0929ada6..68cbee26d4 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
@@ -288,6 +288,9 @@ public class StaticInvocationWriter extends
InvocationWriter {
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 class StaticInvocationWriter extends
InvocationWriter {
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());
diff --git
a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java
b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java
index eadb847bbe..0bf878ed4f 100644
---
a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java
+++
b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesBinaryExpressionMultiTypeDispatcher.java
@@ -64,6 +64,7 @@ import static
org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
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 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher
extends BinaryExpres
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
}
}
diff --git
a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy
b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy
index 453bd0490d..9ce34e2c50 100644
---
a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy
+++
b/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() {
