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 + ''' + } +}
