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/08/20 20:25:53 UTC
[groovy] branch GROOVY-9699 created (now 8de9b7e)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-9699
in repository https://gitbox.apache.org/repos/asf/groovy.git.
at 8de9b7e GROOVY-9699: SC: reposition checkOrMarkPrivateAccess for field reference
This branch includes the following new commits:
new 8de9b7e GROOVY-9699: SC: reposition checkOrMarkPrivateAccess for field reference
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[groovy] 01/01: GROOVY-9699: SC: reposition
checkOrMarkPrivateAccess for field reference
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-9699
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 8de9b7edc711eb13e59618e1474d308472cfc025
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Aug 20 15:23:12 2020 -0500
GROOVY-9699: SC: reposition checkOrMarkPrivateAccess for field reference
and drop extra visitation for "putAt" method call transformation
---
...icTypesBinaryExpressionMultiTypeDispatcher.java | 2 -
.../transform/stc/StaticTypeCheckingVisitor.java | 48 +++++++---------------
.../classgen/asm/sc/bugs/Groovy7361Bug.groovy | 29 +++++++++++++
3 files changed, 44 insertions(+), 35 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 26685e5..9c835fd 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
@@ -44,7 +44,6 @@ import org.codehaus.groovy.classgen.asm.VariableSlotLoader;
import org.codehaus.groovy.classgen.asm.WriterController;
import org.codehaus.groovy.syntax.Token;
import org.codehaus.groovy.syntax.TokenUtil;
-import org.codehaus.groovy.transform.sc.StaticCompilationVisitor;
import org.objectweb.asm.Label;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;
@@ -393,7 +392,6 @@ public class StaticTypesBinaryExpressionMultiTypeDispatcher extends BinaryExpres
MethodCallExpression call = callX(receiver, "putAt", args(subscript, rhsValueLoader));
call.setSafe(safe);
call.setSourcePosition(enclosing);
- call.visit(new StaticCompilationVisitor(controller.getSourceUnit(), controller.getClassNode()));
OperandStack operandStack = controller.getOperandStack();
int height = operandStack.getStackLength();
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 6b69ee6..a2e814d 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -594,6 +594,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (enclosingClosure != null) {
tryVariableExpressionAsProperty(vexp, name);
} else {
+ checkOrMarkPrivateAccess(vexp, (FieldNode) accessedVariable, typeCheckingContext.isTargetOfEnclosingAssignment(vexp));
+
// GROOVY-9454
ClassNode inferredType = getInferredTypeFromTempInfo(vexp, null);
if (inferredType != null && !inferredType.equals(OBJECT_TYPE)) {
@@ -1265,12 +1267,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
ClassNode inferredRightExpressionType = inferredRightExpressionTypeOrig;
if (!typeCheckMultipleAssignmentAndContinue(leftExpression, rightExpression)) return;
- if (leftExpression instanceof VariableExpression
- && ((VariableExpression) leftExpression).getAccessedVariable() instanceof FieldNode) {
- checkOrMarkPrivateAccess(leftExpression, (FieldNode) ((VariableExpression) leftExpression).getAccessedVariable(), true);
- }
-
- //TODO: need errors for write-only too!
+ // TODO: need errors for write-only too!
if (addedReadOnlyPropertyError(leftExpression)) return;
ClassNode leftRedirect = leftExpressionType.redirect();
@@ -4811,9 +4808,15 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (vexp.isSuperExpression()) return makeSuper();
Variable variable = vexp.getAccessedVariable();
if (variable instanceof FieldNode) {
- FieldNode fieldNode = (FieldNode) variable;
- checkOrMarkPrivateAccess(vexp, fieldNode, typeCheckingContext.isTargetOfEnclosingAssignment(vexp));
- return getType(fieldNode);
+ ClassNode fieldType = variable.getOriginType();
+ if (isUsingGenericsOrIsArrayUsingGenerics(fieldType)) {
+ boolean isStatic = (variable.getModifiers() & Opcodes.ACC_STATIC) != 0;
+ ClassNode thisType = typeCheckingContext.getEnclosingClassNode(), declType = ((FieldNode) variable).getDeclaringClass();
+ Map<GenericsTypeName, GenericsType> placeholders = resolvePlaceHoldersFromDeclaration(thisType, declType, null, isStatic);
+
+ fieldType = resolveGenericsWithContext(placeholders, fieldType);
+ }
+ return fieldType;
}
if (variable != vexp && variable instanceof VariableExpression) {
return getType((Expression) variable);
@@ -4856,6 +4859,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
ClassNode ret = getInferredReturnType(exp);
return ret != null ? ret : ((MethodNode) exp).getReturnType();
}
+ if (exp instanceof FieldNode || exp instanceof PropertyNode) {
+ return ((Variable) exp).getOriginType();
+ }
if (exp instanceof RangeExpression) {
ClassNode plain = RANGE_TYPE.getPlainNodeReference();
RangeExpression re = (RangeExpression) exp;
@@ -4884,14 +4890,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (exp instanceof Parameter) {
return ((Parameter) exp).getOriginType();
}
- if (exp instanceof FieldNode) {
- FieldNode fn = (FieldNode) exp;
- return getGenericsResolvedTypeOfFieldOrProperty(fn, fn.getOriginType());
- }
- if (exp instanceof PropertyNode) {
- PropertyNode pn = (PropertyNode) exp;
- return getGenericsResolvedTypeOfFieldOrProperty(pn, pn.getOriginType());
- }
if (exp instanceof ClosureExpression) {
ClassNode irt = getInferredReturnType(exp);
if (irt != null) {
@@ -4931,22 +4929,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return null;
}
- /**
- * resolves a Field or Property node generics by using the current class and
- * the declaring class to extract the right meaning of the generics symbols
- *
- * @param an a FieldNode or PropertyNode
- * @param type the origin type
- * @return the new ClassNode with corrected generics
- */
- private ClassNode getGenericsResolvedTypeOfFieldOrProperty(final AnnotatedNode an, final ClassNode type) {
- if (!type.isUsingGenerics()) return type;
- Map<GenericsTypeName, GenericsType> connections = new HashMap<>();
- // TODO: inner classes mean a different this-type. This is ignored here!
- extractGenericsConnections(connections, typeCheckingContext.getEnclosingClassNode(), an.getDeclaringClass());
- return applyGenericsContext(connections, type);
- }
-
private static ClassNode makeSelf(final ClassNode trait) {
ClassNode selfType = trait;
Set<ClassNode> selfTypes = Traits.collectSelfTypes(selfType, new LinkedHashSet<>());
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 e7f26a4..9878851 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
@@ -58,4 +58,33 @@ final class Groovy7361Bug extends StaticTypeCheckingTestCase implements StaticCo
assert new A().m() == [x:'y']
'''
}
+
+ // GROOVY-9699
+ void testShouldNotEmitErrorForSubscriptPrivateAccess() {
+ assertScript '''
+ class A {
+ private static final java.util.regex.Pattern PATTERN = ~/.*/
+ void checkList() {
+ def list = []
+ def closure = { ->
+ list << PATTERN.pattern()
+ }
+ closure()
+ }
+ void checkMap() {
+ def map = [:]
+ def closure = { ->
+ map[PATTERN.pattern()] = 1
+ }
+ closure()
+ }
+ }
+ class B extends A {
+ }
+ new A().checkList()
+ new B().checkList()
+ new A().checkMap()
+ new B().checkMap()
+ '''
+ }
}