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/08/22 00:36:41 UTC
[groovy] branch GROOVY-8788-take2 created (now f9ef470f32)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-8788-take2
in repository https://gitbox.apache.org/repos/asf/groovy.git
at f9ef470f32 GROOVY-8788: STC: prefer closer parameter match over receiver-type match
This branch includes the following new commits:
new f9ef470f32 GROOVY-8788: STC: prefer closer parameter match over receiver-type match
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.
[groovy] 01/01: GROOVY-8788: STC: prefer closer parameter match over receiver-type match
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-8788-take2
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit f9ef470f321c0770aab97e918f28123a810327d6
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Aug 21 19:36:17 2022 -0500
GROOVY-8788: STC: prefer closer parameter match over receiver-type match
---
src/main/groovy/groovy/grape/GrapeIvy.groovy | 2 +-
.../transform/stc/StaticTypeCheckingSupport.java | 96 ++++++++++------------
src/test/groovy/bugs/Groovy8629Bug.groovy | 5 +-
.../groovy/transform/stc/STCAssignmentTest.groovy | 4 +-
.../classgen/asm/sc/bugs/Groovy7325Bug.groovy | 29 +++----
.../groovy/transform/DelegateTransformTest.groovy | 6 +-
6 files changed, 67 insertions(+), 75 deletions(-)
diff --git a/src/main/groovy/groovy/grape/GrapeIvy.groovy b/src/main/groovy/groovy/grape/GrapeIvy.groovy
index bfc00258f1..8a192b35b4 100644
--- a/src/main/groovy/groovy/grape/GrapeIvy.groovy
+++ b/src/main/groovy/groovy/grape/GrapeIvy.groovy
@@ -596,7 +596,7 @@ class GrapeIvy implements GrapeEngine {
// check for mutually exclusive arguments
Set<String> keys = (Set<String>) args.keySet()
keys.each { key ->
- Set<String> badArgs = MUTUALLY_EXCLUSIVE_KEYS[key]
+ Set<String> badArgs = MUTUALLY_EXCLUSIVE_KEYS.get(key)
if (badArgs && !badArgs.disjoint(keys)) {
throw new RuntimeException("Mutually exclusive arguments passed into grab: ${keys.intersect(badArgs) + key}")
}
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 4d4090fc82..6024b92e61 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -1031,47 +1031,28 @@ public abstract class StaticTypeCheckingSupport {
*
* @return zero or more results
*/
- public static List<MethodNode> chooseBestMethod(final ClassNode receiver, final Collection<MethodNode> methods, final ClassNode... argumentTypes) {
+ public static List<MethodNode> chooseBestMethod(final ClassNode receiver, Collection<MethodNode> methods, final ClassNode... argumentTypes) {
if (!asBoolean(methods)) {
return Collections.emptyList();
}
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, duckType);
-
- for (MethodNode candidate : candidates) {
- MethodNode safeNode = candidate;
- ClassNode[] safeArgs = argumentTypes;
- boolean isExtensionMethod = candidate instanceof ExtensionMethodNode;
- if (isExtensionMethod) {
- int nArgs = argumentTypes.length;
- safeArgs = new ClassNode[nArgs + 1];
- System.arraycopy(argumentTypes, 0, safeArgs, 1, nArgs);
- safeArgs[0] = receiver; // prepend self-type as first argument
- safeNode = ((ExtensionMethodNode) candidate).getExtensionMethodNode();
- }
-
- /* TODO: corner case
- class B extends A {}
- Animal foo(A a) {}
- Person foo(B b) {}
-
- B b = new B()
- Person p = foo(b)
- */
-
- ClassNode declaringClass = candidate.getDeclaringClass();
+ List<MethodNode> bestMethods = new LinkedList<>();
+ boolean duckType = receiver instanceof UnionTypeClassNode;
+ if (methods.size() > 1 && !methods.iterator().next().isConstructor())
+ methods = removeCovariantsAndInterfaceEquivalents(methods, duckType);
+
+ // phase 1: argument-parameter distance classifier
+ for (MethodNode method : methods) {
+ ClassNode declaringClass = method.getDeclaringClass();
ClassNode actualReceiver = receiver != null ? receiver : declaringClass;
Map<GenericsType, GenericsType> spec;
- if (candidate.isStatic()) {
+ if (method.isStatic()) {
spec = Collections.emptyMap(); // none visible
} else {
spec = GenericsUtils.makeDeclaringAndActualGenericsTypeMapOfExactType(declaringClass, actualReceiver);
- GenericsType[] methodGenerics = candidate.getGenericsTypes();
+ GenericsType[] methodGenerics = method.getGenericsTypes();
if (methodGenerics != null) { // GROOVY-10322: remove hidden type parameters
for (int i = 0, n = methodGenerics.length; i < n && !spec.isEmpty(); i += 1) {
for (Iterator<GenericsType> it = spec.keySet().iterator(); it.hasNext(); ) {
@@ -1080,34 +1061,49 @@ public abstract class StaticTypeCheckingSupport {
}
}
}
+ Parameter[] parameters = makeRawTypes(method.getParameters(), spec);
- Parameter[] params = makeRawTypes(safeNode.getParameters(), spec);
- int dist = measureParametersAndArgumentsDistance(params, safeArgs);
- if (dist >= 0) {
- dist += getClassDistance(declaringClass, actualReceiver);
- dist += getExtensionDistance(isExtensionMethod);
+ int dist = measureParametersAndArgumentsDistance(parameters, argumentTypes);
+ if (dist >= 0 && dist <= bestDist) {
if (dist < bestDist) {
bestDist = dist;
- bestChoices.clear();
- bestChoices.add(candidate);
- } else if (dist == bestDist) {
- bestChoices.add(candidate);
+ bestMethods.clear();
+ }
+ bestMethods.add(method);
+ }
+ }
+
+ // phase 2: receiver-provider distance classifier
+ if (bestMethods.size() > 1 && receiver != null) {
+ methods = bestMethods;
+ bestDist = Integer.MAX_VALUE;
+ bestMethods = new LinkedList<>();
+ for (MethodNode method : methods) {
+ int dist = getClassDistance(method.getDeclaringClass(), receiver);
+ if (dist <= bestDist) {
+ if (dist < bestDist) {
+ bestDist = dist;
+ bestMethods.clear();
+ }
+ bestMethods.add(method);
}
}
}
- if (bestChoices.size() > 1 && !duckType) {
- // GROOVY-6849: prefer extension method in case of ambiguity
- List<MethodNode> onlyExtensionMethods = new LinkedList<>();
- for (MethodNode choice : bestChoices) {
- if (choice instanceof ExtensionMethodNode) {
- onlyExtensionMethods.add(choice);
+
+ // phase 3: prefer extension method in case of tie
+ if (bestMethods.size() > 1 && !duckType) {
+ List<MethodNode> extensionMethods = new LinkedList<>();
+ for (MethodNode method : bestMethods) {
+ if (method instanceof ExtensionMethodNode) {
+ extensionMethods.add(method);
}
}
- if (onlyExtensionMethods.size() == 1) {
- return onlyExtensionMethods;
+ if (extensionMethods.size() == 1) {
+ bestMethods = extensionMethods;
}
}
- return bestChoices;
+
+ return bestMethods;
}
private static int measureParametersAndArgumentsDistance(final Parameter[] parameters, final ClassNode[] argumentTypes) {
@@ -1169,10 +1165,6 @@ public abstract class StaticTypeCheckingSupport {
return getDistance(actualReceiverForDistance, declaringClassForDistance);
}
- private static int getExtensionDistance(final boolean isExtensionMethodNode) {
- return isExtensionMethodNode ? 0 : 1;
- }
-
private static Parameter[] makeRawTypes(final Parameter[] parameters, final Map<GenericsType, GenericsType> genericsPlaceholderAndTypeMap) {
return Arrays.stream(parameters).map(param -> {
String name = param.getType().getUnresolvedName();
diff --git a/src/test/groovy/bugs/Groovy8629Bug.groovy b/src/test/groovy/bugs/Groovy8629Bug.groovy
index 3b56d81bc1..643cca4219 100644
--- a/src/test/groovy/bugs/Groovy8629Bug.groovy
+++ b/src/test/groovy/bugs/Groovy8629Bug.groovy
@@ -62,8 +62,8 @@ public class Groovy8629Bug extends GroovyTestCase {
@Override
IntegerPair next() {
String key = keyIterator.next()
- IntegerPair comp = new IntegerPair(m1[key], m2[key])
- return comp
+ IntegerPair pair = new IntegerPair(m1.get(key), m2.get(key))
+ return pair
}
@Override
@@ -87,5 +87,4 @@ public class Groovy8629Bug extends GroovyTestCase {
'''
}
-
}
diff --git a/src/test/groovy/transform/stc/STCAssignmentTest.groovy b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
index c573582bcc..e537f2fd1c 100644
--- a/src/test/groovy/transform/stc/STCAssignmentTest.groovy
+++ b/src/test/groovy/transform/stc/STCAssignmentTest.groovy
@@ -1251,8 +1251,8 @@ class STCAssignmentTest extends StaticTypeCheckingTestCase {
else if (value instanceof GString) {
value = value.toString()
}
- if (mvm[name]) {
- mvm[name].add value
+ if (mvm.containsKey(name)) {
+ mvm.get(name).add(value)
} else {
mvm.put(name, [value])
}
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy
index 49ea6ed365..a50ae36ece 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/bugs/Groovy7325Bug.groovy
@@ -27,22 +27,22 @@ class Groovy7325Bug extends StaticTypeCheckingTestCase implements StaticCompilat
void testGenericIdentityWithClosure() {
assertScript '''
-public static <T> T identity(T self) { self }
-
-@ASTTest(phase=INSTRUCTION_SELECTION,value={
- assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == Integer_TYPE
-})
-Integer i = identity(2)
-
-@ASTTest(phase=INSTRUCTION_SELECTION,value={
- assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == CLOSURE_TYPE
-})
-Closure c = identity {'foo'}
-'''
+ static <T> T itself(T self) { self }
+
+ @ASTTest(phase=INSTRUCTION_SELECTION,value={
+ assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == Integer_TYPE
+ })
+ Integer i = itself(2)
+
+ @ASTTest(phase=INSTRUCTION_SELECTION,value={
+ assert node.rightExpression.getNodeMetaData(INFERRED_TYPE) == CLOSURE_TYPE
+ })
+ Closure c = itself {'foo'}
+ '''
}
void testShouldNotThrowIllegalAccessToProtectedData() {
- shouldFailWithMessages('''
+ shouldFailWithMessages '''
class Test {
final Set<String> HISTORY = [] as HashSet
@@ -53,6 +53,7 @@ Closure c = identity {'foo'}
Test test = new Test()
println test.history
- ''', 'Method clone is protected in java.lang.Object')
+ ''',
+ 'Method clone is protected in java.lang.Object'
}
}
diff --git a/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy b/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy
index 7ad281e6ce..1e20c733f9 100644
--- a/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/DelegateTransformTest.groovy
@@ -112,12 +112,12 @@ final class DelegateTransformTest {
def ms = new MapSet()
assert ms.size() == 1
- assert ms.toString() == '{a=1} [2, 3, 4]'
+ assert ms.toString() == '[a:1] [2, 3, 4]'
ms.remove(3)
assert ms.size() == 1
- assert ms.toString() == '{a=1} [2, 4]'
+ assert ms.toString() == '[a:1] [2, 4]'
ms.clear()
- assert ms.toString() == '{a=1} []'
+ assert ms.toString() == '[a:1] []'
'''
}