groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject groovy git commit: GROOVY-8336: Static compilation requires casting inside instanceof check (additional cases)
Date Sun, 01 Oct 2017 20:36:44 GMT
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_4_X 94bc302b8 -> 7f9180faf


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/7f9180fa
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/7f9180fa
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/7f9180fa

Branch: refs/heads/GROOVY_2_4_X
Commit: 7f9180faf9f524c2f009c03089963c933211dfa6
Parents: 94bc302
Author: paulk <paulk@asert.com.au>
Authored: Sun Oct 1 16:53:25 2017 +1000
Committer: paulk <paulk@asert.com.au>
Committed: Mon Oct 2 06:36:30 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/7f9180fa/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 827fbf4..1b35f4a 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/7f9180fa/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 5872bb8..de155a5 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -470,7 +470,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'
@@ -1007,9 +1024,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
@@ -1021,6 +1038,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
@@ -1835,6 +1856,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()
@@ -3186,7 +3211,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;
@@ -3374,6 +3401,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)
{
@@ -3381,20 +3417,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/7f9180fa/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
+        '''
+    }
+}


Mime
View raw message