You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2015/12/28 08:00:37 UTC

[1/2] groovy git commit: Extract common behaviors from the method signature resolution

Repository: groovy
Updated Branches:
  refs/heads/master ebf4a9227 -> 65f2bc133


Extract common behaviors from the method signature resolution


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/3f4fe426
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/3f4fe426
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/3f4fe426

Branch: refs/heads/master
Commit: 3f4fe4267ac04ed7ba53c7efcae9923495fd7c7f
Parents: ebf4a92
Author: Frank Pavageau <fp...@ekino.com>
Authored: Wed Dec 2 15:23:42 2015 +0100
Committer: pascalschumacher <pa...@gmx.net>
Committed: Mon Dec 28 07:58:43 2015 +0100

----------------------------------------------------------------------
 .../stc/StaticTypeCheckingSupport.java          | 107 ++++++++++---------
 1 file changed, 55 insertions(+), 52 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/3f4fe426/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index d28d5fd..a99b7f1 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -219,7 +219,7 @@ public abstract class StaticTypeCheckingSupport {
         }
         int dist = 0;
         if (args.length<params.length) return -1;
-        // we already know the lengths are equal
+        // we already know there are at least params.length elements in both arrays
         for (int i = 0; i < params.length; i++) {
             ClassNode paramType = params[i].getType();
             ClassNode argType = args[i];
@@ -281,7 +281,7 @@ public abstract class StaticTypeCheckingSupport {
         ClassNode vargsBase = params[params.length - 1].getType().getComponentType();
         for (int i = params.length; i < args.length; i++) {
             if (!isAssignableTo(args[i],vargsBase)) return -1;
-            else if (!args[i].equals(vargsBase)) dist+=getDistance(args[i], vargsBase);
+            else dist += getClassDistance(vargsBase, args[i]);
         }
         return dist;
     }
@@ -949,10 +949,8 @@ public abstract class StaticTypeCheckingSupport {
         int bestDist = Integer.MAX_VALUE;
         Collection<MethodNode> choicesLeft = removeCovariants(methods);
         for (MethodNode candidateNode : choicesLeft) {
-            ClassNode declaringClass = candidateNode.getDeclaringClass();
-            ClassNode actualReceiver = receiver!=null?receiver: declaringClass;
-            final ClassNode declaringClassForDistance = declaringClass;
-            final ClassNode actualReceiverForDistance = actualReceiver;
+            ClassNode declaringClassForDistance = candidateNode.getDeclaringClass();
+            ClassNode actualReceiverForDistance = receiver != null ? receiver : candidateNode.getDeclaringClass();
             MethodNode safeNode = candidateNode;
             ClassNode[] safeArgs = args;
             boolean isExtensionMethodNode = candidateNode instanceof ExtensionMethodNode;
@@ -961,8 +959,6 @@ public abstract class StaticTypeCheckingSupport {
                 System.arraycopy(args, 0, safeArgs, 1, args.length);
                 safeArgs[0] = receiver;
                 safeNode = ((ExtensionMethodNode) candidateNode).getExtensionMethodNode();
-                declaringClass = safeNode.getDeclaringClass();
-                actualReceiver = declaringClass;
             }
 
             // todo : corner case
@@ -979,52 +975,35 @@ public abstract class StaticTypeCheckingSupport {
             Parameter[] params = makeRawTypes(safeNode.getParameters());
             if (params.length == safeArgs.length) {
                 int allPMatch = allParametersAndArgumentsMatch(params, safeArgs);
-                boolean firstParamMatches = true;
-                // check first parameters
-                if (safeArgs.length > 0) {
-                    Parameter[] firstParams = new Parameter[params.length - 1];
-                    System.arraycopy(params, 0, firstParams, 0, firstParams.length);
-                    firstParamMatches = allParametersAndArgumentsMatch(firstParams, safeArgs) >= 0;
-                }
-                int lastArgMatch = isVargs(params) && firstParamMatches?lastArgMatchesVarg(params, safeArgs):-1;
+                int firstParamDist = firstParametersAndArgumentsMatch(params, safeArgs);
+                int lastArgMatch = isVargs(params) && firstParamDist >= 0?lastArgMatchesVarg(params, safeArgs):-1;
                 if (lastArgMatch>=0) {
-                    lastArgMatch += 256-params.length; // ensure exact matches are preferred over vargs
+                    lastArgMatch += getVarargsDistance(params);
                 }
                 int dist = allPMatch>=0?Math.max(allPMatch, lastArgMatch):lastArgMatch;
-                if (dist>=0 && !actualReceiverForDistance.equals(declaringClassForDistance)) dist+=getDistance(actualReceiverForDistance, declaringClassForDistance);
-                if (dist>=0 && !isExtensionMethodNode) {
-                    dist++;
-                }
-                if (dist>=0 && dist<bestDist) {
-                    bestChoices.clear();
-                    bestChoices.add(candidateNode);
-                    bestDist = dist;
-                } else if (dist>=0 && dist==bestDist) {
-                    bestChoices.add(candidateNode);
+                if (dist >= 0) {
+                    dist += getClassDistance(declaringClassForDistance, actualReceiverForDistance);
+                    dist += getExtensionDistance(isExtensionMethodNode);
+                    if (dist < bestDist) {
+                        bestChoices.clear();
+                        bestChoices.add(candidateNode);
+                        bestDist = dist;
+                    } else if (dist == bestDist) {
+                        bestChoices.add(candidateNode);
+                    }
                 }
             } else if (isVargs(params)) {
-                boolean firstParamMatches = true;
-                int dist = -1;
-                // check first parameters
-                if (safeArgs.length > 0) {
-                    Parameter[] firstParams = new Parameter[params.length - 1];
-                    System.arraycopy(params, 0, firstParams, 0, firstParams.length);
-                    dist = allParametersAndArgumentsMatch(firstParams, safeArgs);
-                    firstParamMatches =  dist >= 0;
-                } else {
-                    dist = 0;
-                }
-                if (firstParamMatches) {
+                int dist = firstParametersAndArgumentsMatch(params, safeArgs);
+                if (dist >= 0) {
                     // there are three case for vargs
                     // (1) varg part is left out
                     if (params.length == safeArgs.length + 1) {
-                        if (dist>=0) {
-                            dist += 256-params.length; // ensure exact matches are preferred over vargs
-                        }
-                        if (bestDist > 1+dist) {
+                        dist += getVarargsDistance(params);
+                        dist++; // increment to discriminate foo(Object,String) vs foo(Object,String, Object...)
+                        if (dist < bestDist) {
                             bestChoices.clear();
                             bestChoices.add(candidateNode);
-                            bestDist = 1+dist; // 1+dist to discriminate foo(Object,String) vs foo(Object,String, Object...)
+                            bestDist = dist;
                         }
                     } else {
                         // (2) last argument is put in the vargs array
@@ -1033,20 +1012,18 @@ public abstract class StaticTypeCheckingSupport {
                         int excessArgumentsDistance = excessArgumentsMatchesVargsParameter(params, safeArgs);
                         if (excessArgumentsDistance < 0) continue;
                         dist += excessArgumentsDistance;
-                        if (dist >= 0 && !actualReceiverForDistance.equals(declaringClassForDistance)) dist+=getDistance(actualReceiverForDistance, declaringClassForDistance);
+                        dist += getClassDistance(declaringClassForDistance, actualReceiverForDistance);
                         // varargs methods must not be preferred to methods without varargs
                         // for example :
                         // int sum(int x) should be preferred to int sum(int x, int... y)
-                        dist+=256-params.length;
-                        if (dist>=0 && !isExtensionMethodNode) {
-                            dist++;
-                        }
-                        if (params.length < safeArgs.length && dist >= 0) {
-                            if (dist >= 0 && dist < bestDist) {
+                        dist += getVarargsDistance(params);
+                        dist += getExtensionDistance(isExtensionMethodNode);
+                        if (params.length < safeArgs.length) {
+                            if (dist < bestDist) {
                                 bestChoices.clear();
                                 bestChoices.add(candidateNode);
                                 bestDist = dist;
-                            } else if (dist >= 0 && dist == bestDist) {
+                            } else if (dist == bestDist) {
                                 bestChoices.add(candidateNode);
                             }
                         }
@@ -1069,6 +1046,32 @@ public abstract class StaticTypeCheckingSupport {
         return bestChoices;
     }
 
+    private static int firstParametersAndArgumentsMatch(Parameter[] params, ClassNode[] safeArgs) {
+        int dist = 0;
+        // check first parameters
+        if (params.length > 0) {
+            Parameter[] firstParams = new Parameter[params.length - 1];
+            System.arraycopy(params, 0, firstParams, 0, firstParams.length);
+            dist = allParametersAndArgumentsMatch(firstParams, safeArgs);
+        }
+        return dist;
+    }
+
+    private static int getVarargsDistance(Parameter[] params) {
+        return 256 - params.length; // ensure exact matches are preferred over vargs
+    }
+
+    private static int getClassDistance(ClassNode declaringClassForDistance, ClassNode actualReceiverForDistance) {
+        if (actualReceiverForDistance.equals(declaringClassForDistance)) {
+            return 0;
+        }
+        return getDistance(actualReceiverForDistance, declaringClassForDistance);
+    }
+
+    private static int getExtensionDistance(boolean isExtensionMethodNode) {
+        return isExtensionMethodNode ? 0 : 1;
+    }
+
     private static Parameter[] makeRawTypes(Parameter[] params) {
         Parameter[] newParam = new Parameter[params.length];
         for (int i=0; i<params.length; i++) {


[2/2] groovy git commit: GROOVY-7710 GROOVY-7711 Apply the rules for method resolution consistently (closes #213)

Posted by pa...@apache.org.
GROOVY-7710 GROOVY-7711 Apply the rules for method resolution consistently (closes #213)

A no-arg call to a varargs method needs to be resolved with the same rules
as when there are exactly the same number of parameters, or even more, by
applying the class distance and extension method biases.


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/65f2bc13
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/65f2bc13
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/65f2bc13

Branch: refs/heads/master
Commit: 65f2bc133b38db4564c2f29b3ba67db2ac649173
Parents: 3f4fe42
Author: Frank Pavageau <fp...@ekino.com>
Authored: Mon Dec 14 22:58:17 2015 +0100
Committer: pascalschumacher <pa...@gmx.net>
Committed: Mon Dec 28 07:59:18 2015 +0100

----------------------------------------------------------------------
 .../stc/StaticTypeCheckingSupport.java          | 100 +++++++++----------
 .../groovy/transform/stc/BugsSTCTest.groovy     |  24 +++++
 2 files changed, 69 insertions(+), 55 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/65f2bc13/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
index a99b7f1..b392e39 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
@@ -973,61 +973,16 @@ public abstract class StaticTypeCheckingSupport {
              */
 
             Parameter[] params = makeRawTypes(safeNode.getParameters());
-            if (params.length == safeArgs.length) {
-                int allPMatch = allParametersAndArgumentsMatch(params, safeArgs);
-                int firstParamDist = firstParametersAndArgumentsMatch(params, safeArgs);
-                int lastArgMatch = isVargs(params) && firstParamDist >= 0?lastArgMatchesVarg(params, safeArgs):-1;
-                if (lastArgMatch>=0) {
-                    lastArgMatch += getVarargsDistance(params);
-                }
-                int dist = allPMatch>=0?Math.max(allPMatch, lastArgMatch):lastArgMatch;
-                if (dist >= 0) {
-                    dist += getClassDistance(declaringClassForDistance, actualReceiverForDistance);
-                    dist += getExtensionDistance(isExtensionMethodNode);
-                    if (dist < bestDist) {
-                        bestChoices.clear();
-                        bestChoices.add(candidateNode);
-                        bestDist = dist;
-                    } else if (dist == bestDist) {
-                        bestChoices.add(candidateNode);
-                    }
-                }
-            } else if (isVargs(params)) {
-                int dist = firstParametersAndArgumentsMatch(params, safeArgs);
-                if (dist >= 0) {
-                    // there are three case for vargs
-                    // (1) varg part is left out
-                    if (params.length == safeArgs.length + 1) {
-                        dist += getVarargsDistance(params);
-                        dist++; // increment to discriminate foo(Object,String) vs foo(Object,String, Object...)
-                        if (dist < bestDist) {
-                            bestChoices.clear();
-                            bestChoices.add(candidateNode);
-                            bestDist = dist;
-                        }
-                    } else {
-                        // (2) last argument is put in the vargs array
-                        //      that case is handled above already
-                        // (3) there is more than one argument for the vargs array
-                        int excessArgumentsDistance = excessArgumentsMatchesVargsParameter(params, safeArgs);
-                        if (excessArgumentsDistance < 0) continue;
-                        dist += excessArgumentsDistance;
-                        dist += getClassDistance(declaringClassForDistance, actualReceiverForDistance);
-                        // varargs methods must not be preferred to methods without varargs
-                        // for example :
-                        // int sum(int x) should be preferred to int sum(int x, int... y)
-                        dist += getVarargsDistance(params);
-                        dist += getExtensionDistance(isExtensionMethodNode);
-                        if (params.length < safeArgs.length) {
-                            if (dist < bestDist) {
-                                bestChoices.clear();
-                                bestChoices.add(candidateNode);
-                                bestDist = dist;
-                            } else if (dist == bestDist) {
-                                bestChoices.add(candidateNode);
-                            }
-                        }
-                    }
+            int dist = measureParametersAndArgumentsDistance(params, safeArgs);
+            if (dist >= 0) {
+                dist += getClassDistance(declaringClassForDistance, actualReceiverForDistance);
+                dist += getExtensionDistance(isExtensionMethodNode);
+                if (dist < bestDist) {
+                    bestChoices.clear();
+                    bestChoices.add(candidateNode);
+                    bestDist = dist;
+                } else if (dist == bestDist) {
+                    bestChoices.add(candidateNode);
                 }
             }
         }
@@ -1046,6 +1001,41 @@ public abstract class StaticTypeCheckingSupport {
         return bestChoices;
     }
 
+    private static int measureParametersAndArgumentsDistance(Parameter[] params, ClassNode[] args) {
+        int dist = -1;
+        if (params.length == args.length) {
+            int allPMatch = allParametersAndArgumentsMatch(params, args);
+            int firstParamDist = firstParametersAndArgumentsMatch(params, args);
+            int lastArgMatch = isVargs(params) && firstParamDist >= 0 ? lastArgMatchesVarg(params, args) : -1;
+            if (lastArgMatch >= 0) {
+                lastArgMatch += getVarargsDistance(params);
+            }
+            dist = allPMatch >= 0 ? Math.max(allPMatch, lastArgMatch) : lastArgMatch;
+        } else if (isVargs(params)) {
+            dist = firstParametersAndArgumentsMatch(params, args);
+            if (dist >= 0) {
+                // varargs methods must not be preferred to methods without varargs
+                // for example :
+                // int sum(int x) should be preferred to int sum(int x, int... y)
+                dist += getVarargsDistance(params);
+                // there are three case for vargs
+                // (1) varg part is left out (there's one less argument than there are parameters)
+                // (2) last argument is put in the vargs array
+                //     that case is handled above already when params and args have the same length
+                if (params.length < args.length) {
+                    // (3) there is more than one argument for the vargs array
+                    int excessArgumentsDistance = excessArgumentsMatchesVargsParameter(params, args);
+                    if (excessArgumentsDistance < 0) {
+                        dist = -1;
+                    } else {
+                        dist += excessArgumentsDistance;
+                    }
+                }
+            }
+        }
+        return dist;
+    }
+
     private static int firstParametersAndArgumentsMatch(Parameter[] params, ClassNode[] safeArgs) {
         int dist = 0;
         // check first parameters

http://git-wip-us.apache.org/repos/asf/groovy/blob/65f2bc13/src/test/groovy/transform/stc/BugsSTCTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/transform/stc/BugsSTCTest.groovy b/src/test/groovy/transform/stc/BugsSTCTest.groovy
index 438b585..7f09833 100644
--- a/src/test/groovy/transform/stc/BugsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/BugsSTCTest.groovy
@@ -648,6 +648,30 @@ Printer
         '''
     }
 
+    void testAmbiguousMethodResolutionGroovy7710NoArgsOverloaded() {
+        shouldFailWithMessages '''
+            Arrays.sort()
+        ''', 'Reference to method is ambiguous. Cannot choose between '
+    }
+
+    void testAmbiguousMethodResolutionGroovy7711NoArgsCovariantOverride() {
+        assertScript '''
+            class A {}
+            class B {
+                Object m(Object[] args) {
+                    new Object()
+                }
+            }
+            class C extends B {
+                A m(Object[] args) {
+                    new A()
+                }
+            }
+            C c = new C()
+            A a = c.m()
+        '''
+    }
+
     // GROOVY-6911
     void testShouldNotThrowArrayIndexOfOutBoundsException() {
         assertScript '''