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 2021/04/23 18:47:15 UTC
[groovy] branch master updated: GROOVY-10052: no re-visit for
closure without changed/shared variable(s)
This is an automated email from the ASF dual-hosted git repository.
emilles 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 2885bcc GROOVY-10052: no re-visit for closure without changed/shared variable(s)
2885bcc is described below
commit 2885bcc825257c8a77161dc13e38df72aa9c24b8
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Apr 23 13:31:39 2021 -0500
GROOVY-10052: no re-visit for closure without changed/shared variable(s)
---
.../transform/stc/StaticTypeCheckingVisitor.java | 30 ++++++++++------
.../groovy/transform/stc/ClosuresSTCTest.groovy | 41 +++++++++++++++++++++-
2 files changed, 60 insertions(+), 11 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 8278c5a..061586e 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1943,8 +1943,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
typeCheckingContext.controlStructureVariables.remove(forLoop.getVariable());
}
}
- boolean typeChanged = isSecondPassNeededForControlStructure(varOrigType, oldTracker);
- if (typeChanged) visitForLoop(forLoop);
+ if (isSecondPassNeededForControlStructure(varOrigType, oldTracker)) {
+ visitForLoop(forLoop);
+ }
}
/**
@@ -2365,8 +2366,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
typeCheckingContext.isInStaticContext = false;
// collect every variable expression used in the closure body
- Map<VariableExpression, ClassNode> variableTypes = new HashMap<>();
- expression.getCode().visit(new VariableExpressionTypeMemoizer(variableTypes));
+ Map<VariableExpression, ClassNode> varTypes = new HashMap<>();
+ expression.getCode().visit(new VariableExpressionTypeMemoizer(varTypes, true));
Map<VariableExpression, List<ClassNode>> oldTracker = pushAssignmentTracking();
SharedVariableCollector collector = new SharedVariableCollector(getSourceUnit());
collector.visitClosureExpression(expression);
@@ -2420,9 +2421,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
typeCheckingContext.popEnclosingClosure();
-
- boolean typeChanged = isSecondPassNeededForControlStructure(variableTypes, oldTracker);
- if (typeChanged) visitClosureExpression(expression);
+ // check types of closure shared variables for change
+ if (isSecondPassNeededForControlStructure(varTypes, oldTracker)) {
+ visitClosureExpression(expression);
+ }
// restore original metadata
restoreVariableExpressionMetadata(variableMetadata);
@@ -5973,10 +5975,16 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
protected class VariableExpressionTypeMemoizer extends ClassCodeVisitorSupport {
+ private final boolean onlySharedVariables;
private final Map<VariableExpression, ClassNode> varOrigType;
public VariableExpressionTypeMemoizer(final Map<VariableExpression, ClassNode> varOrigType) {
+ this(varOrigType, false);
+ }
+
+ public VariableExpressionTypeMemoizer(final Map<VariableExpression, ClassNode> varOrigType, final boolean onlySharedVariables) {
this.varOrigType = varOrigType;
+ this.onlySharedVariables = onlySharedVariables;
}
@Override
@@ -5986,12 +5994,14 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
@Override
public void visitVariableExpression(final VariableExpression expression) {
- super.visitVariableExpression(expression);
Variable var = findTargetVariable(expression);
- if (var instanceof VariableExpression) {
+ if ((!onlySharedVariables || var.isClosureSharedVariable()) && var instanceof VariableExpression) {
VariableExpression ve = (VariableExpression) var;
- varOrigType.put(ve, ve.getNodeMetaData(INFERRED_TYPE));
+ ClassNode cn = ve.getNodeMetaData(INFERRED_TYPE);
+ if (cn == null) cn = ve.getOriginType();
+ varOrigType.put(ve, cn);
}
+ super.visitVariableExpression(expression);
}
}
}
diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
index b707e77..2c6ebc9 100644
--- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
@@ -18,6 +18,8 @@
*/
package groovy.transform.stc
+import groovy.test.NotYetImplemented
+
/**
* Unit tests for static type checking : closures.
*/
@@ -32,8 +34,8 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-9079: no params to statically type check but shouldn't get NPE
void testClosureWithoutArgumentsExplicit() {
- // GROOVY-9079: no params to statically type check but shouldn't get NPE
assertScript '''
import java.util.concurrent.Callable
@@ -212,6 +214,43 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
''', 'Cannot find matching method A#m()'
}
+ // GROOVY-10052
+ void testClosureSharedVariable4() {
+ assertScript '''
+ String x
+ def f = { ->
+ x = Optional.of('x').orElseThrow{ new Exception() }
+ }
+ assert f() == 'x'
+ assert x == 'x'
+ '''
+ }
+
+ @NotYetImplemented // GROOVY-10052
+ void testClosureSharedVariable5() {
+ assertScript '''
+ def x
+ def f = { ->
+ x = Optional.of('x').orElseThrow{ new Exception() }
+ }
+ assert f() == 'x'
+ assert x == 'x'
+ '''
+ }
+
+ // GROOVY-10052
+ void testNotClosureSharedVariable() {
+ assertScript '''
+ String x = Optional.of('x').orElseThrow{ new Exception() }
+ def f = { ->
+ String y = Optional.of('y').orElseThrow{ new Exception() }
+ }
+
+ assert x == 'x'
+ assert f() == 'y'
+ '''
+ }
+
void testClosureCallAsAMethod() {
assertScript '''
Closure cl = { 'foo' }