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/06/13 13:40:25 UTC
[groovy] 06/06: GROOVY-9344: SC: retain closure shared variable
metadata for classgen
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 fafa3c60f3beaec71298e1e95bc27f83ccea5e1e
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jun 2 21:55:11 2020 -0500
GROOVY-9344: SC: retain closure shared variable metadata for classgen
(cherry picked from commit 57d7723079479decb122da3d9686dfced7163c6f)
---
.../classgen/asm/sc/StaticTypesTypeChooser.java | 16 ++-----
.../transform/stc/StaticTypeCheckingVisitor.java | 49 ++++++++++++----------
.../asm/sc/StaticCompileFlowTypingTest.groovy | 28 ++++++++++---
3 files changed, 53 insertions(+), 40 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java
index e0fbb42..f641e50 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesTypeChooser.java
@@ -18,13 +18,12 @@
*/
package org.codehaus.groovy.classgen.asm.sc;
-import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
-import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.Variable;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.codehaus.groovy.classgen.AsmClassGenerator;
import org.codehaus.groovy.classgen.asm.StatementMetaTypeChooser;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;
@@ -35,25 +34,18 @@ import org.codehaus.groovy.transform.stc.StaticTypesMarker;
public class StaticTypesTypeChooser extends StatementMetaTypeChooser {
@Override
public ClassNode resolveType(final Expression exp, final ClassNode current) {
- ASTNode target = exp instanceof VariableExpression ? getTarget((VariableExpression) exp) : exp;
+ Expression target = exp instanceof VariableExpression && !((VariableExpression) exp).isClosureSharedVariable() ? getTarget((VariableExpression) exp) : exp;
ClassNode inferredType = target.getNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE);
if (inferredType == null) {
inferredType = target.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
- if (inferredType == null && target instanceof VariableExpression) {
- Variable variable = ((VariableExpression) target).getAccessedVariable();
- if (variable instanceof Parameter) {
- target = (Parameter) variable;
- inferredType = variable.getOriginType();
- }
- }
}
if (inferredType != null && !ClassHelper.VOID_TYPE.equals(inferredType)) {
return inferredType;
}
- if (target instanceof VariableExpression && ((VariableExpression) target).isThisExpression()) {
- // AsmClassGenerator may create "this" expressions that the type checker knows nothing about
+ // AsmClassGenerator may create "this" expressions that the type checker knows nothing about
+ if (AsmClassGenerator.isThisExpression(target)) {
return current;
}
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 be84a4c..1deff6a 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -108,7 +108,6 @@ import org.codehaus.groovy.syntax.Token;
import org.codehaus.groovy.syntax.TokenUtil;
import org.codehaus.groovy.transform.StaticTypesTransformation;
import org.codehaus.groovy.transform.trait.Traits;
-import org.codehaus.groovy.util.ListHashMap;
import org.objectweb.asm.Opcodes;
import java.lang.reflect.InvocationTargetException;
@@ -117,6 +116,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.EnumMap;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
@@ -2315,20 +2315,25 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
boolean oldStaticContext = typeCheckingContext.isInStaticContext;
typeCheckingContext.isInStaticContext = false;
- // collect every variable expression used in the loop body
- Map<VariableExpression, ClassNode> varOrigType = new HashMap<>();
- Statement code = expression.getCode();
- code.visit(new VariableExpressionTypeMemoizer(varOrigType));
+ // collect every variable expression used in the closure body
+ Map<VariableExpression, ClassNode> variableTypes = new HashMap<>();
+ expression.getCode().visit(new VariableExpressionTypeMemoizer(variableTypes));
Map<VariableExpression, List<ClassNode>> oldTracker = pushAssignmentTracking();
-
- // first, collect closure shared variables and reinitialize types
SharedVariableCollector collector = new SharedVariableCollector(getSourceUnit());
collector.visitClosureExpression(expression);
- Set<VariableExpression> closureSharedExpressions = collector.getClosureSharedExpressions();
- Map<VariableExpression, Map<StaticTypesMarker, Object>> typesBeforeVisit = null;
- if (!closureSharedExpressions.isEmpty()) {
- typesBeforeVisit = new HashMap<>();
- saveVariableExpressionMetadata(closureSharedExpressions, typesBeforeVisit);
+
+ Set<VariableExpression> closureSharedVariables = collector.getClosureSharedExpressions();
+ Map<VariableExpression, Map<StaticTypesMarker, Object>> variableMetadata;
+ if (!closureSharedVariables.isEmpty()) {
+ // GROOVY-6921: call getType in order to update closure shared variables
+ // whose types are inferred thanks to closure parameter type inference
+ for (VariableExpression ve : closureSharedVariables) {
+ getType(ve);
+ }
+ variableMetadata = new HashMap<>();
+ saveVariableExpressionMetadata(closureSharedVariables, variableMetadata);
+ } else {
+ variableMetadata = null;
}
// perform visit
@@ -2347,7 +2352,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
super.visitClosureExpression(expression);
typeCheckingContext.delegationMetadata = typeCheckingContext.delegationMetadata.getParent();
- MethodNode node = new MethodNode("dummy", 0, OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, code);
+ MethodNode node = new MethodNode("dummy", 0, OBJECT_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, expression.getCode());
returnAdder.visitMethod(node);
TypeCheckingContext.EnclosingClosure enclosingClosure = typeCheckingContext.getEnclosingClosure();
@@ -2367,11 +2372,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
typeCheckingContext.popEnclosingClosure();
- boolean typeChanged = isSecondPassNeededForControlStructure(varOrigType, oldTracker);
+ boolean typeChanged = isSecondPassNeededForControlStructure(variableTypes, oldTracker);
if (typeChanged) visitClosureExpression(expression);
// restore original metadata
- restoreVariableExpressionMetadata(typesBeforeVisit);
+ restoreVariableExpressionMetadata(variableMetadata);
typeCheckingContext.isInStaticContext = oldStaticContext;
for (Parameter parameter : getParametersSafe(expression)) {
typeCheckingContext.controlStructureVariables.remove(parameter);
@@ -2404,10 +2409,12 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
protected void saveVariableExpressionMetadata(final Set<VariableExpression> closureSharedExpressions, final Map<VariableExpression, Map<StaticTypesMarker, Object>> typesBeforeVisit) {
for (VariableExpression ve : closureSharedExpressions) {
- // GROOVY-6921: We must force a call to getType in order to update closure shared variable whose
- // types are inferred thanks to closure parameter type inference
- getType(ve);
- Map<StaticTypesMarker, Object> metadata = new ListHashMap<>();
+ Variable v;
+ while ((v = ve.getAccessedVariable()) != ve && v instanceof VariableExpression) {
+ ve = (VariableExpression) v;
+ }
+
+ Map<StaticTypesMarker, Object> metadata = new EnumMap<>(StaticTypesMarker.class);
for (StaticTypesMarker marker : StaticTypesMarker.values()) {
Object value = ve.getNodeMetaData(marker);
if (value != null) {
@@ -2415,10 +2422,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
typesBeforeVisit.put(ve, metadata);
- Variable accessedVariable = ve.getAccessedVariable();
- if (accessedVariable != ve && accessedVariable instanceof VariableExpression) {
- saveVariableExpressionMetadata(Collections.singleton((VariableExpression) accessedVariable), typesBeforeVisit);
- }
}
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileFlowTypingTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileFlowTypingTest.groovy
index 696164d..86632f8 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileFlowTypingTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileFlowTypingTest.groovy
@@ -43,7 +43,7 @@ final class StaticCompileFlowTypingTest {
'''
}
- @NotYetImplemented @Test // GROOVY-9344
+ @Test // GROOVY-9344
void testFlowTyping2() {
assertScript '''
class A {}
@@ -51,15 +51,33 @@ final class StaticCompileFlowTypingTest {
@groovy.transform.CompileStatic
String m() {
- def var
- var = new A()
+ def var = new A()
+ def c = { ->
+ var = new B()
+ var.class.simpleName
+ }
+ c()
+ }
+ assert m() == 'B'
+ '''
+ }
+
+ @NotYetImplemented @Test // GROOVY-9344
+ void testFlowTyping3() {
+ assertScript '''
+ class A {}
+ class B {}
+
+ @groovy.transform.CompileStatic
+ String m() {
+ def var = new A()
def c = { ->
var = new B()
}
c()
- var.toString()
+ var.class.simpleName
}
- assert m() != null
+ assert m() == 'B'
'''
}