Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 2215F200D0F for ; Fri, 29 Sep 2017 14:37:16 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 206271609D1; Fri, 29 Sep 2017 12:37:16 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 3FE5F1609C5 for ; Fri, 29 Sep 2017 14:37:15 +0200 (CEST) Received: (qmail 70945 invoked by uid 500); 29 Sep 2017 12:37:14 -0000 Mailing-List: contact commits-help@groovy.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@groovy.apache.org Delivered-To: mailing list commits@groovy.apache.org Received: (qmail 70935 invoked by uid 99); 29 Sep 2017 12:37:14 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 29 Sep 2017 12:37:14 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 13141F580B; Fri, 29 Sep 2017 12:37:12 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: paulk@apache.org To: commits@groovy.apache.org Message-Id: <9cf507a7a324496a9df21701da9bccda@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: groovy git commit: GROOVY-8260: Static compilation requires casting inside instanceof check (handle additional cases) Date: Fri, 29 Sep 2017 12:37:12 +0000 (UTC) archived-at: Fri, 29 Sep 2017 12:37:16 -0000 Repository: groovy Updated Branches: refs/heads/GROOVY_2_5_X 3d927668c -> caa1e5f6a GROOVY-8260: Static compilation requires casting inside instanceof check (handle additional cases) Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/caa1e5f6 Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/caa1e5f6 Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/caa1e5f6 Branch: refs/heads/GROOVY_2_5_X Commit: caa1e5f6ad55c9456833bb787f69d7d718a2e915 Parents: 3d92766 Author: paulk Authored: Fri Sep 29 22:35:54 2017 +1000 Committer: paulk Committed: Fri Sep 29 22:36:57 2017 +1000 ---------------------------------------------------------------------- .../stc/StaticTypeCheckingVisitor.java | 51 ++++++++++++++-- .../groovy/transform/stc/MiscSTCTest.groovy | 63 +++++++++++++++++++- 2 files changed, 105 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/caa1e5f6/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 20f5d97..cca4167 100644 --- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -479,7 +479,30 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } } - if (! (vexp.getAccessedVariable() instanceof DynamicVariable)) return; + if (!(vexp.getAccessedVariable() instanceof DynamicVariable)) { + 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)); + if (inferredType != null && !inferredType.getName().equals("java.lang.Object")) { + if (typeCheckingContext.getEnclosingBinaryExpression() != null) { + // TODO narrow this down to assignment + if (typeCheckingContext.getEnclosingBinaryExpression().getRightExpression() == vexp) { + vexp.putNodeMetaData(StaticTypesMarker.INFERRED_TYPE, inferredType); + } + } else { + // stash away type info that will be lost later to handle case + // where this expression has return added later - piggy back on existing key + 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' @@ -1835,6 +1858,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { if (typeCheckingContext.getEnclosingClosure()!=null) { return type; } + 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() @@ -3204,7 +3230,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { int depth = typeCheckingContext.temporaryIfBranchTypeInformation.size(); while (classNodes == null && depth > 0) { final Map> 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; @@ -3392,6 +3420,14 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { typeCheckingContext.popTemporaryTypeInfo(); falseExpression.visit(this); ClassNode resultType; + ClassNode typeOfFalse = getType(falseExpression); + ClassNode typeOfTrue = getType(trueExpression); + 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) { @@ -3399,20 +3435,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 catchStatements = statement.getCatchStatements(); http://git-wip-us.apache.org/repos/asf/groovy/blob/caa1e5f6/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..fadac0a 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,67 @@ 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 + ''' + } }