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/07/04 10:39:30 UTC
[groovy] branch check-PR1293 created (now fb7de35)
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a change to branch check-PR1293
in repository https://gitbox.apache.org/repos/asf/groovy.git.
at fb7de35 GROOVY-7971: @CS flow typing incorrectly casting to map at runtime
This branch includes the following new commits:
new fb7de35 GROOVY-7971: @CS flow typing incorrectly casting to map at runtime
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-7971: @CS flow typing incorrectly casting to
map at runtime
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch check-PR1293
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit fb7de350135eaecc0b16c8eba1ccddfd8bf646e3
Author: Paul King <pa...@asert.com.au>
AuthorDate: Mon Jun 29 20:34:09 2020 +1000
GROOVY-7971: @CS flow typing incorrectly casting to map at runtime
---
build.gradle | 3 +-
.../transform/sc/StaticCompilationVisitor.java | 15 +++--
.../transform/stc/StaticTypeCheckingVisitor.java | 64 +++++++++++++++++-----
.../groovy/transform/stc/MethodCallsSTCTest.groovy | 50 +++++++++++++++++
4 files changed, 113 insertions(+), 19 deletions(-)
diff --git a/build.gradle b/build.gradle
index 33d4bf5..cea4dd3 100644
--- a/build.gradle
+++ b/build.gradle
@@ -208,10 +208,11 @@ dependencies {
antlr2 "org.apache.ant:ant-antlr:$antVersion"
testImplementation project(':groovy-ant')
- testImplementation project(':groovy-xml')
testImplementation project(':groovy-dateutil')
+ testImplementation project(':groovy-json')
testImplementation project(':groovy-test')
testImplementation project(':groovy-macro')
+ testImplementation project(':groovy-xml')
}
ext.generatedDirectory = "${buildDir}/generated/sources"
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
index 79fe33a..e00d14b 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
@@ -408,10 +408,17 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
public void visitMethodCallExpression(final MethodCallExpression call) {
super.visitMethodCallExpression(call);
- MethodNode target = call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET);
- if (target != null) {
- call.setMethodTarget(target);
- memorizeInitialExpressions(target);
+ MethodNode newTarget = call.getNodeMetaData(DIRECT_METHOD_CALL_TARGET);
+ if (newTarget != null) {
+ MethodNode existingTarget = call.getMethodTarget();
+ if (existingTarget != null)
+ if (!(newTarget.getDeclaringClass().equals(existingTarget.getDeclaringClass())) || !(newTarget.getTypeDescriptor().equals(existingTarget.getTypeDescriptor()))) {
+ // avoid cast class exception
+ newTarget.putNodeMetaData(DYNAMIC_RESOLUTION, Boolean.TRUE);
+ call.putNodeMetaData(DYNAMIC_RESOLUTION, OBJECT_TYPE);
+ }
+ call.setMethodTarget(newTarget);
+ memorizeInitialExpressions(newTarget);
}
if (call.getMethodTarget() == null && call.getLineNumber() > 0) {
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 38eb2f3..5e8cd04 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -52,6 +52,7 @@ import org.codehaus.groovy.ast.expr.ArgumentListExpression;
import org.codehaus.groovy.ast.expr.AttributeExpression;
import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.BitwiseNegationExpression;
+import org.codehaus.groovy.ast.expr.BooleanExpression;
import org.codehaus.groovy.ast.expr.CastExpression;
import org.codehaus.groovy.ast.expr.ClassExpression;
import org.codehaus.groovy.ast.expr.ClosureExpression;
@@ -184,6 +185,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.constX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.getSetterName;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.ifElseS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.isOrImplements;
import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
@@ -218,6 +220,7 @@ import static org.codehaus.groovy.syntax.Types.INTDIV_EQUAL;
import static org.codehaus.groovy.syntax.Types.KEYWORD_IN;
import static org.codehaus.groovy.syntax.Types.KEYWORD_INSTANCEOF;
import static org.codehaus.groovy.syntax.Types.LEFT_SQUARE_BRACKET;
+import static org.codehaus.groovy.syntax.Types.LOGICAL_OR;
import static org.codehaus.groovy.syntax.Types.MINUS_MINUS;
import static org.codehaus.groovy.syntax.Types.MOD;
import static org.codehaus.groovy.syntax.Types.MOD_EQUAL;
@@ -3798,21 +3801,8 @@ 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();
- visitStatement(ifElse);
- ifElse.getBooleanExpression().visit(this);
- ifElse.getIfBlock().visit(this);
-
- // pop if-then-else temporary type info
- typeCheckingContext.popTemporaryTypeInfo();
-
- // GROOVY-6099: restore assignment info as before the if branch
- restoreTypeBeforeConditional();
-
- ifElse.getElseBlock().visit(this);
+ visitIfElseMaybeOrBranches(ifElse, true);
} finally {
popAssignmentTracking(oldTracker);
}
@@ -3827,6 +3817,52 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
+ private void visitIfElseMaybeOrBranches(IfStatement ifElse, boolean topLevel) {
+ BooleanExpression condition = ifElse.getBooleanExpression();
+ BinaryExpression lor = null;
+ if (condition.getExpression() instanceof BinaryExpression) {
+ lor = (BinaryExpression) condition.getExpression();
+ if (lor.getOperation().getType() != LOGICAL_OR) {
+ lor = null;
+ }
+ }
+ // for logical OR, any one branch may be true branch, so traverse separately
+ if (lor != null) {
+ IfStatement left = ifElseS(lor.getLeftExpression(), ifElse.getIfBlock(), ifElse.getElseBlock());
+// left.setSourcePosition(ifElse);
+ typeCheckingContext.pushTemporaryTypeInfo();
+ visitIfElseMaybeOrBranches(left, false);
+ typeCheckingContext.popTemporaryTypeInfo();
+ restoreTypeBeforeConditional();
+ IfStatement right = ifElseS(lor.getRightExpression(), ifElse.getIfBlock(), ifElse.getElseBlock());
+// right.setSourcePosition(ifElse);
+ typeCheckingContext.pushTemporaryTypeInfo();
+ visitIfElseMaybeOrBranches(right, false);
+ typeCheckingContext.popTemporaryTypeInfo();
+ restoreTypeBeforeConditional();
+ }
+ if (topLevel || lor == null) {
+ // do it all again to get correct union type for casting (hush warnings?)
+ visitIfElseBranches(ifElse);
+ }
+ }
+
+ private void visitIfElseBranches(IfStatement ifElse) {
+ // create a new temporary element in the if-then-else type info
+ typeCheckingContext.pushTemporaryTypeInfo();
+ visitStatement(ifElse);
+ ifElse.getBooleanExpression().visit(this);
+ ifElse.getIfBlock().visit(this);
+
+ // pop if-then-else temporary type info
+ typeCheckingContext.popTemporaryTypeInfo();
+
+ // GROOVY-6099: restore assignment info as before the if branch
+ restoreTypeBeforeConditional();
+
+ ifElse.getElseBlock().visit(this);
+ }
+
protected void visitInstanceofNot(final BinaryExpression be) {
BlockStatement currentBlock = typeCheckingContext.enclosingBlocks.getFirst();
assert currentBlock != null;
diff --git a/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy b/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
index 16359b8..23d7467 100644
--- a/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/MethodCallsSTCTest.groovy
@@ -424,6 +424,56 @@ class MethodCallsSTCTest extends StaticTypeCheckingTestCase {
''', 'Reference to method is ambiguous'
}
+ // GROOVY-7971
+ void testShouldNotFailWithMultiplePossibleMethods() {
+ assertScript '''
+ import groovy.json.JsonOutput
+
+ def test(value) {
+ def out = new StringBuilder()
+ def isString = value.class == String
+ if (isString || value instanceof Map) {
+ out.append(JsonOutput.toJson(value))
+ }
+ return out.toString()
+ }
+
+ def string = test('two')
+ assert string == '"two"'
+ '''
+ }
+
+ // GROOVY-7971
+ void testShouldNotFailWithUnionTypeLUB() {
+ assertScript '''
+ class Foo extends AbstractCollection {
+ def peek() { 3 }
+ int size() { -1 }
+ void clear() { }
+ Iterator iterator() { null }
+ }
+
+ def method(idx) {
+ def component = idx == 1 ? new ArrayDeque() : new Stack()
+ if (idx == 3) component = new Foo()
+ component.clear() // 'clear' in LUB (AbstractCollection or Serializable or Cloneable)
+ if (component instanceof ArrayDeque) {
+ component.addFirst(1) // 'addFirst' only in ArrayDeque
+ } else if (component instanceof Stack) {
+ component.addElement(2) // 'addElement' only in Stack
+ }
+ if (component instanceof Foo || component instanceof ArrayDeque || component instanceof Stack) {
+ // checked duck typing
+ assert component.peek() in 1..3 // 'peek' in ArrayDeque and Stack but not LUB
+ }
+ }
+
+ method(1)
+ method(2)
+ method(3)
+ '''
+ }
+
void testInstanceOfOnExplicitParameter() {
assertScript '''
1.with { obj ->