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 2021/11/25 17:44:29 UTC
[groovy] branch master updated: GROOVY-5746, GROOVY-6137, GROOVY-7473: SC: pop temp variable(s)
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 ca1c3b4 GROOVY-5746, GROOVY-6137, GROOVY-7473: SC: pop temp variable(s)
ca1c3b4 is described below
commit ca1c3b481cff88b70268c46475cd67e062143dc3
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Nov 25 11:30:23 2021 -0600
GROOVY-5746, GROOVY-6137, GROOVY-7473: SC: pop temp variable(s)
---
.../groovy/classgen/AsmClassGenerator.java | 154 +++++++++++----------
.../transformers/BinaryExpressionTransformer.java | 17 ++-
.../groovy/transform/stc/STCAssignmentTest.groovy | 6 +-
.../transform/stc/STCnAryExpressionTest.groovy | 9 +-
4 files changed, 100 insertions(+), 86 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 58516a4..c310229 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -131,6 +131,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
+import java.util.function.Consumer;
import static org.apache.groovy.ast.tools.ClassNodeUtils.getField;
import static org.apache.groovy.ast.tools.ExpressionUtils.isThisOrSuper;
@@ -824,19 +825,21 @@ public class AsmClassGenerator extends ClassGenerator {
public void visitTernaryExpression(final TernaryExpression expression) {
onLineNumber(expression, "visitTernaryExpression");
controller.getBinaryExpressionHelper().evaluateTernary(expression);
+ doPostVisit(expression); // GROOVY-7473
}
@Override
public void visitDeclarationExpression(final DeclarationExpression expression) {
- onLineNumber(expression, "visitDeclarationExpression: \"" + expression.getText() + "\"");
- controller.getBinaryExpressionHelper().evaluateEqual(expression,true);
+ onLineNumber(expression, "visitDeclarationExpression: " + expression.getText());
+ controller.getBinaryExpressionHelper().evaluateEqual(expression, true);
}
@Override
public void visitBinaryExpression(final BinaryExpression expression) {
- onLineNumber(expression, "visitBinaryExpression: \"" + expression.getOperation().getText() + "\" ");
+ onLineNumber(expression, "visitBinaryExpression: " + expression.getOperation().getText());
controller.getBinaryExpressionHelper().eval(expression);
controller.getAssertionWriter().record(expression.getOperation());
+ doPostVisit(expression); // GROOVY-5746
}
@Override
@@ -1460,24 +1463,6 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private void loadThis(final VariableExpression thisOrSuper) {
- MethodVisitor mv = controller.getMethodVisitor();
- mv.visitVarInsn(ALOAD, 0);
- OperandStack operandStack = controller.getOperandStack();
- if (controller.isInGeneratedFunction() && !controller.getCompileStack().isImplicitThis()) {
- mv.visitMethodInsn(INVOKEVIRTUAL, "groovy/lang/Closure", "getThisObject", "()Ljava/lang/Object;", false);
- ClassNode expectedType = controller.getTypeChooser().resolveType(thisOrSuper, controller.getOutermostClass());
- if (!isObjectType(expectedType) && !isPrimitiveType(expectedType)) {
- BytecodeHelper.doCast(mv, expectedType);
- operandStack.push(expectedType);
- } else {
- operandStack.push(ClassHelper.OBJECT_TYPE);
- }
- } else {
- operandStack.push(controller.getClassNode());
- }
- }
-
protected void createInterfaceSyntheticStaticFields() {
ClassNode icl = controller.getInterfaceClassLoadingClass();
@@ -1656,31 +1641,6 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- public void despreadList(final List<Expression> expressions, final boolean wrap) {
- List<Expression> spreadIndexes = new ArrayList<>();
- List<Expression> spreadExpressions = new ArrayList<>();
- List<Expression> normalArguments = new ArrayList<>();
- for (int i = 0, n = expressions.size(); i < n; i += 1) {
- Expression expr = expressions.get(i);
- if (!(expr instanceof SpreadExpression)) {
- normalArguments.add(expr);
- } else {
- spreadIndexes.add(new ConstantExpression(i - spreadExpressions.size(), true));
- spreadExpressions.add(((SpreadExpression) expr).getExpression());
- }
- }
-
- // load normal arguments as array
- visitTupleExpression(new ArgumentListExpression(normalArguments), wrap);
- // load spread expressions as array
- new TupleExpression(spreadExpressions).visit(this);
- // load insertion index
- new ArrayExpression(ClassHelper.int_TYPE, spreadIndexes, null).visit(this);
-
- controller.getOperandStack().remove(1);
- despreadList.call(controller.getMethodVisitor());
- }
-
@Override
public void visitTupleExpression(final TupleExpression expression) {
visitTupleExpression(expression, false);
@@ -1707,18 +1667,6 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- public void loadWrapper(final Expression argument) {
- MethodVisitor mv = controller.getMethodVisitor();
- ClassNode goalClass = argument.getType();
- visitClassExpression(new ClassExpression(goalClass));
- if (goalClass.isDerivedFromGroovyObject()) {
- createGroovyObjectWrapperMethod.call(mv);
- } else {
- createPojoWrapperMethod.call(mv);
- }
- controller.getOperandStack().remove(1);
- }
-
@Override
public void visitArrayExpression(final ArrayExpression expression) {
MethodVisitor mv = controller.getMethodVisitor();
@@ -2279,21 +2227,6 @@ public class AsmClassGenerator extends ClassGenerator {
}
}
- private static int determineCommonArrayType(final List<Expression> values) {
- Expression expr = values.get(0);
- int arrayElementType = -1;
- if (expr instanceof AnnotationConstantExpression) {
- arrayElementType = 1;
- } else if (expr instanceof ConstantExpression) {
- arrayElementType = 2;
- } else if (expr instanceof ClassExpression || expr instanceof ClosureExpression) {
- arrayElementType = 3;
- } else if (expr instanceof PropertyExpression) {
- arrayElementType = 4;
- }
- return arrayElementType;
- }
-
private void visitAnnotationArrayElement(final Expression expr, final int arrayElementType, final AnnotationVisitor av) {
switch (arrayElementType) {
case 1:
@@ -2370,6 +2303,51 @@ public class AsmClassGenerator extends ClassGenerator {
return false;
}
+ public void despreadList(final List<Expression> expressions, final boolean wrap) {
+ List<Expression> spreadIndexes = new ArrayList<>();
+ List<Expression> spreadExpressions = new ArrayList<>();
+ List<Expression> normalArguments = new ArrayList<>();
+ for (int i = 0, n = expressions.size(); i < n; i += 1) {
+ Expression expr = expressions.get(i);
+ if (!(expr instanceof SpreadExpression)) {
+ normalArguments.add(expr);
+ } else {
+ spreadIndexes.add(new ConstantExpression(i - spreadExpressions.size(), true));
+ spreadExpressions.add(((SpreadExpression) expr).getExpression());
+ }
+ }
+
+ // load normal arguments as array
+ visitTupleExpression(new ArgumentListExpression(normalArguments), wrap);
+ // load spread expressions as array
+ new TupleExpression(spreadExpressions).visit(this);
+ // load insertion index
+ new ArrayExpression(ClassHelper.int_TYPE, spreadIndexes, null).visit(this);
+
+ controller.getOperandStack().remove(1);
+ despreadList.call(controller.getMethodVisitor());
+ }
+
+ private static int determineCommonArrayType(final List<Expression> values) {
+ Expression expr = values.get(0);
+ int arrayElementType = -1;
+ if (expr instanceof AnnotationConstantExpression) {
+ arrayElementType = 1;
+ } else if (expr instanceof ConstantExpression) {
+ arrayElementType = 2;
+ } else if (expr instanceof ClassExpression || expr instanceof ClosureExpression) {
+ arrayElementType = 3;
+ } else if (expr instanceof PropertyExpression) {
+ arrayElementType = 4;
+ }
+ return arrayElementType;
+ }
+
+ private void doPostVisit(final ASTNode node) {
+ Consumer<WriterController> callback = node.getNodeMetaData("classgen.callback");
+ if (callback != null) callback.accept(controller);
+ }
+
@Deprecated
public static boolean isThisExpression(final Expression expression) {
return ExpressionUtils.isThisExpression(expression);
@@ -2389,6 +2367,36 @@ public class AsmClassGenerator extends ClassGenerator {
return (parameters.length > 0 && parameters[parameters.length - 1].getType().isArray());
}
+ private void loadThis(final VariableExpression thisOrSuper) {
+ MethodVisitor mv = controller.getMethodVisitor();
+ mv.visitVarInsn(ALOAD, 0);
+ OperandStack operandStack = controller.getOperandStack();
+ if (controller.isInGeneratedFunction() && !controller.getCompileStack().isImplicitThis()) {
+ mv.visitMethodInsn(INVOKEVIRTUAL, "groovy/lang/Closure", "getThisObject", "()Ljava/lang/Object;", false);
+ ClassNode expectedType = controller.getTypeChooser().resolveType(thisOrSuper, controller.getOutermostClass());
+ if (!isObjectType(expectedType) && !isPrimitiveType(expectedType)) {
+ BytecodeHelper.doCast(mv, expectedType);
+ operandStack.push(expectedType);
+ } else {
+ operandStack.push(ClassHelper.OBJECT_TYPE);
+ }
+ } else {
+ operandStack.push(controller.getClassNode());
+ }
+ }
+
+ public void loadWrapper(final Expression argument) {
+ MethodVisitor mv = controller.getMethodVisitor();
+ ClassNode goalClass = argument.getType();
+ visitClassExpression(new ClassExpression(goalClass));
+ if (goalClass.isDerivedFromGroovyObject()) {
+ createGroovyObjectWrapperMethod.call(mv);
+ } else {
+ createPojoWrapperMethod.call(mv);
+ }
+ controller.getOperandStack().remove(1);
+ }
+
public void onLineNumber(final ASTNode statement, final String message) {
if (statement == null || statement instanceof BlockStatement) return;
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
index b31d5fc..f1a6de5 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
@@ -36,6 +36,7 @@ import org.codehaus.groovy.ast.expr.RangeExpression;
import org.codehaus.groovy.ast.expr.TupleExpression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.ast.tools.WideningCategories;
+import org.codehaus.groovy.classgen.asm.WriterController;
import org.codehaus.groovy.classgen.asm.sc.StaticPropertyAccessHelper;
import org.codehaus.groovy.runtime.DefaultGroovyMethods;
import org.codehaus.groovy.syntax.Token;
@@ -52,6 +53,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
+import java.util.function.Consumer;
import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
@@ -243,7 +245,9 @@ public class BinaryExpressionTransformer {
// GROOVY-6137, GROOVY-7473: null safety and one-time evaluation
call.setObjectExpression(rightExpression = transformRepeatedReference(rightExpression));
- return staticCompilationTransformer.transform(ternaryX(isNullX(rightExpression), isNullX(leftExpression), call));
+ Expression safe = ternaryX(isNullX( rightExpression ), isNullX( leftExpression ), call);
+ safe.putNodeMetaData("classgen.callback", classgenCallback(call.getObjectExpression()));
+ return staticCompilationTransformer.transform(safe);
}
private Expression transformRepeatedReference(final Expression exp) {
@@ -384,16 +388,19 @@ public class BinaryExpressionTransformer {
}
call.setImplicitThis(false);
if (Types.isAssignment(operationType)) { // +=, -=, /=, ...
+ // call handles the operation, so we must add the assignment now
+ expr = binX(left, Token.newSymbol(Types.ASSIGN, operation.getStartLine(), operation.getStartColumn()), call);
// GROOVY-5746: one execution of receiver and subscript
if (left instanceof BinaryExpression) {
BinaryExpression be = (BinaryExpression) left;
if (be.getOperation().getType() == Types.LEFT_SQUARE_BRACKET) {
be.setLeftExpression(transformRepeatedReference(be.getLeftExpression()));
be.setRightExpression(transformRepeatedReference(be.getRightExpression()));
+ expr.putNodeMetaData("classgen.callback", classgenCallback(be.getRightExpression())
+ .andThen(classgenCallback(be.getLeftExpression()))
+ );
}
}
- // call handles the operation, so we must add the assignment now
- expr = binX(left, Token.newSymbol(Types.ASSIGN, operation.getStartLine(), operation.getStartColumn()), call);
} else {
expr = call;
}
@@ -472,4 +479,8 @@ public class BinaryExpressionTransformer {
}
throw new IllegalArgumentException("Unsupported conversion: " + target.getText());
}
+
+ private static Consumer<WriterController> classgenCallback(final Expression source) {
+ return (source instanceof TemporaryVariableExpression ? ((TemporaryVariableExpression) source)::remove : wc -> {});
+ }
}
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index de133f5..390b3da 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -159,11 +159,9 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
@Field int i = 0
int getIndex() { i++ }
def list = ['x','y','z']
- def x = (list[index] += '!')
- def y = (list[index] += '!')
- assert x == 'x!'
- assert y == 'y!'
+ assert (list[index] += '!') == 'x!'
+ assert (list[index] += '!') == 'y!'
assert list.toString() == '[x!, y!, z]'
'''
}
diff --git a/src/test/groovy/transform/stc/STCnAryExpressionTest.groovy b/src/test/groovy/transform/stc/STCnAryExpressionTest.groovy
index 3da730f..51fe8a4 100644
--- a/src/test/groovy/transform/stc/STCnAryExpressionTest.groovy
+++ b/src/test/groovy/transform/stc/STCnAryExpressionTest.groovy
@@ -142,8 +142,7 @@ class STCnAryExpressionTest extends StaticTypeCheckingTestCase {
int getA() { i++ }
int getB() { j++ }
- def result = a in b
- assert result
+ assert a in b
assert i == 1
assert j == 1
'''
@@ -155,8 +154,7 @@ class STCnAryExpressionTest extends StaticTypeCheckingTestCase {
def getA() { i++; null }
def getB() { j++ }
- def result = a in b
- assert !result
+ assert !(a in b)
assert i == 1
assert j == 1
'''
@@ -168,8 +166,7 @@ class STCnAryExpressionTest extends StaticTypeCheckingTestCase {
def getA() { i++ }
def getB() { j++; null }
- def result = a in b
- assert !result
+ assert !(a in b)
assert i == 1
assert j == 1
'''