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 2020/10/06 11:49:35 UTC

[groovy] branch master updated: GROOVY-9769: avoid unnecessary creation of UnionTypeClassNode (closes #1394)

This is an automated email from the ASF dual-hosted git repository.

paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new 7cf8724  GROOVY-9769: avoid unnecessary creation of UnionTypeClassNode (closes #1394)
7cf8724 is described below

commit 7cf8724dd6fe2ca17224ed24911a98966649c70e
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Oct 3 11:25:09 2020 -0500

    GROOVY-9769: avoid unnecessary creation of UnionTypeClassNode (closes #1394)
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 27 +++++++-------
 .../classgen/asm/sc/bugs/Groovy7333Bug.groovy      | 43 +++++++++++++++++++---
 2 files changed, 51 insertions(+), 19 deletions(-)

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 e36cf53..62c37bd 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -2265,26 +2265,27 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         ).toArray(ClassNode[]::new);
     }
 
-    private ClassNode getInferredTypeFromTempInfo(final Expression exp, ClassNode result) {
-        if (exp instanceof VariableExpression && !typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
-            List<ClassNode> classNodes = getTemporaryTypesForExpression(exp);
-            if (classNodes != null && !classNodes.isEmpty()) {
-                List<ClassNode> types = new ArrayList<>(classNodes.size() + 1);
-                if (result != null && !classNodes.contains(result)) types.add(result);
-                types.addAll(classNodes);
-                // GROOVY-7333: filter out Object
-                types.removeIf(OBJECT_TYPE::equals);
+    private ClassNode getInferredTypeFromTempInfo(final Expression expression, final ClassNode expressionType) {
+        if (expression instanceof VariableExpression && !typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
+            List<ClassNode> tempTypes = getTemporaryTypesForExpression(expression);
+            if (tempTypes != null && !tempTypes.isEmpty()) {
+                List<ClassNode> types = new ArrayList<>(tempTypes.size() + 1);
+                if (expressionType != null && !expressionType.equals(ClassHelper.OBJECT_TYPE) // GROOVY-7333
+                        && tempTypes.stream().noneMatch(t -> implementsInterfaceOrIsSubclassOf(t, expressionType))) { // GROOVY-9769
+                    types.add(expressionType);
+                }
+                types.addAll(tempTypes);
 
                 if (types.isEmpty()) {
-                    result = OBJECT_TYPE.getPlainNodeReference();
+                    return OBJECT_TYPE;
                 } else if (types.size() == 1) {
-                    result = types.get(0);
+                    return types.get(0);
                 } else {
-                    result = new UnionTypeClassNode(types.toArray(ClassNode.EMPTY_ARRAY));
+                    return new UnionTypeClassNode(types.toArray(ClassNode.EMPTY_ARRAY));
                 }
             }
         }
-        return result;
+        return expressionType;
     }
 
     @Override
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7333Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7333Bug.groovy
index ecb3c49..0916e27 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7333Bug.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7333Bug.groovy
@@ -16,17 +16,15 @@
  *  specific language governing permissions and limitations
  *  under the License.
  */
-
-
-
-
 package org.codehaus.groovy.classgen.asm.sc.bugs
 
 import groovy.transform.stc.StaticTypeCheckingTestCase
 import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
 
-class Groovy7333Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
-    void testIncorrectInstanceOfInference() {
+final class Groovy7333Bug extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
+
+    // GROOVY-7333
+    void testIncorrectInstanceOfInference1() {
         assertScript '''
             int len(byte[] arr) { arr.length }
             def foo(arg) {
@@ -37,4 +35,37 @@ class Groovy7333Bug extends StaticTypeCheckingTestCase implements StaticCompilat
             assert foo(new byte[3]) == 3
         '''
     }
+
+    // GROOVY-9769
+    void testIncorrectInstanceOfInference2() {
+        assertScript '''
+            interface A {
+                def foo()
+            }
+            interface B extends A {
+                def bar()
+            }
+            @groovy.transform.CompileStatic
+            void test(A a) {
+                if (a instanceof B) {
+                    @ASTTest(phase=INSTRUCTION_SELECTION, value={
+                        def type = node.rightExpression.getNodeMetaData(INFERRED_RETURN_TYPE)
+                        assert type.text == 'B' // not '<UnionType:A+B>'
+                    })
+                    def x = a
+                    a.foo()
+                    a.bar()
+                }
+            }
+
+            def result = ''
+
+            test([
+                foo: { -> result += 'foo' },
+                bar: { -> result += 'bar' }
+            ] as B)
+
+            assert result == 'foobar'
+        '''
+    }
 }