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:22 UTC
[groovy] branch GROOVY-8965 updated (28a2dc2d50 -> dfe1a0eac0)
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 28a2dc2d50 GROOVY-8965, GROOVY-10180, GROOVY-10668: STC:
new dfe1a0eac0 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 (28a2dc2d50)
\
N -- N -- N refs/heads/GROOVY-8965 (dfe1a0eac0)
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 | 11 ++--
.../transform/stc/StaticTypeCheckingVisitor.java | 11 ++++
src/test/groovy/transform/stc/BugsSTCTest.groovy | 48 ++++++++-------
.../transform/stc/TypeInferenceSTCTest.groovy | 2 -
.../asm/sc/TypeInferenceStaticCompileTest.groovy | 15 +----
.../asm/sc/bugs/ReproducibleBytecodeBugs.groovy | 68 +++++++++++-----------
6 files changed, 74 insertions(+), 81 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 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)")
}
}
}