You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2020/12/31 22:15:53 UTC

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

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

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


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

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

    GROOVY-9769: avoid unnecessary creation of UnionTypeClassNode (closes
    #1394)
    
    Conflicts:
    	src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
---
 .../transform/stc/StaticTypeCheckingVisitor.java   | 50 ++++++++++++----------
 .../classgen/asm/sc/bugs/Groovy7333Bug.groovy      | 43 ++++++++++++++++---
 2 files changed, 64 insertions(+), 29 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 87b7af9..6f54579 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -40,6 +40,7 @@ import org.codehaus.groovy.ast.ConstructorNode;
 import org.codehaus.groovy.ast.DynamicVariable;
 import org.codehaus.groovy.ast.FieldNode;
 import org.codehaus.groovy.ast.GenericsType;
+import org.codehaus.groovy.ast.GenericsType.GenericsTypeName;
 import org.codehaus.groovy.ast.InnerClassNode;
 import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.Parameter;
@@ -90,6 +91,7 @@ import org.codehaus.groovy.ast.stmt.TryCatchStatement;
 import org.codehaus.groovy.ast.stmt.WhileStatement;
 import org.codehaus.groovy.ast.tools.GenericsUtils;
 import org.codehaus.groovy.ast.tools.WideningCategories;
+import org.codehaus.groovy.ast.tools.WideningCategories.LowestUpperBoundClassNode;
 import org.codehaus.groovy.classgen.ReturnAdder;
 import org.codehaus.groovy.classgen.asm.InvocationWriter;
 import org.codehaus.groovy.control.CompilationUnit;
@@ -169,7 +171,6 @@ import static org.codehaus.groovy.ast.ClassHelper.long_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.make;
 import static org.codehaus.groovy.ast.ClassHelper.short_TYPE;
 import static org.codehaus.groovy.ast.ClassHelper.void_WRAPPER_TYPE;
-import static org.codehaus.groovy.ast.GenericsType.GenericsTypeName;
 import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.args;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.binX;
@@ -180,7 +181,6 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.thisPropX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
 import static org.codehaus.groovy.ast.tools.GenericsUtils.toGenericTypesString;
-import static org.codehaus.groovy.ast.tools.WideningCategories.LowestUpperBoundClassNode;
 import static org.codehaus.groovy.ast.tools.WideningCategories.isBigDecCategory;
 import static org.codehaus.groovy.ast.tools.WideningCategories.isBigIntCategory;
 import static org.codehaus.groovy.ast.tools.WideningCategories.isDouble;
@@ -2276,32 +2276,36 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
         return ret;
     }
 
-    private ClassNode getInferredTypeFromTempInfo(Expression exp, ClassNode result) {
-        Map<Object, List<ClassNode>> info = typeCheckingContext.temporaryIfBranchTypeInformation.empty() ? null : typeCheckingContext.temporaryIfBranchTypeInformation.peek();
-        if (exp instanceof VariableExpression && info != null) {
-            List<ClassNode> classNodes = getTemporaryTypesForExpression(exp);
-            if (classNodes != null && !classNodes.isEmpty()) {
-                ArrayList<ClassNode> arr = new ArrayList<ClassNode>(classNodes.size() + 1);
-                if (result != null && !classNodes.contains(result)) arr.add(result);
-                arr.addAll(classNodes);
-                // GROOVY-7333: filter out Object
-                Iterator<ClassNode> iterator = arr.iterator();
-                while (iterator.hasNext()) {
-                    ClassNode next = iterator.next();
-                    if (ClassHelper.OBJECT_TYPE.equals(next)) {
-                        iterator.remove();
-                    }
+    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
+                        && noneMatch(tempTypes, expressionType)) { // GROOVY-9769
+                    types.add(expressionType);
                 }
-                if (arr.isEmpty()) {
-                    result = ClassHelper.OBJECT_TYPE.getPlainNodeReference();
-                } else if (arr.size() == 1) {
-                    result = arr.get(0);
+                types.addAll(tempTypes);
+
+                if (types.isEmpty()) {
+                    return OBJECT_TYPE;
+                } else if (types.size() == 1) {
+                    return types.get(0);
                 } else {
-                    result = new UnionTypeClassNode(arr.toArray(ClassNode.EMPTY_ARRAY));
+                    return new UnionTypeClassNode(types.toArray(ClassNode.EMPTY_ARRAY));
                 }
             }
         }
-        return result;
+        return expressionType;
+    }
+
+    private static boolean noneMatch(final List<ClassNode> types, final ClassNode type) {
+        for (ClassNode t : types) {
+            if (implementsInterfaceOrIsSubclassOf(t, type)) {
+                return false;
+            }
+        }
+        return true;
     }
 
     @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'
+        '''
+    }
 }