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