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 18:05:39 UTC
[groovy] branch GROOVY-8965 updated (dfe1a0eac0 -> 206ce5b855)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-8965
in repository https://gitbox.apache.org/repos/asf/groovy.git
discard dfe1a0eac0 GROOVY-8965, GROOVY-10180, GROOVY-10668: STC: multiple `instanceof`
new 206ce5b855 GROOVY-8965, GROOVY-10180, GROOVY-10668: STC: multiple `instanceof`
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (dfe1a0eac0)
\
N -- N -- N refs/heads/GROOVY-8965 (206ce5b855)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../transform/stc/StaticTypeCheckingSupport.java | 14 ++++++------
.../transform/stc/StaticTypeCheckingVisitor.java | 26 ++++++++++++----------
src/test/groovy/transform/stc/BugsSTCTest.groovy | 5 +----
.../asm/sc/bugs/ReproducibleBytecodeBugs.groovy | 3 +--
4 files changed, 23 insertions(+), 25 deletions(-)
[groovy] 01/01: GROOVY-8965, GROOVY-10180, GROOVY-10668: STC: multiple `instanceof`
Posted by em...@apache.org.
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 206ce5b8554362ffd4c963b4f8e228f53f105959
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 ae0acf3d27..a0bc295c95 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;
}
@@ -4702,37 +4716,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)")
}
}
}