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/11/03 23:28:59 UTC
[groovy] branch master updated: GROOVY-9786: STC: propagate
assignments to outer if-else for if-else-if
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 024ffdc GROOVY-9786: STC: propagate assignments to outer if-else for if-else-if
024ffdc is described below
commit 024ffdc1a3e8ad67f9dc7ebcb2d247c9a0902010
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Oct 22 11:32:04 2020 -0500
GROOVY-9786: STC: propagate assignments to outer if-else for if-else-if
---
.../transform/stc/StaticTypeCheckingVisitor.java | 85 +++++++++++-----------
.../groovy/transform/stc/STCAssignmentTest.groovy | 44 +++++++++--
2 files changed, 81 insertions(+), 48 deletions(-)
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 62c37bd..b590d5c 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -855,28 +855,22 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
resultType = originType;
}
- // if we are in an if/else branch, keep track of assignment
- if (typeCheckingContext.ifElseForWhileAssignmentTracker != null && leftExpression instanceof VariableExpression
- && !isNullConstant(rightExpression)) {
+ // track conditional assignment
+ if (!isNullConstant(rightExpression)
+ && leftExpression instanceof VariableExpression
+ && typeCheckingContext.ifElseForWhileAssignmentTracker != null) {
Variable accessedVariable = ((VariableExpression) leftExpression).getAccessedVariable();
if (accessedVariable instanceof Parameter) {
accessedVariable = new ParameterVariableExpression((Parameter) accessedVariable);
}
if (accessedVariable instanceof VariableExpression) {
- VariableExpression var = (VariableExpression) accessedVariable;
- List<ClassNode> types = typeCheckingContext.ifElseForWhileAssignmentTracker.get(var);
- if (types == null) {
- types = new LinkedList<>();
- ClassNode type = var.getNodeMetaData(INFERRED_TYPE);
- types.add(type);
- typeCheckingContext.ifElseForWhileAssignmentTracker.put(var, types);
- }
- types.add(resultType);
+ recordAssignment((VariableExpression) accessedVariable, resultType);
}
}
+
storeType(leftExpression, resultType);
- // if right expression is a ClosureExpression, store parameter type information
+ // propagate closure parameter type information
if (leftExpression instanceof VariableExpression) {
if (rightExpression instanceof ClosureExpression) {
leftExpression.putNodeMetaData(CLOSURE_ARGUMENTS, ((ClosureExpression) rightExpression).getParameters());
@@ -3836,7 +3830,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
@Override
public void visitIfElse(final IfStatement ifElse) {
Map<VariableExpression, List<ClassNode>> oldTracker = pushAssignmentTracking();
-
try {
// create a new temporary element in the if-then-else type info
typeCheckingContext.pushTemporaryTypeInfo();
@@ -3851,15 +3844,23 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
restoreTypeBeforeConditional();
ifElse.getElseBlock().visit(this);
+
+ // GROOVY-9786: if chaining: "if (...) x=?; else if (...) x=?;"
+ Map<VariableExpression, ClassNode> updates =
+ ifElse.getElseBlock().getNodeMetaData("assignments");
+ if (updates != null) {
+ updates.forEach(this::recordAssignment);
+ }
} finally {
- popAssignmentTracking(oldTracker);
+ ifElse.putNodeMetaData("assignments", popAssignmentTracking(oldTracker));
}
- BinaryExpression instanceOfExpression = findInstanceOfNotReturnExpression(ifElse);
- if (instanceOfExpression == null) {
- instanceOfExpression = findNotInstanceOfReturnExpression(ifElse);
- }
- if (instanceOfExpression != null) {
- if (!typeCheckingContext.enclosingBlocks.isEmpty()) {
+
+ if (!typeCheckingContext.enclosingBlocks.isEmpty()) {
+ BinaryExpression instanceOfExpression = findInstanceOfNotReturnExpression(ifElse);
+ if (instanceOfExpression == null) {
+ instanceOfExpression = findNotInstanceOfReturnExpression(ifElse);
+ }
+ if (instanceOfExpression != null) {
visitInstanceofNot(instanceOfExpression);
}
}
@@ -4009,32 +4010,30 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
restoreTypeBeforeConditional();
}
+ private void recordAssignment(final VariableExpression lhsExpr, final ClassNode rhsType) {
+ typeCheckingContext.ifElseForWhileAssignmentTracker.computeIfAbsent(lhsExpr, lhs -> {
+ ClassNode lhsType = lhs.getNodeMetaData(INFERRED_TYPE);
+ List<ClassNode> types = new ArrayList<>(2);
+ types.add(lhsType);
+ return types;
+ }).add(rhsType);
+ }
+
private void restoreTypeBeforeConditional() {
- Set<Map.Entry<VariableExpression, List<ClassNode>>> entries = typeCheckingContext.ifElseForWhileAssignmentTracker.entrySet();
- for (Map.Entry<VariableExpression, List<ClassNode>> entry : entries) {
- VariableExpression var = entry.getKey();
- List<ClassNode> items = entry.getValue();
- ClassNode originValue = items.get(0);
- storeType(var, originValue);
- }
+ typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, types) -> {
+ ClassNode originType = types.get(0);
+ storeType(var, originType);
+ });
}
protected Map<VariableExpression, ClassNode> popAssignmentTracking(final Map<VariableExpression, List<ClassNode>> oldTracker) {
- Map<VariableExpression, ClassNode> assignments = new HashMap<VariableExpression, ClassNode>();
- if (!typeCheckingContext.ifElseForWhileAssignmentTracker.isEmpty()) {
- for (Map.Entry<VariableExpression, List<ClassNode>> entry : typeCheckingContext.ifElseForWhileAssignmentTracker.entrySet()) {
- VariableExpression key = entry.getKey();
- List<ClassNode> allValues = entry.getValue();
- // GROOVY-6099: First element of the list may be null, if no assignment was made before the branch
- List<ClassNode> nonNullValues = new ArrayList<>(allValues.size());
- for (ClassNode value : allValues) {
- if (value != null) nonNullValues.add(value);
- }
- ClassNode cn = lowestUpperBound(nonNullValues);
- storeType(key, cn);
- assignments.put(key, cn);
- }
- }
+ Map<VariableExpression, ClassNode> assignments = new HashMap<>();
+ typeCheckingContext.ifElseForWhileAssignmentTracker.forEach((var, types) -> {
+ ClassNode type = types.stream().filter(Objects::nonNull) // GROOVY-6099
+ .reduce(WideningCategories::lowestUpperBound).get();
+ assignments.put(var, type);
+ storeType(var, type);
+ });
typeCheckingContext.ifElseForWhileAssignmentTracker = oldTracker;
return assignments;
}
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index ace7882..1c6752a 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -549,18 +549,52 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
void testIfWithCommonInterface() {
assertScript '''
- interface Foo { void foo() }
- class A implements Foo { void foo() { println 'A' } }
- class B implements Foo { void foo() { println 'B' } }
+ interface I {
+ def foo()
+ }
+ class A implements I {
+ def foo() { 'A' }
+ }
+ class B implements I {
+ def foo() { 'B' }
+ }
+
def x = new A()
- def y = 'foo'
+ def y = true
if (y) {
x = new B()
}
- x.foo()
+ assert x.foo() == 'B'
'''
}
+ // GROOVY-9786
+ void testIfElseIfWithCommonInterface() {
+ ['I', 'def', 'var', 'Object'].each {
+ assertScript """
+ interface I {
+ def foo()
+ }
+ class A implements I {
+ def foo() { 'A' }
+ }
+ class B implements I {
+ def foo() { 'B' }
+ }
+
+ $it x
+ def y = false
+ def z = true
+ if (y) {
+ x = new A()
+ } else if (z) {
+ x = new B()
+ }
+ assert x.foo() == 'B'
+ """
+ }
+ }
+
void testForLoopWithNewAssignment() {
shouldFailWithMessages '''
def x = '123'