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/12/07 14:43:44 UTC
[groovy] branch GROOVY_2_5_X updated: GROOVY-9771: visit receiver
for private field access
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 8cb19a3 GROOVY-9771: visit receiver for private field access
8cb19a3 is described below
commit 8cb19a362cbd6251ffd713a522731afa7849e964
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Dec 7 08:34:39 2020 -0600
GROOVY-9771: visit receiver for private field access
---
...icTypesBinaryExpressionMultiTypeDispatcher.java | 42 +++++++++++-----------
.../transform/stc/StaticTypeCheckingSupport.java | 23 ++++++------
.../classgen/asm/sc/bugs/Groovy7361Bug.groovy | 16 +++++++++
3 files changed, 48 insertions(+), 33 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 8ab997b..7cbb3e6 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
@@ -51,6 +51,7 @@ import org.codehaus.groovy.classgen.asm.WriterController;
import org.codehaus.groovy.runtime.MetaClassHelper;
import org.codehaus.groovy.syntax.Token;
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;
@@ -145,7 +146,7 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
pexp instanceof AttributeExpression)) return;
}
}
- // GROOVY-5620: Spread safe/Null safe operator on LHS is not supported
+ // GROOVY-5620: spread-safe operator on LHS is not supported
if (expression.getLeftExpression() instanceof PropertyExpression
&& ((PropertyExpression) expression.getLeftExpression()).isSpreadSafe()
&& StaticTypeCheckingSupport.isAssignment(expression.getOperation().getType())) {
@@ -309,7 +310,6 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
return false;
}
- @SuppressWarnings("unchecked")
private boolean makeSetPrivateFieldWithBridgeMethod(final Expression receiver, final ClassNode receiverType, final String fieldName, final Expression arguments, final boolean safe, final boolean spreadSafe, final boolean implicitThis) {
WriterController controller = getController();
FieldNode field = receiverType.getField(fieldName);
@@ -359,44 +359,44 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
return false;
}
- protected void assignToArray(Expression parent, Expression receiver, Expression index, Expression rhsValueLoader) {
- ClassNode current = getController().getClassNode();
- ClassNode arrayType = getController().getTypeChooser().resolveType(receiver, current);
+ protected void assignToArray(final Expression parent, final Expression receiver, final Expression subscript, final Expression rhsValueLoader) {
+ WriterController controller = getController();
+ ClassNode current = controller.getClassNode();
+ ClassNode arrayType = controller.getTypeChooser().resolveType(receiver, current);
+
ClassNode arrayComponentType = arrayType.getComponentType();
int operationType = getOperandType(arrayComponentType);
BinaryExpressionWriter bew = binExpWriter[operationType];
if (bew.arraySet(true) && arrayType.isArray()) {
- super.assignToArray(parent, receiver, index, rhsValueLoader);
+ super.assignToArray(parent, receiver, subscript, rhsValueLoader);
} else {
- /******
- / This code path is needed because ACG creates array access expressions
- *******/
+ /*
+ * This code path is needed because ACG creates array access expressions
+ */
- WriterController controller = getController();
- // let's replace this assignment to a subscript operator with a
- // method call
- // e.g. x[5] = 10
- // -> (x, [], 5), =, 10
- // -> methodCall(x, "putAt", [5, 10])
- ArgumentListExpression ae = new ArgumentListExpression(index, rhsValueLoader);
+ // GROOVY-6061
if (rhsValueLoader instanceof VariableSlotLoader && parent instanceof BinaryExpression) {
- // GROOVY-6061
rhsValueLoader.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE,
- controller.getTypeChooser().resolveType(parent, controller.getClassNode()));
+ controller.getTypeChooser().resolveType(parent, current));
}
- MethodCallExpression mce = new MethodCallExpression(receiver, "putAt", ae);
+ // 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);
OperandStack operandStack = controller.getOperandStack();
int height = operandStack.getStackLength();
mce.visit(controller.getAcg());
operandStack.pop();
- operandStack.remove(operandStack.getStackLength()-height);
+ operandStack.remove(operandStack.getStackLength() - height);
// return value of assignment
rhsValueLoader.visit(controller.getAcg());
}
}
-
}
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index b3dae09..3e1ee52 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -23,6 +23,7 @@ import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.GenericsType;
+import org.codehaus.groovy.ast.GenericsType.GenericsTypeName;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.Variable;
@@ -109,7 +110,6 @@ import static org.codehaus.groovy.ast.ClassHelper.make;
import static org.codehaus.groovy.ast.ClassHelper.makeWithoutCaching;
import static org.codehaus.groovy.ast.ClassHelper.short_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.void_WRAPPER_TYPE;
-import static org.codehaus.groovy.ast.GenericsType.GenericsTypeName;
import static org.codehaus.groovy.ast.tools.GenericsUtils.getSuperClass;
import static org.codehaus.groovy.runtime.DefaultGroovyMethods.asBoolean;
import static org.codehaus.groovy.syntax.Types.ASSIGN;
@@ -1458,7 +1458,7 @@ public abstract class StaticTypeCheckingSupport {
boolean failure = false;
// correct receiver for inner class
- // we assume the receiver is an instance of the declaring class of the
+ // we assume the receiver is an instance of the declaring class of the
// candidate method, but findMethod returns also outer class methods
// for that receiver. For now we skip receiver based checks in that case
// TODO: correct generics for when receiver is to be skipped
@@ -1479,7 +1479,7 @@ public abstract class StaticTypeCheckingSupport {
// we have here different generics contexts we have to deal with.
// There is firstly the context given through the class, and the method.
- // The method context may hide generics given through the class, but use
+ // The method context may hide generics given through the class, but use
// the non-hidden ones.
Map<GenericsTypeName, GenericsType> resolvedMethodGenerics = new HashMap<GenericsTypeName, GenericsType>();
if (!skipBecauseOfInnerClassNotReceiver) {
@@ -1495,9 +1495,9 @@ public abstract class StaticTypeCheckingSupport {
}
// the outside context parts till now define placeholder we are not allowed to
// generalize, thus we save that for later use...
- // extension methods are special, since they set the receiver as
+ // extension methods are special, since they set the receiver as
// first parameter. While we normally allow generalization for the first
- // parameter, in case of an extension method we must not.
+ // parameter, in case of an extension method we must not.
Set<GenericsTypeName> fixedGenericsPlaceHolders = extractResolvedPlaceHolders(resolvedMethodGenerics);
for (int i = 0; i < arguments.length; i++) {
@@ -1552,7 +1552,7 @@ public abstract class StaticTypeCheckingSupport {
// information. This way the method level information slowly turns
// into information for the callsite
applyGenericsConnections(connections, resolvedMethodGenerics);
- // since it is possible that the callsite uses some generics as well,
+ // since it is possible that the callsite uses some generics as well,
// we may have to add additional elements here
addMissingEntries(connections, resolvedMethodGenerics);
// to finally see if the parameter and the argument fit together,
@@ -1773,7 +1773,7 @@ public abstract class StaticTypeCheckingSupport {
ClassNode corrected = getCorrectedClassNode(type, superClass, true);
extractGenericsConnections(connections, corrected, target);
} else {
- // if we reach here, we have an unhandled case
+ // if we reach here, we have an unhandled case
throw new GroovyBugError("The type " + type + " seems not to normally extend " + target + ". Sorry, I cannot handle this.");
}
}
@@ -1790,7 +1790,7 @@ public abstract class StaticTypeCheckingSupport {
}
private static void extractGenericsConnections(Map<GenericsTypeName, GenericsType> connections, GenericsType[] usage, GenericsType[] declaration) {
- // if declaration does not provide generics, there is no connection to make
+ // if declaration does not provide generics, there is no connection to make
if (usage == null || declaration == null || declaration.length == 0) return;
if (usage.length != declaration.length) return;
@@ -2017,12 +2017,11 @@ public abstract class StaticTypeCheckingSupport {
return gt;
}
- public static boolean isUnboundedWildcard(GenericsType gt) {
+ public static boolean isUnboundedWildcard(final GenericsType gt) {
if (gt.isWildcard() && gt.getLowerBound() == null) {
ClassNode[] upperBounds = gt.getUpperBounds();
- return upperBounds == null ||
- upperBounds.length == 0 ||
- (upperBounds.length == 1 && OBJECT_TYPE.equals(upperBounds[0]));
+ return (upperBounds == null || upperBounds.length == 0 || (upperBounds.length == 1
+ && upperBounds[0].equals(OBJECT_TYPE) && !upperBounds[0].isGenericsPlaceHolder()));
}
return false;
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7361Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7361Bug.groovy
index 6e2adc7..be1a05b 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7361Bug.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7361Bug.groovy
@@ -87,4 +87,20 @@ class Groovy7361Bug extends StaticTypeCheckingTestCase implements StaticCompilat
new B().checkMap()
'''
}
+
+ // GROOVY-9771
+ void testShouldNotThrowClassCastExceptionForSubscriptPrivateAccess() {
+ assertScript '''
+ class C {
+ private final Map<String, Boolean> map = [:]
+ void checkMap() {
+ { ->
+ map['key'] = true
+ }.call()
+ assert map == [key: true]
+ }
+ }
+ new C().checkMap()
+ '''
+ }
}