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/11 15:41:23 UTC
[groovy] 01/01: 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 GROOVY-8965
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit dfe1a0eac084db60588e17a5a6488cbc893b896e
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 | 11 ++--
.../transform/stc/StaticTypeCheckingVisitor.java | 64 ++++++++++----------
src/test/groovy/transform/stc/BugsSTCTest.groovy | 48 ++++++++-------
.../transform/stc/TypeInferenceSTCTest.groovy | 2 +-
.../groovy/parser/antlr4/util/AstDumper.groovy | 20 +++----
.../asm/sc/TypeInferenceStaticCompileTest.groovy | 30 +---------
.../asm/sc/bugs/ReproducibleBytecodeBugs.groovy | 68 +++++++++++-----------
7 files changed, 104 insertions(+), 139 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..10925b65fa 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -1196,8 +1196,7 @@ public abstract class StaticTypeCheckingSupport {
}
private static List<MethodNode> removeCovariantsAndInterfaceEquivalents(final Collection<MethodNode> collection) {
- List<MethodNode> toBeRemoved = new ArrayList<>();
- List<MethodNode> list = new ArrayList<>(new LinkedHashSet<>(collection));
+ 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;
@@ -1227,12 +1226,10 @@ 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)) {
+ // GROOVY-6882: drop overridden or interface equivalent method
+ 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() ? twoDC.implementsInterface(oneDC) : 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 ae0acf3d27..77c9712763 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -3752,7 +3752,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 +3763,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 +3796,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;
}
@@ -4702,37 +4703,36 @@ 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) {
+ // GROOVY-6970, GROOVY-8142, GROOVY-8965, ...
+ if (receiver instanceof UnionTypeClassNode) {
+ ClassNode returnType = methods.stream().map(MethodNode::getReturnType)
+ .reduce(WideningCategories::lowestUpperBound).get();
+ ((ASTNode) call).putNodeMetaData(StaticTypesMarker.DYNAMIC_RESOLUTION, returnType);
+
+ MethodNode dummyMethod = new MethodNode(((MethodCall) call).getMethodAsString(), 0, returnType, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, EmptyStatement.INSTANCE);
+ dummyMethod.setDeclaringClass(receiver);
+ return Collections.singletonList(dummyMethod);
+ }
+
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..8030a1d3b3 100644
--- a/src/test/groovy/transform/stc/BugsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/BugsSTCTest.groovy
@@ -18,6 +18,8 @@
*/
package groovy.transform.stc
+import groovy.test.NotYetImplemented
+
import static groovy.test.GroovyAssert.isAtLeastJdk
/**
@@ -680,40 +682,39 @@ Printer
'''
}
- // GROOVY-6970
+ @NotYetImplemented // GROOVY-6970
void testShouldBeAbleToChooseBetweenTwoEquivalentInterfaceMethods1() {
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())
'''
}
+ @NotYetImplemented
void testShouldBeAbleToChooseBetweenTwoEquivalentInterfaceMethods2() {
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 +722,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..a7181fa316 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,35 @@
* specific language governing permissions and limitations
* under the License.
*/
-
-
package org.codehaus.groovy.classgen.asm.sc.bugs
+import groovy.test.NotYetImplemented
import groovy.transform.stc.StaticTypeCheckingTestCase
import org.codehaus.groovy.classgen.asm.sc.StaticCompilationTestSupport
class ReproducibleBytecodeBugs extends StaticTypeCheckingTestCase implements StaticCompilationTestSupport {
- // GROOVY-8142
- void testShouldNotProduceDeterministicBytecode() {
+
+ @NotYetImplemented // GROOVY-8142
+ 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 +54,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 +75,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)")
}
}
}