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 9faaa88535 GROOVY-11483: SC: elide spurious `ACONST_NULL` and `POP`
after setter
9faaa88535 is described below
commit 9faaa88535d5f867bd4ae76bd4cffaa065819d74
Author: Eric Milles <[email protected]>
AuthorDate: Sun Jan 18 13:50:45 2026 -0600
GROOVY-11483: SC: elide spurious `ACONST_NULL` and `POP` after setter
---
.../asm/sc/StaticPropertyAccessHelper.java | 33 ++++++++++++----------
...icTypesBinaryExpressionMultiTypeDispatcher.java | 10 ++++---
.../transformers/ConstructorCallTransformer.java | 12 ++++----
.../asm/sc/StaticCompileConstructorsTest.groovy | 17 +++++++++--
4 files changed, 46 insertions(+), 26 deletions(-)
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 6cffa34b55..b655ce705d 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
@@ -45,27 +45,30 @@ public abstract class StaticPropertyAccessHelper {
final boolean spreadSafe,
final boolean returnValue,
final Expression sourceExpression) {
+ MethodCallExpression call;
+ Expression result;
if (returnValue) {
- TemporaryVariableExpression tmp = new
TemporaryVariableExpression(valueExpression);
- PoppingMethodCallExpression call = new
PoppingMethodCallExpression(receiver, setterMethod, tmp);
- call.setSafe(safe);
- call.setSpreadSafe(spreadSafe);
- call.setImplicitThis(implicitThis);
- call.setSourcePosition(sourceExpression);
- PoppingListOfExpressionsExpression list = new
PoppingListOfExpressionsExpression(tmp, call);
- list.setSourcePosition(sourceExpression);
- return list;
+ var temp = new TemporaryVariableExpression(valueExpression);
+ var pmce = new PoppingMethodCallExpression(receiver, setterMethod,
temp);
+ result = new PoppingListOfExpressionsExpression(temp, pmce);
+ result.setSourcePosition(sourceExpression);
+ call = pmce;
} else {
- MethodCallExpression call = new MethodCallExpression(receiver,
setterMethod.getName(), valueExpression);
- call.setSafe(safe);
- call.setSpreadSafe(spreadSafe);
- call.setImplicitThis(implicitThis);
+ call = new MethodCallExpression(receiver, setterMethod.getName(),
valueExpression);
call.setMethodTarget(setterMethod);
- call.setSourcePosition(sourceExpression);
- return call;
+ result = call;
}
+ call.setSafe(safe);
+ call.setSpreadSafe(spreadSafe);
+ call.setImplicitThis(implicitThis);
+ call.setNodeMetaData(AsmClassGenerator.ELIDE_EXPRESSION_VALUE,
Boolean.TRUE); // GROOVY-11843
+ call.setSourcePosition(sourceExpression);
+
+ return result;
}
+
//--------------------------------------------------------------------------
+
private static class PoppingListOfExpressionsExpression extends
ListOfExpressionsExpression {
private final TemporaryVariableExpression tmp;
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 e139b7331b..c8508b02bf 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
@@ -163,7 +163,8 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher
extends BinaryExpres
expressionWithoutAssignment,
pexp.isSafe(),
pexp.isSpreadSafe(),
- pexp.isImplicitThis())) {
+ pexp.isImplicitThis(),
+ true)) { // TODO: GROOVY-11843
return;
}
}
@@ -181,7 +182,8 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher
extends BinaryExpres
expression.getRightExpression(),
pexp.isSafe(),
pexp.isSpreadSafe(),
- pexp.isImplicitThis())) {
+ pexp.isImplicitThis(),
+
!Boolean.TRUE.equals(expression.getNodeMetaData(ELIDE_EXPRESSION_VALUE)))) { //
GROOVY-11843
return;
}
// GROOVY-5620: spread-safe operator on LHS is not supported
@@ -241,7 +243,7 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher
extends BinaryExpres
result.visit(controller.getAcg());
}
- private boolean makeSetProperty(final Expression receiver, final
Expression message, final Expression arguments, final boolean safe, final
boolean spreadSafe, final boolean implicitThis) {
+ private boolean makeSetProperty(final Expression receiver, final
Expression message, final Expression arguments, final boolean safe, final
boolean spreadSafe, final boolean implicitThis, final boolean returnValue) {
var receiverType = controller.getTypeChooser().resolveType(receiver,
controller.getClassNode());
var thisReceiver = isThisExpression(receiver);
var propertyName = message.getText();
@@ -282,7 +284,7 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher
extends BinaryExpres
implicitThis,
safe,
spreadSafe,
- true, // to be replaced with a proper test whether a
return value should be used or not
+ returnValue,
message
);
call.visit(controller.getAcg());
diff --git
a/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java
b/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java
index 7190f94147..af0160e54a 100644
---
a/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java
+++
b/src/main/java/org/codehaus/groovy/transform/sc/transformers/ConstructorCallTransformer.java
@@ -34,7 +34,6 @@ import org.codehaus.groovy.classgen.BytecodeExpression;
import org.codehaus.groovy.classgen.asm.BytecodeHelper;
import org.codehaus.groovy.classgen.asm.CompileStack;
import org.codehaus.groovy.classgen.asm.OperandStack;
-import org.codehaus.groovy.classgen.asm.WriterController;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor;
import org.objectweb.asm.MethodVisitor;
@@ -130,9 +129,8 @@ public class ConstructorCallTransformer {
@Override
public void visit(final MethodVisitor mv) {
- WriterController controller = acg.getController();
- CompileStack compileStack = controller.getCompileStack();
- OperandStack operandStack = controller.getOperandStack();
+ CompileStack compileStack = acg.getController().getCompileStack();
+ OperandStack operandStack = acg.getController().getOperandStack();
ClassNode ctorType = getType();
// create temporary variable to store new object instance
@@ -149,6 +147,8 @@ public class ConstructorCallTransformer {
mv.visitMethodInsn(INVOKESPECIAL, ctorTypeName, "<init>",
signature, false);
mv.visitVarInsn(ASTORE, tmpObj);
+ int mark = operandStack.getStackLength();
+
// process property initializers
for (MapEntryExpression entryExpression :
map.getMapEntryExpressions()) {
Expression keyExpression = entryExpression.getKeyExpression();
@@ -163,9 +163,11 @@ public class ConstructorCallTransformer {
Expression setExpression =
staticCompilationTransformer.transform(
assignX(varExpression, valExpression) // tmp.key =
value
);
+
setExpression.putNodeMetaData(AsmClassGenerator.ELIDE_EXPRESSION_VALUE,
Boolean.TRUE);
setExpression.setSourcePosition(entryExpression);
setExpression.visit(acg);
- operandStack.pop();
+
+ operandStack.popDownTo(mark);
}
// result object
diff --git
a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
index c9d8050965..40a2fbd25e 100644
---
a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
+++
b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
@@ -23,7 +23,7 @@ import groovy.transform.stc.ConstructorsSTCTest
/**
* Unit tests for static compilation: constructors.
*/
-class StaticCompileConstructorsTest extends ConstructorsSTCTest implements
StaticCompilationTestSupport {
+final class StaticCompileConstructorsTest extends ConstructorsSTCTest
implements StaticCompilationTestSupport {
void testMapConstructorError() {
assertScript '''
@@ -40,11 +40,24 @@ class StaticCompileConstructorsTest extends
ConstructorsSTCTest implements Stati
class Person {
String name
}
-
C.test()
'''
}
+ // GROOVY-11843
+ void testMapConstructorOptimization() {
+ assertScript '''
+ class C {
+ int i, j, k
+ }
+ new C(i:1, j:2, k:3)
+ '''
+ String script = astTrees.find{ it.key != 'C' }.value[1]
+ Number offset = script.indexOf('run()Ljava/lang/Object;')
+ String method = script.substring(offset, script.indexOf('\n' +
System.lineSeparator(), offset) + 1)
+ assert method.count('ACONST_NULL') == 0
+ }
+
void testPrivateConstructorFromClosure() {
assertScript '''
class C {
