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/06/03 02:55:29 UTC
[groovy] branch GROOVY-9344 created (now 4bbc059)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-9344
in repository https://gitbox.apache.org/repos/asf/groovy.git.
at 4bbc059 GROOVY-9344: SC: retain closure shared variable metadata for classgen
This branch includes the following new commits:
new 4bbc059 GROOVY-9344: SC: retain closure shared variable metadata for classgen
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-9344: SC: retain closure shared variable
metadata for classgen
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-9344
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 4bbc059ee71d3bfd49c71227502fc9f922f63f09
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
---
.../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 67e1d99..067ab02 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;
@@ -2300,20 +2300,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
@@ -2332,7 +2337,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();
@@ -2352,11 +2357,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);
@@ -2389,10 +2394,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) {
@@ -2400,10 +2407,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'
'''
}