You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/07/28 00:04:35 UTC
[groovy] 02/03: GROOVY-7304: handle private field access from
closure for ++x and x++
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 40ee3fd0c19530b4308c7c1b3e30abd4d45ed54a
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Jul 27 12:34:14 2020 -0500
GROOVY-7304: handle private field access from closure for ++x and x++
closes #1299
(cherry picked from commit b5c80ae6e66bb2add1722fb1256be490959a7696)
---
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 42 ++++++++++++++++------
.../transform/sc/StaticCompilationVisitor.java | 14 ++++----
.../groovy/classgen/asm/sc/bugs/Groovy7276.groovy | 4 +--
.../groovy/classgen/asm/sc/bugs/Groovy7304.groovy | 42 ++++++++++++++++++++++
4 files changed, 83 insertions(+), 19 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 880c75f..9fe6321 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;
@@ -832,14 +833,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 (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);
@@ -870,12 +892,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();
@@ -898,5 +918,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..16c721d 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
@@ -23,7 +23,6 @@ import groovy.transform.CompileStatic;
import groovy.transform.TypeChecked;
import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.AnnotatedNode;
-import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
@@ -74,10 +73,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,19 +271,20 @@ 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);
+ accessor.setNodeMetaData(STATIC_COMPILE_NODE, Boolean.TRUE);
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);
+ mutator.setNodeMetaData(STATIC_COMPILE_NODE, Boolean.TRUE);
privateFieldMutators.put(fieldNode.getName(), mutator);
}
}
@@ -375,8 +377,8 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
if (origGenericsTypes != null) {
bridge.setGenericsTypes(applyGenericsContextToPlaceHolders(genericsSpec, origGenericsTypes));
}
+ bridge.setNodeMetaData(STATIC_COMPILE_NODE, Boolean.TRUE);
privateBridgeMethods.put(method, bridge);
- bridge.addAnnotation(new AnnotationNode(COMPILESTATIC_CLASSNODE));
}
}
if (!privateBridgeMethods.isEmpty()) {
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 """
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7304.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7304.groovy
new file mode 100644
index 0000000..13b5222
--- /dev/null
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7304.groovy
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.classgen.asm.sc.bugs
+
+import groovy.transform.CompileStatic
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+@CompileStatic
+final class Groovy7304 {
+
+ @Test // bridge methods should also be statically compiled
+ void testShouldGoThroughPrivateBridgeMethod() {
+ assertScript '''
+ class A {
+ private int i = 1
+ @groovy.transform.CompileStatic
+ int m() { new String().with { i++ } }
+ }
+ class B extends A {
+ }
+ assert new B().m() == 1
+ '''
+ }
+}