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 2021/01/19 21:37:26 UTC
[groovy] branch GROOVY-9881 updated (3f40e59 -> 4c4edb9)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY-9881
in repository https://gitbox.apache.org/repos/asf/groovy.git.
discard 3f40e59 GROOVY-9881: prefer same parameter count for SAM-type overload selection
new 4c4edb9 GROOVY-9881: prefer same parameter count for SAM-type overload selection
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 (3f40e59)
\
N -- N -- N refs/heads/GROOVY-9881 (4c4edb9)
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:
src/test/groovy/transform/stc/ClosuresSTCTest.groovy | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
[groovy] 01/01: GROOVY-9881: prefer same parameter count for
SAM-type overload selection
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY-9881
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 4c4edb9e7958917d32e3865b042c005adc9c92d1
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jan 19 14:20:35 2021 -0600
GROOVY-9881: prefer same parameter count for SAM-type overload selection
---
.../codehaus/groovy/runtime/MetaClassHelper.java | 22 +++++++-
.../transform/stc/StaticTypeCheckingSupport.java | 12 +++--
.../transform/stc/StaticTypeCheckingVisitor.java | 17 +++---
.../groovy/transform/stc/ClosuresSTCTest.groovy | 61 +++++++++++++++++-----
src/test/groovy/transform/stc/LambdaTest.groovy | 35 ++++++++++++-
5 files changed, 122 insertions(+), 25 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java b/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java
index 73b81dc..812618e 100644
--- a/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java
+++ b/src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java
@@ -33,6 +33,7 @@ import org.codehaus.groovy.util.FastArray;
import java.lang.reflect.Array;
import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.math.BigDecimal;
import java.math.BigInteger;
@@ -292,6 +293,23 @@ public class MetaClassHelper {
return Math.max(max, superClassMax);
}
+ private static int getParameterCount(final Class<?> closureOrLambdaClass) {
+ int parameterCount = -2;
+ if (GeneratedClosure.class.isAssignableFrom(closureOrLambdaClass)) {
+ // determine parameter count from generated "doCall" method(s)
+ for (Method m : closureOrLambdaClass.getDeclaredMethods()) {
+ if (m.getName().equals("doCall")) {
+ if (parameterCount != -2) {
+ parameterCount = -1; // 0 or 1
+ } else {
+ parameterCount = m.getParameterCount();
+ }
+ }
+ }
+ }
+ return parameterCount;
+ }
+
private static long calculateParameterDistance(final Class<?> argument, final CachedClass parameter) {
/*
* note: when shifting with 32 bit, you should only shift on a long. If you do
@@ -325,8 +343,10 @@ public class MetaClassHelper {
objectDistance += 4;
}
+ Method sam;
for (Class<?> c = ReflectionCache.autoboxType(argument); c != null && c != parameterClass; c = c.getSuperclass()) {
- if (c == Closure.class && parameterClass.isInterface() && getSAMMethod(parameterClass) != null) {
+ if (c == Closure.class && parameterClass.isInterface() && (sam = getSAMMethod(parameterClass)) != null) {
+ if (getParameterCount(argument) == sam.getParameterCount()) objectDistance -= 1; // GROOVY-9881
objectDistance += 5; // ahead of Object but behind GroovyObjectSupport
break;
}
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 ff36a20..072287b 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -20,7 +20,6 @@ package org.codehaus.groovy.transform.stc;
import org.apache.groovy.util.Maps;
import org.codehaus.groovy.GroovyBugError;
-import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.GenericsType;
import org.codehaus.groovy.ast.GenericsType.GenericsTypeName;
@@ -93,6 +92,7 @@ import static org.codehaus.groovy.ast.ClassHelper.boolean_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.byte_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.char_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.double_TYPE;
+import static org.codehaus.groovy.ast.ClassHelper.findSAM;
import static org.codehaus.groovy.ast.ClassHelper.float_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.getUnwrapper;
import static org.codehaus.groovy.ast.ClassHelper.getWrapper;
@@ -910,14 +910,18 @@ public abstract class StaticTypeCheckingSupport {
if (receiver.isArray()) {
dist += 256; // GROOVY-5114: Object[] vs Object
}
- if (compare.isInterface()) {
+ if (compare.isInterface()) { MethodNode sam;
if (receiver.implementsInterface(compare)) {
return dist + getMaximumInterfaceDistance(receiver, compare);
- } else if (receiver.equals(CLOSURE_TYPE) && isSAMType(compare)) {
+ } else if (receiver.equals(CLOSURE_TYPE) && (sam = findSAM(compare)) != null) {
+ // GROOVY-9881: in case of multiple overloads, give preference to equal parameter count
+ Integer closureParamCount = receiver.getNodeMetaData(StaticTypesMarker.CLOSURE_ARGUMENTS);
+ if (closureParamCount != null && closureParamCount == sam.getParameters().length) dist -= 1;
+
return dist + 13; // GROOVY-9852: @FunctionalInterface vs Object
}
}
- ClassNode cn = isPrimitiveType(receiver) && !isPrimitiveType(compare) ? ClassHelper.getWrapper(receiver) : receiver;
+ ClassNode cn = isPrimitiveType(receiver) && !isPrimitiveType(compare) ? getWrapper(receiver) : receiver;
while (cn != null && !cn.equals(compare)) {
cn = cn.getSuperClass();
dist += 1;
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 cf59147..a401ed1 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -4897,13 +4897,18 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return ((Parameter) exp).getOriginType();
}
if (exp instanceof ClosureExpression) {
- ClassNode irt = getInferredReturnType(exp);
- if (irt != null) {
- irt = wrapTypeIfNecessary(irt);
- ClassNode result = CLOSURE_TYPE.getPlainNodeReference();
- result.setGenericsTypes(new GenericsType[]{new GenericsType(irt)});
- return result;
+ ClassNode type = CLOSURE_TYPE.getPlainNodeReference();
+ ClassNode returnType = getInferredReturnType(exp);
+ if (returnType != null) {
+ type.setGenericsTypes(new GenericsType[]{
+ new GenericsType(wrapTypeIfNecessary(returnType))
+ });
}
+ Parameter[] parameters = ((ClosureExpression) exp).getParameters();
+ int nParameters = parameters == null ? 0
+ : parameters.length == 0 ? -1 : parameters.length;
+ type.putNodeMetaData(CLOSURE_ARGUMENTS, nParameters);
+ return type;
} else if (exp instanceof MethodCall) {
MethodNode target = exp.getNodeMetaData(DIRECT_METHOD_CALL_TARGET);
if (target != null) {
diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
index 3fa8cd9..0e1ad4b 100644
--- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
+++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
@@ -388,6 +388,55 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
}
}
+ void testSAMsInMethodSelection2() {
+ shouldFailWithMessages '''
+ interface One { void m() }
+ interface Two { void m() }
+ def foo(One one) { one.m() }
+ def foo(Two two) { two.m() }
+ foo {
+ print 'bar'
+ }
+ ''', 'Reference to method is ambiguous. Cannot choose between'
+
+ ['', 'x, y ->'].each { params ->
+ shouldFailWithMessages """
+ import java.util.function.Function
+ import java.util.function.Supplier
+ def foo(Function f) { f.apply(0) }
+ def foo(Supplier s) { s.get() }
+ foo { $params 'bar' }
+ """, 'Reference to method is ambiguous. Cannot choose between'
+ }
+ }
+
+ // GROOVY-9881
+ void testSAMsInMethodSelection3() {
+ // Closure implements both and Runnable is "closer"
+ assertScript '''
+ import java.util.concurrent.Callable
+ def foo(Callable c) { 'call' }
+ def foo(Runnable r) { 'run' }
+ def which = foo {
+ print 'bar'
+ }
+ assert which == 'run'
+ '''
+
+ ['->', 'x ->'].each { params ->
+ assertScript """
+ import java.util.function.Function
+ import java.util.function.Supplier
+ def foo(Function f) { f.apply(0) }
+ def foo(Supplier s) { s.get() }
+ def ret = foo { $params
+ 'bar'
+ }
+ assert ret == 'bar'
+ """
+ }
+ }
+
void testSAMVariable() {
assertScript """
interface SAM { def foo(); }
@@ -491,18 +540,6 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
'''
}
- void testAmbiguousSAMOverload() {
- shouldFailWithMessages '''
- interface Sammy { def sammy() }
- interface Sam { def sam() }
- def method(Sam sam) { sam.sam() }
- def method(Sammy sammy) { sammy.sammy() }
- method {
- println 'foo'
- }
- ''', 'Reference to method is ambiguous. Cannot choose between'
- }
-
void testSAMType() {
assertScript """
interface Foo {int foo()}
diff --git a/src/test/groovy/transform/stc/LambdaTest.groovy b/src/test/groovy/transform/stc/LambdaTest.groovy
index af9458c..33c6078 100644
--- a/src/test/groovy/transform/stc/LambdaTest.groovy
+++ b/src/test/groovy/transform/stc/LambdaTest.groovy
@@ -336,6 +336,7 @@ final class LambdaTest {
'''
}
+ @Test
void testFunctionWithLocalVariables4() {
assertScript '''
import groovy.transform.CompileStatic
@@ -833,6 +834,36 @@ final class LambdaTest {
'''
}
+ @Test // GROOVY-9881
+ void testFunctionalInterface4() {
+ assertScript '''
+ import java.util.function.*
+
+ class Value<V> {
+ final V val
+ Value(V v) {
+ this.val = v
+ }
+ String toString() {
+ val as String
+ }
+ def <T> Value<T> replace(Supplier<T> supplier) {
+ new Value<>(supplier.get())
+ }
+ def <T> Value<T> replace(Function<? super V, ? extends T> function) {
+ new Value(function.apply(val))
+ }
+ }
+
+ @groovy.transform.CompileStatic
+ void test() {
+ assert new Value(123).replace(() -> 'foo').toString() == 'foo'
+ assert new Value(123).replace((Integer v) -> 'bar').toString() == 'bar'
+ }
+ test()
+ '''
+ }
+
@Test
void testFunctionWithUpdatingLocalVariable() {
assertScript '''
@@ -1813,8 +1844,8 @@ final class LambdaTest {
'''
}
- @Test
- void testScriptWithExistingMainCS() { // GROOVY-9146
+ @Test // GROOVY-9146
+ void testScriptWithExistingMainCS() {
assertScript '''
@groovy.transform.CompileStatic
static void main(args) {