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 2022/07/12 21:51:07 UTC
[groovy] 01/03: GROOVY-8965, GROOVY-10180, GROOVY-10668: STC: multiple `instanceof`
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit c2fcbc288013794177435a273c3efb596d1fe268
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Jul 7 15:39:41 2022 -0500
GROOVY-8965, GROOVY-10180, GROOVY-10668: STC: multiple `instanceof`
---
.../transform/stc/StaticTypeCheckingSupport.java | 19 +++---
.../transform/stc/StaticTypeCheckingVisitor.java | 68 +++++++++++-----------
src/test/groovy/transform/stc/BugsSTCTest.groovy | 43 ++++++--------
.../transform/stc/TypeInferenceSTCTest.groovy | 2 +-
.../groovy/parser/antlr4/util/AstDumper.groovy | 20 +++----
.../asm/sc/TypeInferenceStaticCompileTest.groovy | 30 +---------
.../asm/sc/bugs/ReproducibleBytecodeBugs.groovy | 65 ++++++++++-----------
7 files changed, 105 insertions(+), 142 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index f68572e192..18093b95f5 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -903,8 +903,7 @@ public abstract class StaticTypeCheckingSupport {
}
if (result) return true;
} else if (superOrInterface instanceof UnionTypeClassNode) {
- UnionTypeClassNode union = (UnionTypeClassNode) superOrInterface;
- for (ClassNode delegate : union.getDelegates()) {
+ for (ClassNode delegate : ((UnionTypeClassNode) superOrInterface).getDelegates()) {
if (implementsInterfaceOrIsSubclassOf(type, delegate)) return true;
}
}
@@ -1040,8 +1039,9 @@ public abstract class StaticTypeCheckingSupport {
int bestDist = Integer.MAX_VALUE;
List<MethodNode> bestChoices = new LinkedList<>();
+ boolean duckType = receiver instanceof UnionTypeClassNode; // GROOVY-8965: type disjunction
boolean noCulling = methods.size() <= 1 || "<init>".equals(methods.iterator().next().getName());
- Iterable<MethodNode> candidates = noCulling ? methods : removeCovariantsAndInterfaceEquivalents(methods);
+ Iterable<MethodNode> candidates = noCulling ? methods : removeCovariantsAndInterfaceEquivalents(methods, duckType);
for (MethodNode candidate : candidates) {
MethodNode safeNode = candidate;
@@ -1096,7 +1096,7 @@ public abstract class StaticTypeCheckingSupport {
}
}
}
- if (bestChoices.size() > 1) {
+ if (bestChoices.size() > 1 && !duckType) {
// GROOVY-6849: prefer extension method in case of ambiguity
List<MethodNode> onlyExtensionMethods = new LinkedList<>();
for (MethodNode choice : bestChoices) {
@@ -1195,9 +1195,8 @@ public abstract class StaticTypeCheckingSupport {
return raw;
}
- private static List<MethodNode> removeCovariantsAndInterfaceEquivalents(final Collection<MethodNode> collection) {
- List<MethodNode> toBeRemoved = new ArrayList<>();
- List<MethodNode> list = new ArrayList<>(new LinkedHashSet<>(collection));
+ private static List<MethodNode> removeCovariantsAndInterfaceEquivalents(final Collection<MethodNode> collection, final boolean disjoint) {
+ List<MethodNode> list = new ArrayList<>(new LinkedHashSet<>(collection)), toBeRemoved = new ArrayList<>();
for (int i = 0, n = list.size(); i < n - 1; i += 1) {
MethodNode one = list.get(i);
if (toBeRemoved.contains(one)) continue;
@@ -1228,11 +1227,9 @@ public abstract class StaticTypeCheckingSupport {
} else if (!oneDC.equals(twoDC)) {
if (ParameterUtils.parametersEqual(one.getParameters(), two.getParameters())) {
// GROOVY-6882, GROOVY-6970: drop overridden or interface equivalent method
- if (twoDC.isInterface() ? oneDC.implementsInterface(twoDC)
- : oneDC.isDerivedFrom(twoDC)) {
+ if (twoDC.isInterface() ? oneDC.implementsInterface(twoDC) : oneDC.isDerivedFrom(twoDC)) {
toBeRemoved.add(two);
- } else if (oneDC.isInterface() ? twoDC.isInterface()
- : twoDC.isDerivedFrom(oneDC)) {
+ } else if (oneDC.isInterface() ? (disjoint ? twoDC.implementsInterface(oneDC) : twoDC.isInterface()) : twoDC.isDerivedFrom(oneDC)) {
toBeRemoved.add(one);
}
}
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 8c45a0b7d9..f56bc8eaf3 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -3503,7 +3503,20 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (areCategoryMethodCalls(mn, name, args)) {
addCategoryMethodCallError(call);
}
- mn = disambiguateMethods(mn, chosenReceiver != null ? chosenReceiver.getType() : null, args, call);
+
+ {
+ ClassNode obj = chosenReceiver != null ? chosenReceiver.getType() : null;
+ if (mn.size() > 1 && obj instanceof UnionTypeClassNode) { // GROOVY-8965: support duck-typing using dynamic resolution
+ ClassNode returnType = mn.stream().map(MethodNode::getReturnType).reduce(WideningCategories::lowestUpperBound).get();
+ call.putNodeMetaData(DYNAMIC_RESOLUTION, returnType);
+
+ MethodNode tmp = new MethodNode(name, 0, returnType, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
+ tmp.setDeclaringClass(obj); // for storeTargetMethod
+ mn = Collections.singletonList(tmp);
+ } else {
+ mn = disambiguateMethods(mn, obj, args, call);
+ }
+ }
if (mn.size() == 1) {
MethodNode targetMethodCandidate = mn.get(0);
@@ -3752,7 +3765,8 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
/**
- * Given an object expression (a receiver expression), generate the list of potential receiver types.
+ * Given an object expression (a message receiver expression), generate list
+ * of possible types.
*
* @param objectExpression the receiver expression
* @return the list of types the receiver may be
@@ -3762,23 +3776,20 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
List<Receiver<String>> owners = new ArrayList<>();
if (typeCheckingContext.delegationMetadata != null
&& objectExpression instanceof VariableExpression
- && ((VariableExpression) objectExpression).getName().equals("owner")
+ && ((Variable) objectExpression).getName().equals("owner")
&& /*isNested:*/typeCheckingContext.delegationMetadata.getParent() != null) {
List<Receiver<String>> enclosingClass = Collections.singletonList(
Receiver.make(typeCheckingContext.getEnclosingClassNode()));
addReceivers(owners, enclosingClass, typeCheckingContext.delegationMetadata.getParent(), "owner.");
} else {
- if (!typeCheckingContext.temporaryIfBranchTypeInformation.isEmpty()) {
- List<ClassNode> potentialReceiverType = getTemporaryTypesForExpression(objectExpression);
- if (potentialReceiverType != null && !potentialReceiverType.isEmpty()) {
- for (ClassNode node : potentialReceiverType) {
- owners.add(Receiver.make(node));
- }
- }
+ List<ClassNode> temporaryTypes = getTemporaryTypesForExpression(objectExpression);
+ int temporaryTypesCount = (temporaryTypes != null ? temporaryTypes.size() : 0);
+ if (temporaryTypesCount > 0) { // GROOVY-8965, GROOVY-10180, GROOVY-10668
+ owners.add(Receiver.make(lowestUpperBound(temporaryTypes)));
}
if (typeCheckingContext.lastImplicitItType != null
&& objectExpression instanceof VariableExpression
- && ((VariableExpression) objectExpression).getName().equals("it")) {
+ && ((Variable) objectExpression).getName().equals("it")) {
owners.add(Receiver.make(typeCheckingContext.lastImplicitItType));
}
if (isClassClassNodeWrappingConcreteType(receiver)) {
@@ -3798,6 +3809,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
}
}
+ if (temporaryTypesCount > 1 && !(objectExpression instanceof VariableExpression)) {
+ owners.add(Receiver.make(new UnionTypeClassNode(temporaryTypes.toArray(ClassNode.EMPTY_ARRAY))));
+ }
}
return owners;
}
@@ -4717,37 +4731,25 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return null;
}
- private List<MethodNode> disambiguateMethods(List<MethodNode> methods, final ClassNode receiver, final ClassNode[] argTypes, final Expression call) {
- if (methods.size() > 1 && receiver != null && argTypes != null) {
+ private List<MethodNode> disambiguateMethods(List<MethodNode> methods, final ClassNode receiver, final ClassNode[] arguments, final Expression call) {
+ if (methods.size() > 1 && receiver != null && arguments != null) {
List<MethodNode> filteredWithGenerics = new LinkedList<>();
- for (MethodNode methodNode : methods) {
- if (typeCheckMethodsWithGenerics(receiver, argTypes, methodNode)) {
- if ((methodNode.getModifiers() & Opcodes.ACC_BRIDGE) == 0) {
- filteredWithGenerics.add(methodNode);
- }
+ for (MethodNode method : methods) {
+ if (typeCheckMethodsWithGenerics(receiver, arguments, method)
+ && (method.getModifiers() & Opcodes.ACC_BRIDGE) == 0) {
+ filteredWithGenerics.add(method);
}
}
if (filteredWithGenerics.size() == 1) {
return filteredWithGenerics;
}
+
methods = extension.handleAmbiguousMethods(methods, call);
}
- if (methods.size() > 1) {
- if (call instanceof MethodCall) {
- List<MethodNode> methodNodeList = new LinkedList<>();
-
- String methodName = ((MethodCall) call).getMethodAsString();
-
- for (MethodNode methodNode : methods) {
- if (!methodNode.getName().equals(methodName)) {
- continue;
- }
- methodNodeList.add(methodNode);
- }
-
- methods = methodNodeList;
- }
+ if (methods.size() > 1 && call instanceof MethodCall) {
+ String methodName = ((MethodCall) call).getMethodAsString();
+ methods = methods.stream().filter(m -> m.getName().equals(methodName)).collect(Collectors.toList());
}
return methods;
diff --git a/src/test/groovy/transform/stc/BugsSTCTest.groovy b/src/test/groovy/transform/stc/BugsSTCTest.groovy
index 45f0cd4a91..591a31d724 100644
--- a/src/test/groovy/transform/stc/BugsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/BugsSTCTest.groovy
@@ -685,18 +685,16 @@ Printer
assertScript '''
interface A { void m() }
interface B { void m() }
- interface C extends A, B {}
-
- class D {
- D(C c) {
- c.m()
- }
+ interface C extends A,B {
}
- class CImpl implements C {
- void m() { }
+ class Impl implements C {
+ void m() {}
}
- new D(new CImpl())
+ void test(C c) {
+ c.m()
+ }
+ test(new Impl())
'''
}
@@ -704,16 +702,16 @@ Printer
assertScript '''
interface A { void m() }
interface B { void m() }
- class C implements A,B {
- void m() {}
+ interface C extends A,B {
}
- class D {
- D(C c) {
- c.m()
- }
+ class Impl implements C,A,B {
+ void m() {}
}
- new D(new C())
+ void test(C c) {
+ c.m()
+ }
+ test(new Impl())
'''
}
@@ -721,17 +719,14 @@ Printer
assertScript '''
interface A { void m() }
interface B { void m() }
- interface C extends A,B {}
- class CImpl implements C, A,B {
+ class C implements A,B {
void m() {}
}
- class D {
- D(C c) {
- c.m()
- }
- }
- new D(new CImpl())
+ void test(C c) {
+ c.m()
+ }
+ test(new C())
'''
}
diff --git a/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy b/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
index 93d67cd829..6f1efc8dc7 100644
--- a/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
+++ b/src/test/groovy/transform/stc/TypeInferenceSTCTest.groovy
@@ -365,7 +365,7 @@ class TypeInferenceSTCTest extends StaticTypeCheckingTestCase {
thing.addElement(2) // 'addElement' only in Stack
}
if (thing instanceof Deque || thing instanceof Stack) {
- assert thing.peek() in 1..2 // 'peek' in ArrayDeque and Stack but not LUB
+ assert thing.peek() in 1..2 // 'peek' in Deque and Stack but not LUB
}
}
test(new Stack())
diff --git a/src/test/org/apache/groovy/parser/antlr4/util/AstDumper.groovy b/src/test/org/apache/groovy/parser/antlr4/util/AstDumper.groovy
index 165f2f26eb..f67e01e6c6 100644
--- a/src/test/org/apache/groovy/parser/antlr4/util/AstDumper.groovy
+++ b/src/test/org/apache/groovy/parser/antlr4/util/AstDumper.groovy
@@ -641,8 +641,7 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
@Override
void visitMethodCallExpression(MethodCallExpression expression) {
-
- Expression objectExp = expression.getObjectExpression()
+ Expression objectExp = expression.objectExpression
if (objectExp instanceof VariableExpression) {
visitVariableExpression(objectExp, false)
} else {
@@ -655,25 +654,23 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
print '?'
}
print '.'
- Expression method = expression.getMethod()
+ Expression method = expression.method
if (method instanceof ConstantExpression) {
visitConstantExpression(method, true)
} else {
method.visit(this)
}
- expression.getArguments().visit(this)
+ expression.arguments.visit(this)
}
@Override
void visitStaticMethodCallExpression(StaticMethodCallExpression expression) {
print expression?.ownerType?.name + '.' + expression?.method
- if (expression?.arguments instanceof VariableExpression || expression?.arguments instanceof MethodCallExpression) {
- print '('
- expression?.arguments?.visit this
- print ')'
- } else {
- expression?.arguments?.visit this
- }
+ boolean parens = expression?.arguments instanceof VariableExpression
+ || expression?.arguments instanceof MethodCallExpression
+ if (parens) print '('
+ expression?.arguments?.visit this
+ if (parens) print ')'
}
@Override
@@ -1132,5 +1129,4 @@ class AstNodeToScriptVisitor implements CompilationUnit.IPrimaryClassNodeOperati
}
return true
}
-
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy
index b1ff1521c5..b7ae38f82b 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/TypeInferenceStaticCompileTest.groovy
@@ -26,33 +26,9 @@ import groovy.transform.stc.TypeInferenceSTCTest
*/
class TypeInferenceStaticCompileTest extends TypeInferenceSTCTest implements StaticCompilationTestSupport {
- @Override @NotYetImplemented
+ @Override
+ @NotYetImplemented
void testInstanceOf9() {
- super.testInstanceOf9()
- }
-
- @Override @NotYetImplemented
- void testMultipleInstanceOf1() {
- super.testMultipleInstanceOf1()
- }
-
- @Override @NotYetImplemented
- void testMultipleInstanceOf2() {
- super.testMultipleInstanceOf2()
- }
-
- @Override @NotYetImplemented
- void testMultipleInstanceOf3() {
- super.testMultipleInstanceOf3()
- }
-
- @Override @NotYetImplemented
- void testMultipleInstanceOf4() {
- super.testMultipleInstanceOf4()
- }
-
- @Override @NotYetImplemented
- void testMultipleInstanceOf5() {
- super.testMultipleInstanceOf5()
+ super.testInstanceOf9() // GROOVY-7971
}
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/ReproducibleBytecodeBugs.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/ReproducibleBytecodeBugs.groovy
index 7ca44be0f8..c4586791c0 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/ReproducibleBytecodeBugs.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/ReproducibleBytecodeBugs.groovy
@@ -16,35 +16,34 @@
* 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 ReproducibleBytecodeBugs extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
+
// GROOVY-8142
- void testShouldNotProduceDeterministicBytecode() {
+ void testShouldProduceDeterministicBytecode() {
100.times {
assertScript '''
- interface Project {
- File file()
- }
- interface FileOperations {
- File file()
- }
- interface ProjectInternal extends Project, FileOperations {
- }
+ interface Project {
+ File file()
+ }
+ interface FileOperations {
+ File file()
+ }
+ interface ProjectInternal extends Project, FileOperations {
+ }
- class Check {
- void test(ProjectInternal p) {
- def f = p.file()
+ class Check {
+ void test(ProjectInternal p) {
+ def f = p.file()
+ }
}
- }
- def c = new Check()
- '''
+ def c = new Check()
+ '''
assert astTrees['Check'][1].contains('INVOKEINTERFACE FileOperations.file ()Ljava/io/File;') : "Incorrect bytecode found in iteration $it"
}
@@ -54,20 +53,20 @@ class ReproducibleBytecodeBugs extends StaticTypeCheckingTestCase implements Sta
void testShouldAlwaysAddClosureSharedVariablesInSameOrder() {
100.times {
assertScript '''
- class Check {
- void test() {
- def xx = true
- def moot = "bar"
- def kr = [:]
- def zorg = []
- def cl = {
- def (x,y,z,t) = [xx, moot, kr , zorg]
+ class Check {
+ void test() {
+ def xx = true
+ def moot = "bar"
+ def kr = [:]
+ def zorg = []
+ def cl = {
+ def (x,y,z,t) = [xx, moot, kr , zorg]
+ }
}
}
- }
- def c = new Check()
- '''
+ def c = new Check()
+ '''
def bytecode = astTrees['Check$_test_closure1'][1]
assertOrdered it, bytecode,
@@ -75,17 +74,15 @@ class ReproducibleBytecodeBugs extends StaticTypeCheckingTestCase implements Sta
'PUTFIELD Check$_test_closure1.moot',
'PUTFIELD Check$_test_closure1.kr',
'PUTFIELD Check$_test_closure1.zorg'
-
}
}
-
- private static void assertOrdered(int iteration, String bytecode, String... elements) {
+ private static void assertOrdered(int iteration, String bytecode, String... strings) {
int start = 0
- elements.eachWithIndex { it, i ->
- start = bytecode.indexOf(it, start)
+ strings.eachWithIndex { string, index ->
+ start = bytecode.indexOf(string, start)
if (start == -1) {
- throw new AssertionError("Iteration $iteration - Element [$it] not found in order (expected to find it at index $i)")
+ throw new AssertionError("Iteration $iteration - Element [$string] not found in order (expected to find it at index $index)")
}
}
}