Merge pull request #676 in G/fastr from...

Merge pull request #676 in G/fastr from ~STEPAN.SINDELAR_ORACLE.COM/fastr:fix/symbol-eq-args-matching to master

* commit 'cf846431':
  Argument matching: first by exact name and only then by partial
  Deparse in BinaryBooleanNode removes backticks, e.g. around `+`

com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/binary/BinaryBooleanNode.java

@@ -133,15 +133,19 @@ public abstract class BinaryBooleanNode extends RBuiltinNode {
                     @Cached("createRecursive()") BinaryBooleanNode recursive) {
         Object recursiveLeft = left;
         if (isSymbolOrLang(left)) {
-            recursiveLeft = RString.valueOf(RDeparse.deparse(left));
+            recursiveLeft = deparseSymbolOrLang(left);
         }
         Object recursiveRight = right;
         if (isSymbolOrLang(right)) {
-            recursiveRight = RString.valueOf(RDeparse.deparse(right));
+            recursiveRight = deparseSymbolOrLang(right);
         }
         return recursive.execute(frame, recursiveLeft, recursiveRight);
     }
 
+    private static RString deparseSymbolOrLang(Object val) {
+        return RString.valueOf(RDeparse.deparse(val, RDeparse.MAX_Cutoff, false, RDeparse.KEEPINTEGER, -1));
+    }
+
     protected BinaryBooleanNode createRecursive() {
         return BinaryBooleanNode.create(factory);
     }

com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/ArgumentMatcher.java

 /*
- * Copyright (c) 2013, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -483,7 +483,7 @@ public class ArgumentMatcher {
     }
 
     /**
-     * /** This method does the heavy lifting of re-arranging arguments by their names and position,
+     * This method does the heavy lifting of re-arranging arguments by their names and position,
      * also handling varargs.
      *
      * @param signature The signature (==names) of the supplied arguments
@@ -507,19 +507,27 @@ public class ArgumentMatcher {
         Arrays.fill(resultPermutation, MatchPermutation.UNMATCHED);
         Arrays.fill(resultSignature, ArgumentsSignature.UNMATCHED);
 
+        // MATCH in two phases: first by exact name (if we have actual: 'x', 'xa' and formal 'xa',
+        // then actual 'x' should not steal the position of 'xa'), then by partial
         boolean[] matchedSuppliedArgs = new boolean[signature.getLength()];
-        for (int suppliedIndex = 0; suppliedIndex < signature.getLength(); suppliedIndex++) {
-            String suppliedName = signature.getName(suppliedIndex);
-            if (suppliedName == null || suppliedName.isEmpty()) {
-                continue;
-            }
+        boolean[] formalsMatchedByExactName = new boolean[formalSignature.getLength()];
+        for (boolean byExactName : new boolean[]{true, false}) {
+            for (int suppliedIndex = 0; suppliedIndex < signature.getLength(); suppliedIndex++) {
+                String suppliedName = signature.getName(suppliedIndex);
+                boolean wasMatchedByExactName = !byExactName && matchedSuppliedArgs[suppliedIndex];
+                if (wasMatchedByExactName || suppliedName == null || suppliedName.isEmpty()) {
+                    continue;
+                }
 
-            // Search for argument name inside formal arguments
-            int formalIndex = findParameterPosition(formalSignature, suppliedName, resultPermutation, suppliedIndex, hasVarArgs, callingNode, varArgIndex, errorString, builtin);
-            if (formalIndex != MatchPermutation.UNMATCHED) {
-                resultPermutation[formalIndex] = suppliedIndex;
-                resultSignature[formalIndex] = suppliedName;
-                matchedSuppliedArgs[suppliedIndex] = true;
+                // Search for argument name inside formal arguments
+                int formalIndex = findParameterPosition(formalSignature, suppliedName, resultPermutation, suppliedIndex, hasVarArgs, callingNode, varArgIndex, errorString, builtin,
+                                formalsMatchedByExactName, byExactName);
+                if (formalIndex != MatchPermutation.UNMATCHED) {
+                    resultPermutation[formalIndex] = suppliedIndex;
+                    resultSignature[formalIndex] = suppliedName;
+                    formalsMatchedByExactName[formalIndex] = byExactName;
+                    matchedSuppliedArgs[suppliedIndex] = true;
+                }
             }
         }
 
@@ -536,7 +544,8 @@ public class ArgumentMatcher {
                         break outer;
                     }
                     if (!matchedSuppliedArgs[suppliedIndex]) {
-                        if (signature.getName(suppliedIndex) == null || signature.getName(suppliedIndex).isEmpty()) {
+                        String suppliedName = signature.getName(suppliedIndex);
+                        if (suppliedName == null || suppliedName.isEmpty()) {
                             // unnamed parameter, match by position
                             break;
                         }
@@ -622,14 +631,23 @@ public class ArgumentMatcher {
         return sum;
     }
 
+    private static <T> int findParameterPosition(ArgumentsSignature formalsSignature, String suppliedName, int[] resultPermutation, int suppliedIndex, boolean hasVarArgs, RBaseNode callingNode,
+                    int varArgIndex, IntFunction<String> errorString, RBuiltinDescriptor builtin, boolean[] formalsMatchedByExactName, boolean exactNameMatch) {
+        if (exactNameMatch) {
+            return findParameterPositionByExactName(formalsSignature, suppliedName, resultPermutation, hasVarArgs, callingNode, builtin);
+        } else {
+            return findParameterPositionByPartialName(formalsSignature, formalsMatchedByExactName, suppliedName, resultPermutation, suppliedIndex, hasVarArgs, callingNode, varArgIndex, errorString);
+        }
+    }
+
     /**
      * Searches for suppliedName inside formalNames and returns its (formal) index.
      *
      * @return The position of the given suppliedName inside the formalNames. Throws errors if the
      *         argument has been matched before
      */
-    private static <T> int findParameterPosition(ArgumentsSignature formalsSignature, String suppliedName, int[] resultPermutation, int suppliedIndex, boolean hasVarArgs, RBaseNode callingNode,
-                    int varArgIndex, IntFunction<String> errorString, RBuiltinDescriptor builtin) {
+    private static <T> int findParameterPositionByExactName(ArgumentsSignature formalsSignature, String suppliedName, int[] resultPermutation, boolean hasVarArgs,
+                    RBaseNode callingNode, RBuiltinDescriptor builtin) {
         assert suppliedName != null && !suppliedName.isEmpty();
         for (int i = 0; i < formalsSignature.getLength(); i++) {
             String formalName = formalsSignature.getName(i);
@@ -638,9 +656,8 @@ public class ArgumentMatcher {
                     if (resultPermutation[i] != MatchPermutation.UNMATCHED) {
                         if (builtin != null && builtin.getKind() == RBuiltinKind.PRIMITIVE && hasVarArgs) {
                             // for primitives, the first argument is matched, and the others are
-                            // folded
-                            // into varargs, for example:
-                            // x<-1:64; dim(x)<-c(4,4,2,2); x[1,drop=FALSE,1,drop=TRUE,-1]
+                            // folded into varargs, for example: x<-1:64; dim(x)<-c(4,4,2,2);
+                            // x[1,drop=FALSE,1,drop=TRUE,-1]
                             return MatchPermutation.UNMATCHED;
                         } else {
                             // Has already been matched: Error!
@@ -651,8 +668,25 @@ public class ArgumentMatcher {
                 }
             }
         }
+        return MatchPermutation.UNMATCHED;
+    }
+
+    /**
+     * Searches for partial match of suppliedName inside formalNames and returns its (formal) index.
+     *
+     * @return The position of the given suppliedName inside the formalNames. Throws errors if the
+     *         argument has been matched before
+     */
+    private static <T> int findParameterPositionByPartialName(ArgumentsSignature formalsSignature, boolean[] formalsMatchedByExactName, String suppliedName, int[] resultPermutation, int suppliedIndex,
+                    boolean hasVarArgs, RBaseNode callingNode, int varArgIndex, IntFunction<String> errorString) {
+        assert suppliedName != null && !suppliedName.isEmpty();
         int found = MatchPermutation.UNMATCHED;
         for (int i = 0; i < formalsSignature.getLength(); i++) {
+            if (formalsMatchedByExactName[i]) {
+                // was already matched by some exact match
+                continue;
+            }
+
             String formalName = formalsSignature.getName(i);
             if (formalName != null) {
                 if (formalName.startsWith(suppliedName) && ((varArgIndex != ArgumentsSignature.NO_VARARG && i < varArgIndex) || varArgIndex == ArgumentsSignature.NO_VARARG)) {

com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test

@@ -34312,6 +34312,14 @@ NULL
 [1] "some"
 [1] "test"
+##com.oracle.truffle.r.test.builtins.TestBuiltin_operators.testBinaryOperators#
+#as.symbol('*') == '*'
+[1] TRUE
+
+##com.oracle.truffle.r.test.builtins.TestBuiltin_operators.testBinaryOperators#
+#as.symbol('<-') == '<-'
+[1] TRUE
+
 ##com.oracle.truffle.r.test.builtins.TestBuiltin_operators.testColon#
 #8.2:NULL
 Error in 8.2:NULL : argument of length 0
@@ -72312,6 +72320,20 @@ $v
 [1] 8
+##com.oracle.truffle.r.test.functions.TestFunctions.testMatching#
+#{ foo <- function(xa, ...) list(xa=xa, ...); foo(x=4,xa=5); }
+$xa
+[1] 5
+
+$x
+[1] 4
+
+
+##com.oracle.truffle.r.test.functions.TestFunctions.testMatching#
+#{ foo <- function(xaaa, ...) list(xaa=xaaa, ...); foo(xa=4,xaa=5); }
+Error in foo(xa = 4, xaa = 5) :
+  formal argument "xaaa" matched by multiple actual arguments
+
 ##com.oracle.truffle.r.test.functions.TestFunctions.testMatching#
 #{ x<-function(foo,bar){foo*bar} ; x(f=10,2) }
 [1] 20

com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_operators.java

@@ -1926,6 +1926,13 @@ public class TestBuiltin_operators extends TestBase {
                         "data[c('a','b')] + 1; 1 + data[c('a','b')]; data[c('a','b')] + c(1,2); c(1,2) + data[c('a','b')]");
     }
 
+    @Test
+    public void testBooleanOperators() {
+        // tests that deparse in as.symbol removes backticks
+        assertEval("as.symbol('*') == '*'");
+        assertEval("as.symbol('<-') == '<-'");
+    }
+
     @Test
     public void testOperators() {
         assertEval("{ `+`(1,2) }");

com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/functions/TestFunctions.java

@@ -4,7 +4,7 @@
  * http://www.gnu.org/licenses/gpl-2.0.html
  *
  * Copyright (c) 2012-2014, Purdue University
- * Copyright (c) 2013, 2016, Oracle and/or its affiliates
+ * Copyright (c) 2013, 2017, Oracle and/or its affiliates
  *
  * All rights reserved.
  */
@@ -235,6 +235,11 @@ public class TestFunctions extends TestBase {
         // with ... partial-match only if formal parameter are before ...
         assertEval("{ f<-function(..., val=1) { c(list(...), val) }; f(v=7, 2) }");
         assertEval("{ f<-function(er=1, ..., val=1) { c(list(...), val, er) }; f(v=7, 2, e=8) }");
+
+        // exact match of 'xa' is not "stolen" by partial match of 'x'
+        assertEval("{ foo <- function(xa, ...) list(xa=xa, ...); foo(x=4,xa=5); }");
+        // however, two partial matches produce error, even if one is "longer"
+        assertEval("{ foo <- function(xaaa, ...) list(xaa=xaaa, ...); foo(xa=4,xaa=5); }");
     }
 
     @Test