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 2022/10/22 22:04:24 UTC
[groovy] branch GROOVY_2_5_X updated: GROOVY-9771: SC: private field access without revisit
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
new 1370e64a6b GROOVY-9771: SC: private field access without revisit
1370e64a6b is described below
commit 1370e64a6bd4b7f4cd425763b9f91147cec9d14f
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Oct 22 14:23:18 2022 -0500
GROOVY-9771: SC: private field access without revisit
---
...icTypesBinaryExpressionMultiTypeDispatcher.java | 51 ++++++++-------------
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 53 +++++++++++-----------
2 files changed, 46 insertions(+), 58 deletions(-)
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 e98d4a72a7..5171d523a0 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
@@ -18,7 +18,6 @@
*/
package org.codehaus.groovy.classgen.asm.sc;
-import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.FieldNode;
import org.codehaus.groovy.ast.MethodNode;
@@ -41,7 +40,6 @@ import org.codehaus.groovy.ast.stmt.ExpressionStatement;
import org.codehaus.groovy.ast.stmt.ForStatement;
import org.codehaus.groovy.ast.tools.WideningCategories;
import org.codehaus.groovy.classgen.asm.BinaryExpressionMultiTypeDispatcher;
-import org.codehaus.groovy.classgen.asm.BinaryExpressionWriter;
import org.codehaus.groovy.classgen.asm.BytecodeHelper;
import org.codehaus.groovy.classgen.asm.CompileStack;
import org.codehaus.groovy.classgen.asm.OperandStack;
@@ -52,7 +50,6 @@ import org.codehaus.groovy.runtime.MetaClassHelper;
import org.codehaus.groovy.syntax.Token;
import org.codehaus.groovy.syntax.TokenUtil;
import org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys;
-import org.codehaus.groovy.transform.sc.StaticCompilationVisitor;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport;
import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;
@@ -66,9 +63,12 @@ import java.util.Objects;
import java.util.concurrent.atomic.AtomicInteger;
import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.VOID_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.char_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.double_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.float_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.isNumberType;
+import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
import static org.codehaus.groovy.ast.ClassHelper.long_TYPE;
import static org.codehaus.groovy.transform.sc.StaticCompilationVisitor.ARRAYLIST_ADD_METHOD;
import static org.codehaus.groovy.transform.sc.StaticCompilationVisitor.ARRAYLIST_CLASSNODE;
@@ -83,7 +83,6 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
private final AtomicInteger labelCounter = new AtomicInteger();
private static final MethodNode CLOSURE_GETTHISOBJECT_METHOD = CLOSURE_TYPE.getMethod("getThisObject", Parameter.EMPTY_ARRAY);
-
public StaticTypesBinaryExpressionMultiTypeDispatcher(WriterController wc) {
super(wc);
}
@@ -106,7 +105,7 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
}
ClassNode top = operandStack.getTopOperand();
- if (ClassHelper.isPrimitiveType(top) && (ClassHelper.isNumberType(top)||char_TYPE.equals(top))) {
+ if (isPrimitiveType(top) && (isNumberType(top) || char_TYPE.equals(top))) {
MethodVisitor mv = controller.getMethodVisitor();
visitInsnByType(top, mv, ICONST_1, LCONST_1, FCONST_1, DCONST_1);
if ("next".equals(method)) {
@@ -312,7 +311,7 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
setterMethod = new MethodNode(
setter,
ACC_PUBLIC,
- ClassHelper.VOID_TYPE,
+ VOID_TYPE,
new Parameter[]{new Parameter(propertyNode.getOriginType(), "value")},
ClassNode.EMPTY_ARRAY,
EmptyStatement.INSTANCE
@@ -392,41 +391,31 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
return false;
}
- protected void assignToArray(final Expression parent, final Expression receiver, final Expression subscript, final Expression rhsValueLoader) {
+ @Override
+ protected void assignToArray(final Expression origin, final Expression receiver, final Expression subscript, final Expression rhsValueLoader) {
WriterController controller = getController();
- ClassNode current = controller.getClassNode();
- ClassNode arrayType = controller.getTypeChooser().resolveType(receiver, current);
+ ClassNode currentClass = controller.getClassNode();
+ ClassNode receiverType = controller.getTypeChooser().resolveType(receiver, currentClass);
- ClassNode arrayComponentType = arrayType.getComponentType();
- int operationType = getOperandType(arrayComponentType);
- BinaryExpressionWriter bew = binExpWriter[operationType];
+ if (receiverType.isArray() && binExpWriter[getOperandType(receiverType.getComponentType())].arraySet(true)) {
+ super.assignToArray(origin, receiver, subscript, rhsValueLoader);
- if (bew.arraySet(true) && arrayType.isArray()) {
- super.assignToArray(parent, receiver, subscript, rhsValueLoader);
- } else {
- /*
- * This code path is needed because ACG creates array access expressions
- */
-
- // GROOVY-6061
- if (rhsValueLoader instanceof VariableSlotLoader && parent instanceof BinaryExpression) {
- rhsValueLoader.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE,
- controller.getTypeChooser().resolveType(parent, current));
+ } else { // this code path is needed because ACG creates array access expressions
+ if (rhsValueLoader instanceof VariableSlotLoader && origin instanceof BinaryExpression) { // GROOVY-6061
+ rhsValueLoader.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, controller.getTypeChooser().resolveType(origin, currentClass));
}
- // GROOVY-9771
- receiver.visit(new StaticCompilationVisitor(controller.getSourceUnit(), current));
// replace assignment to a subscript operator with a method call
// e.g. x[5] = 10 -> methodCall(x, "putAt", [5, 10])
- ArgumentListExpression ale = new ArgumentListExpression(subscript, rhsValueLoader);
- MethodCallExpression mce = new MethodCallExpression(receiver, "putAt", ale);
- mce.setSourcePosition(parent);
+ ArgumentListExpression args = new ArgumentListExpression(subscript, rhsValueLoader);
+ MethodCallExpression call = new MethodCallExpression(receiver, "putAt", args);
+ call.setSourcePosition(origin);
OperandStack operandStack = controller.getOperandStack();
- int height = operandStack.getStackLength();
- mce.visit(controller.getAcg());
+ int len = operandStack.getStackLength();
+ call.visit(controller.getAcg());
operandStack.pop();
- operandStack.remove(operandStack.getStackLength() - height);
+ operandStack.remove(operandStack.getStackLength() - len);
// return value of assignment
rhsValueLoader.visit(controller.getAcg());
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
index 082cb106b3..89ea087fb6 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -22,7 +22,6 @@ import org.apache.groovy.ast.tools.ClassNodeUtils;
import org.apache.groovy.ast.tools.ExpressionUtils;
import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.ast.ASTNode;
-import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.FieldNode;
import org.codehaus.groovy.ast.InnerClassNode;
@@ -58,7 +57,6 @@ import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
-import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -86,7 +84,9 @@ import static org.codehaus.groovy.ast.ClassHelper.int_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveType;
import static org.codehaus.groovy.ast.ClassHelper.make;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callThisX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
@@ -378,39 +378,38 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
if (field == null && implicitThis && outerClass != null && !receiverType.isStaticClass()) {
Expression pexp;
if (controller.isInClosure()) {
- MethodCallExpression mce = new MethodCallExpression(
- new VariableExpression("this"),
- "getThisObject",
- ArgumentListExpression.EMPTY_ARGUMENTS
- );
- mce.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, controller.getOutermostClass());
- mce.setImplicitThis(true);
- mce.setMethodTarget(CLOSURE_GETTHISOBJECT_METHOD);
- pexp = new CastExpression(controller.getOutermostClass(),mce);
+ MethodCallExpression call = callThisX("getThisObject");
+ call.setImplicitThis(true);
+ call.setMethodTarget(CLOSURE_GETTHISOBJECT_METHOD);
+ call.setNodeMetaData(StaticTypesMarker.INFERRED_TYPE, controller.getOutermostClass());
+ pexp = new CastExpression(controller.getOutermostClass(), call);
} else {
- pexp = new PropertyExpression(
- new ClassExpression(outerClass),
- "this"
- );
- ((PropertyExpression)pexp).setImplicitThis(true);
+ pexp = new PropertyExpression(classX(outerClass), "this");
}
pexp.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, outerClass);
pexp.setSourcePosition(receiver);
return makeGetPrivateFieldWithBridgeMethod(pexp, outerClass, fieldName, safe, true);
}
ClassNode classNode = controller.getClassNode();
- if (field != null && Modifier.isPrivate(field.getModifiers()) && !receiverType.equals(classNode)
- && (StaticInvocationWriter.isPrivateBridgeMethodsCallAllowed(receiverType, classNode) || StaticInvocationWriter.isPrivateBridgeMethodsCallAllowed(classNode,receiverType))) {
+ if (field != null && field.isPrivate() && !receiverType.equals(classNode)
+ && (StaticInvocationWriter.isPrivateBridgeMethodsCallAllowed(receiverType, classNode)
+ || StaticInvocationWriter.isPrivateBridgeMethodsCallAllowed(classNode, receiverType))) {
Map<String, MethodNode> accessors = receiverType.redirect().getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_FIELDS_ACCESSORS);
- if (accessors!=null) {
+ if (accessors != null) {
MethodNode methodNode = accessors.get(fieldName);
- if (methodNode!=null) {
- MethodCallExpression mce = new MethodCallExpression(receiver, methodNode.getName(),
- new ArgumentListExpression(field.isStatic() ? new ConstantExpression(null) : receiver));
- mce.setMethodTarget(methodNode);
- mce.setSafe(safe);
- mce.setImplicitThis(implicitThis);
- mce.visit(controller.getAcg());
+ if (methodNode != null) {
+ Expression thisObject;
+ if (field.isStatic()) {
+ thisObject = nullX();
+ } else if (!ExpressionUtils.isThisExpression(receiver)) {
+ thisObject = receiver;
+ } else { // GROOVY-7304, GROOVY-9771, GROOVY-9872, et al.
+ thisObject = new PropertyExpression(classX(receiverType), "this");
+ }
+
+ MethodCallExpression methodCall = callX(classX(receiverType), methodNode.getName(), thisObject);
+ methodCall.setMethodTarget(methodNode);
+ methodCall.visit(controller.getAcg());
return true;
}
}
@@ -560,7 +559,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
}
mv.visitFieldInsn(GETFIELD, BytecodeHelper.getClassInternalName(field.getOwner()), fieldName, BytecodeHelper.getTypeDescription(replacementType));
if (safe) {
- if (ClassHelper.isPrimitiveType(replacementType)) {
+ if (isPrimitiveType(replacementType)) {
operandStack.replace(replacementType);
operandStack.box();
replacementType = operandStack.getTopOperand();