You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2017/10/01 20:35:20 UTC
groovy git commit: GROOVY-8336: Static compilation requires casting
inside instanceof check (additional cases)
Repository: groovy
Updated Branches:
refs/heads/master 6f968d6c7 -> 1b7988218
GROOVY-8336: Static compilation requires casting inside instanceof check (additional cases)
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/1b798821
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/1b798821
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/1b798821
Branch: refs/heads/master
Commit: 1b79882184484086220516b182c56a6c78e56ffd
Parents: 6f968d6
Author: paulk <pa...@asert.com.au>
Authored: Sun Oct 1 16:53:25 2017 +1000
Committer: paulk <pa...@asert.com.au>
Committed: Sun Oct 1 16:53:25 2017 +1000
----------------------------------------------------------------------
.../asm/sc/StaticTypesCallSiteWriter.java | 2 +-
.../stc/StaticTypeCheckingVisitor.java | 55 +++++++++++--
.../groovy/transform/stc/MiscSTCTest.groovy | 83 +++++++++++++++++++-
3 files changed, 127 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/1b798821/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
index d558f9d..8d003eb 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -629,7 +629,7 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter implements Opcodes
}
// now try with flow type instead of declaration type
rType = receiver.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
- if (receiver instanceof VariableExpression && receiver.getNodeMetaData().isEmpty()) {
+ if (receiver instanceof VariableExpression && rType == null) {
// TODO: can STCV be made smarter to avoid this check?
VariableExpression ve = (VariableExpression) ((VariableExpression)receiver).getAccessedVariable();
rType = ve.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
http://git-wip-us.apache.org/repos/asf/groovy/blob/1b798821/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 807449a..39e2114 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -480,7 +480,24 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
- if (! (vexp.getAccessedVariable() instanceof DynamicVariable)) return;
+ if (!(vexp.getAccessedVariable() instanceof DynamicVariable)) {
+ if (typeCheckingContext.getEnclosingClosure() == null) {
+ VariableExpression variable = null;
+ if (vexp.getAccessedVariable() instanceof Parameter) {
+ variable = new ParameterVariableExpression((Parameter) vexp.getAccessedVariable());
+ } else if (vexp.getAccessedVariable() instanceof VariableExpression) {
+ variable = (VariableExpression) vexp.getAccessedVariable();
+ }
+ if (variable != null) {
+ ClassNode inferredType = getInferredTypeFromTempInfo(variable, variable.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE));
+ // instanceof applies, stash away the type, reusing key used elsewhere
+ if (inferredType != null && !inferredType.getName().equals("java.lang.Object")) {
+ vexp.putNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE, inferredType);
+ }
+ }
+ }
+ return;
+ }
// a dynamic variable is either an undeclared variable
// or a member of a class used in a 'with'
@@ -1005,9 +1022,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
final Expression leftExpression,
final ClassNode leftExpressionType,
final Expression rightExpression,
- final ClassNode inferredRightExpressionType)
+ final ClassNode inferredRightExpressionTypeOrig)
{
-
+ ClassNode inferredRightExpressionType = inferredRightExpressionTypeOrig;
if (!typeCheckMultipleAssignmentAndContinue(leftExpression, rightExpression)) return;
if (leftExpression instanceof VariableExpression
@@ -1019,6 +1036,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (addedReadOnlyPropertyError(leftExpression)) return;
ClassNode leftRedirect = leftExpressionType.redirect();
+ // see if instanceof applies
+ if (rightExpression instanceof VariableExpression && hasInferredReturnType(rightExpression) && assignmentExpression.getOperation().getType() == EQUAL) {
+ inferredRightExpressionType = rightExpression.getNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE);
+ }
ClassNode wrappedRHS = adjustTypeForSpreading(inferredRightExpressionType, leftExpression);
// check types are compatible for assignment
@@ -1833,6 +1854,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (typeCheckingContext.getEnclosingClosure()!=null) {
return type;
}
+ // handle instanceof cases
+ if ((expression instanceof VariableExpression) && hasInferredReturnType(expression)) {
+ type = expression.getNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE);
+ }
MethodNode enclosingMethod = typeCheckingContext.getEnclosingMethod();
if (enclosingMethod != null && typeCheckingContext.getEnclosingClosure()==null) {
if (!enclosingMethod.isVoidMethod()
@@ -3205,7 +3230,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
int depth = typeCheckingContext.temporaryIfBranchTypeInformation.size();
while (classNodes == null && depth > 0) {
final Map<Object, List<ClassNode>> tempo = typeCheckingContext.temporaryIfBranchTypeInformation.get(--depth);
- Object key = extractTemporaryTypeInfoKey(objectExpression);
+ Object key = objectExpression instanceof ParameterVariableExpression
+ ? ((ParameterVariableExpression) objectExpression).parameter
+ : extractTemporaryTypeInfoKey(objectExpression);
classNodes = tempo.get(key);
}
return classNodes;
@@ -3393,6 +3420,15 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
typeCheckingContext.popTemporaryTypeInfo();
falseExpression.visit(this);
ClassNode resultType;
+ ClassNode typeOfFalse = getType(falseExpression);
+ ClassNode typeOfTrue = getType(trueExpression);
+ // handle instanceof cases
+ if (hasInferredReturnType(falseExpression)) {
+ typeOfFalse = falseExpression.getNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE);
+ }
+ if (hasInferredReturnType(trueExpression)) {
+ typeOfTrue = trueExpression.getNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE);
+ }
if (isNullConstant(trueExpression) || isNullConstant(falseExpression)) {
BinaryExpression enclosingBinaryExpression = typeCheckingContext.getEnclosingBinaryExpression();
if (enclosingBinaryExpression != null && enclosingBinaryExpression.getRightExpression()==expression) {
@@ -3400,20 +3436,23 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
} else if (isNullConstant(trueExpression) && isNullConstant(falseExpression)) {
resultType = OBJECT_TYPE;
} else if (isNullConstant(trueExpression)) {
- resultType = wrapTypeIfNecessary(getType(falseExpression));
+ resultType = wrapTypeIfNecessary(typeOfFalse);
} else {
- resultType = wrapTypeIfNecessary(getType(trueExpression));
+ resultType = wrapTypeIfNecessary(typeOfTrue);
}
} else {
// store type information
- final ClassNode typeOfTrue = getType(trueExpression);
- final ClassNode typeOfFalse = getType(falseExpression);
resultType = lowestUpperBound(typeOfTrue, typeOfFalse);
}
storeType(expression, resultType);
popAssignmentTracking(oldTracker);
}
+ private boolean hasInferredReturnType(Expression expression) {
+ ClassNode type = expression.getNodeMetaData(StaticTypesMarker.INFERRED_RETURN_TYPE);
+ return type != null && !type.getName().equals("java.lang.Object");
+ }
+
@Override
public void visitTryCatchFinally(final TryCatchStatement statement) {
final List<CatchStatement> catchStatements = statement.getCatchStatements();
http://git-wip-us.apache.org/repos/asf/groovy/blob/1b798821/src/test/groovy/transform/stc/MiscSTCTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/transform/stc/MiscSTCTest.groovy b/src/test/groovy/transform/stc/MiscSTCTest.groovy
index 13bdb40..94d8c0e 100644
--- a/src/test/groovy/transform/stc/MiscSTCTest.groovy
+++ b/src/test/groovy/transform/stc/MiscSTCTest.groovy
@@ -22,8 +22,6 @@ import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor
/**
* Unit tests for static type checking : miscellaneous tests.
- *
- * @author Cedric Champeau
*/
class MiscSTCTest extends StaticTypeCheckingTestCase {
@@ -272,8 +270,85 @@ class MiscSTCTest extends StaticTypeCheckingTestCase {
}
}
- public static class MiscSTCTestSupport {
+ static class MiscSTCTestSupport {
static def foo() { '123' }
}
-}
+ void testTernaryParam() {
+ assertScript '''
+ Date ternaryParam(Object input) {
+ input instanceof Date ? input : null
+ }
+ def d = new Date()
+ assert ternaryParam(42) == null
+ assert ternaryParam('foo') == null
+ assert ternaryParam(d) == d
+ '''
+ }
+
+ void testTernaryLocalVar() {
+ assertScript '''
+ Date ternaryLocalVar(Object input) {
+ Object copy = input
+ copy instanceof Date ? copy : null
+ }
+ def d = new Date()
+ assert ternaryLocalVar(42) == null
+ assert ternaryLocalVar('foo') == null
+ assert ternaryLocalVar(d) == d
+ '''
+ }
+
+ void testIfThenElseParam() {
+ assertScript '''
+ Date ifThenElseParam(Object input) {
+ if (input instanceof Date) {
+ input
+ } else {
+ null
+ }
+ }
+ def d = new Date()
+ assert ifThenElseParam(42) == null
+ assert ifThenElseParam('foo') == null
+ assert ifThenElseParam(d) == d
+ '''
+ }
+
+ void testIfThenElseLocalVar() {
+ assertScript '''
+ Date ifThenElseLocalVar(Object input) {
+ Date result
+ if (input instanceof Date) {
+ result = input
+ } else {
+ result = null
+ }
+ result
+ }
+ def d = new Date()
+ assert ifThenElseLocalVar(42) == null
+ assert ifThenElseLocalVar('foo') == null
+ assert ifThenElseLocalVar(d) == d
+ '''
+ }
+
+ void testIfThenElseLocalVar2() {
+ assertScript '''
+ class FooBase {}
+ class FooChild extends FooBase{}
+ FooChild ifThenElseLocalVar2(FooBase input) {
+ FooChild result
+ if (input instanceof FooChild) {
+ result = input
+ } else {
+ result = null
+ }
+ result
+ }
+ def fc = new FooChild()
+ assert ifThenElseLocalVar2(fc) == fc
+ assert ifThenElseLocalVar2(new FooBase()) == null
+ '''
+ }
+}