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 2020/07/05 01:53:29 UTC
[groovy] 01/01: GROOVY-7304: handle private field access from
closure for ++x and x++
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-7304
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 0204d3c7f29cfe51468fab6d5451e6611a6b3325
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Jul 4 20:49:41 2020 -0500
GROOVY-7304: handle private field access from closure for ++x and x++
---
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 42 ++++++++++++++++------
.../transform/sc/StaticCompilationVisitor.java | 9 ++---
.../groovy/classgen/asm/sc/bugs/Groovy7276.groovy | 4 +--
3 files changed, 38 insertions(+), 17 deletions(-)
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 9c4b041..ce0fb66 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
@@ -41,6 +41,7 @@ import org.codehaus.groovy.classgen.asm.CompileStack;
import org.codehaus.groovy.classgen.asm.MethodCallerMultiAdapter;
import org.codehaus.groovy.classgen.asm.OperandStack;
import org.codehaus.groovy.classgen.asm.TypeChooser;
+import org.codehaus.groovy.classgen.asm.VariableSlotLoader;
import org.codehaus.groovy.runtime.InvokerHelper;
import org.codehaus.groovy.syntax.SyntaxException;
import org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys;
@@ -831,14 +832,35 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
@Override
public void fallbackAttributeOrPropertySite(final PropertyExpression expression, final Expression objectExpression, final String name, final MethodCallerMultiAdapter adapter) {
- if (name != null && (adapter == AsmClassGenerator.setField || adapter == AsmClassGenerator.setGroovyObjectField)) {
- TypeChooser typeChooser = controller.getTypeChooser();
+ if (name != null && controller.getCompileStack().isLHS()) {
ClassNode classNode = controller.getClassNode();
- ClassNode rType = typeChooser.resolveType(objectExpression, classNode);
- if (controller.getCompileStack().isLHS()) {
- if (setField(expression, objectExpression, rType, name)) return;
- } else {
- if (getField(expression, objectExpression, rType, name)) return;
+ ClassNode receiverType = controller.getTypeChooser().resolveType(objectExpression, classNode);
+ if (adapter == AsmClassGenerator.setField || adapter == AsmClassGenerator.setGroovyObjectField) {
+ if (setField(expression, objectExpression, receiverType, name)) return;
+ } else if (AsmClassGenerator.isThisExpression(objectExpression)) {
+ FieldNode fieldNode = receiverType.getField(name);
+ if (fieldNode != null && fieldNode.isPrivate() && !receiverType.equals(classNode)
+ && StaticInvocationWriter.isPrivateBridgeMethodsCallAllowed(receiverType, classNode)) {
+ Map<String, MethodNode> mutators = receiverType.redirect().getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_FIELDS_MUTATORS);
+ if (mutators != null) {
+ MethodNode methodNode = mutators.get(name);
+ if (methodNode != null) {
+ ClassNode rhsType = controller.getOperandStack().getTopOperand();
+ int i = controller.getCompileStack().defineTemporaryVariable("$rhsValue", rhsType, true);
+ VariableSlotLoader rhsValue = new VariableSlotLoader(rhsType, i, controller.getOperandStack());
+
+ MethodCallExpression call = callX(objectExpression, methodNode.getName(), args(fieldNode.isStatic() ? nullX() : objectExpression, rhsValue));
+ call.setImplicitThis(expression.isImplicitThis());
+ call.setSpreadSafe(expression.isSpreadSafe());
+ call.setSafe(expression.isSafe());
+ call.setMethodTarget(methodNode);
+ call.visit(controller.getAcg());
+
+ controller.getCompileStack().removeVar(i);
+ return;
+ }
+ }
+ }
}
}
super.fallbackAttributeOrPropertySite(expression, objectExpression, name, adapter);
@@ -869,12 +891,10 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
mv.visitFieldInsn(PUTSTATIC, ownerName, name, BytecodeHelper.getTypeDescription(fn.getType()));
}
- //mv.visitInsn(ACONST_NULL);
- //stack.replace(OBJECT_TYPE);
return true;
}
- private boolean getField(final PropertyExpression expression, final Expression receiver, ClassNode receiverType, final String name) {
+ /*private boolean getField(final PropertyExpression expression, final Expression receiver, ClassNode receiverType, final String name) {
boolean safe = expression.isSafe();
boolean implicitThis = expression.isImplicitThis();
@@ -897,5 +917,5 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
return true;
}
return false;
- }
+ }*/
}
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
index 79fe33a..baa8a79 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
@@ -74,10 +74,12 @@ import static org.codehaus.groovy.ast.ClassHelper.STRING_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.int_TYPE;
import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
import static org.codehaus.groovy.ast.tools.GeneralUtils.assignS;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.attrX;
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.ctorThisS;
-import static org.codehaus.groovy.ast.tools.GeneralUtils.propX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
import static org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics;
import static org.codehaus.groovy.ast.tools.GenericsUtils.applyGenericsContextToPlaceHolders;
@@ -270,18 +272,17 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
acc += 1;
Parameter param = new Parameter(node.getPlainNodeReference(), "$that");
Expression receiver = fieldNode.isStatic() ? classX(node) : varX(param);
- Statement body = new ExpressionStatement(propX(receiver, fieldNode.getName()));
+ Statement body = returnS(attrX(receiver, constX(fieldNode.getName())));
MethodNode accessor = node.addMethod("pfaccess$" + acc, modifiers, fieldNode.getOriginType(), new Parameter[]{param}, ClassNode.EMPTY_ARRAY, body);
privateFieldAccessors.put(fieldNode.getName(), accessor);
}
-
if (generateMutator) {
// increment acc if it hasn't been incremented in the current iteration
if (!generateAccessor) acc += 1;
Parameter param = new Parameter(node.getPlainNodeReference(), "$that");
Expression receiver = fieldNode.isStatic() ? classX(node) : varX(param);
Parameter value = new Parameter(fieldNode.getOriginType(), "$value");
- Statement body = assignS(propX(receiver, fieldNode.getName()), varX(value));
+ Statement body = assignS(attrX(receiver, constX(fieldNode.getName())), varX(value));
MethodNode mutator = node.addMethod("pfaccess$0" + acc, modifiers, fieldNode.getOriginType(), new Parameter[]{param, value}, ClassNode.EMPTY_ARRAY, body);
privateFieldMutators.put(fieldNode.getName(), mutator);
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7276.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7276.groovy
index a42177c..0cff79c 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7276.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7276.groovy
@@ -37,7 +37,7 @@ final class Groovy7276 extends StaticTypeCheckingTestCase implements StaticCompi
}
void testShouldGoThroughPrivateBridgeMethod2() {
- ['i'/*, 'i++'*/].each { // GROOVY-7304
+ ['i', 'i++'].each { // GROOVY-7304
assertScript """
class Foo {
private int i = 1
@@ -62,7 +62,7 @@ final class Groovy7276 extends StaticTypeCheckingTestCase implements StaticCompi
}
}
- @NotYetImplemented // GROOVY-7304
+ // GROOVY-7304
void testShouldGoThroughPrivateBridgeMethod4() {
['++i', 'i+=1', 'i=i+1'].each {
assertScript """