From 70b874cd118f2082f3bc47d02ecc0cfc9aac82a1 Mon Sep 17 00:00:00 2001
From: Lukas Stadler <lukas.stadler@oracle.com>
Date: Tue, 14 Jun 2016 13:39:44 +0200
Subject: [PATCH] change S3 lookup order to consider enclosing env and method
 table simultaneously

---
 .../truffle/r/nodes/function/RCallNode.java   |  2 +-
 .../nodes/function/S3FunctionLookupNode.java  | 35 +++++++++++--------
 .../truffle/r/test/ExpectedTestOutput.test    |  8 +++++
 .../r/test/functions/TestS3Dispatch.java      |  7 ++++
 4 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/RCallNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/RCallNode.java
index a2343f6a08..b39bfbd862 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/RCallNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/RCallNode.java
@@ -399,6 +399,7 @@ public abstract class RCallNode extends RCallBaseNode implements RSyntaxNode, RS
                     @Cached("createUninitializedExplicitCall()") FunctionDispatch call) {
 
         Object[] args = explicitArgs != null ? ((RArgsValuesAndNames) explicitArgs.execute(frame)).getArguments() : callArguments.evaluateFlattenObjects(frame, lookupVarArgs(frame));
+        ArgumentsSignature argsSignature = explicitArgs != null ? ((RArgsValuesAndNames) explicitArgs.execute(frame)).getSignature() : callArguments.flattenNames(lookupVarArgs(frame));
 
         RBuiltinDescriptor builtin = builtinProfile.profile(function.getRBuiltin());
         RDispatch dispatch = builtin.getDispatch();
@@ -462,7 +463,6 @@ public abstract class RCallNode extends RCallBaseNode implements RSyntaxNode, RS
             }
             resultFunction = result.function;
         }
-        ArgumentsSignature argsSignature = explicitArgs != null ? ((RArgsValuesAndNames) explicitArgs.execute(frame)).getSignature() : callArguments.flattenNames(lookupVarArgs(frame));
         return call.execute(frame, resultFunction, new RArgsValuesAndNames(args, argsSignature), s3Args);
     }
 
diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/S3FunctionLookupNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/S3FunctionLookupNode.java
index 256121fac1..e2dd419352 100644
--- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/S3FunctionLookupNode.java
+++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/function/S3FunctionLookupNode.java
@@ -101,22 +101,17 @@ public abstract class S3FunctionLookupNode extends RBaseNode {
     @TruffleBoundary
     private static Result performLookup(MaterializedFrame callerFrame, String genericName, String groupName, RStringVector type, boolean nextMethod, LookupOperation op, GetMethodsTable getTable) {
         Result result;
-        // look for a generic function reachable from the caller frame
-        if ((result = lookupClassGenerics(callerFrame, genericName, groupName, type, nextMethod, false, op)) != null) {
-            return result;
-        }
         Object methodsTable = getTable.get();
         if (methodsTable instanceof RPromise) {
             methodsTable = PromiseHelperNode.evaluateSlowPath(null, (RPromise) methodsTable);
         }
         MaterializedFrame methodsTableFrame = methodsTable == null ? null : ((REnvironment) methodsTable).getFrame();
 
-        if (methodsTableFrame != null) {
-            // look for a generic function in the methods table
-            if ((result = lookupClassGenerics(methodsTableFrame, genericName, groupName, type, nextMethod, true, op)) != null) {
-                return result;
-            }
+        // look for a generic function reachable from the caller frame
+        if ((result = lookupClassGenerics(callerFrame, methodsTableFrame, genericName, groupName, type, nextMethod, op)) != null) {
+            return result;
         }
+
         // look for the default method
         String functionName = genericName + RRuntime.RDOT + RRuntime.DEFAULT;
         RFunction function = checkPromise(op.read(callerFrame, functionName, false));
@@ -129,19 +124,29 @@ public abstract class S3FunctionLookupNode extends RBaseNode {
         return null;
     }
 
-    private static Result lookupClassGenerics(MaterializedFrame callerFrame, String genericName, String groupName, RStringVector type, boolean nextMethod, boolean inMethodsTable, LookupOperation op) {
+    private static Result lookupClassGenerics(MaterializedFrame callerFrame, MaterializedFrame methodsTableFrame, String genericName, String groupName, RStringVector type, boolean nextMethod,
+                    LookupOperation op) {
         Result result = null;
         for (int i = nextMethod ? 1 : 0; i < type.getLength(); i++) {
             String clazzName = type.getDataAt(i);
             boolean groupMatch = false;
 
             String functionName = genericName + RRuntime.RDOT + type.getDataAt(i);
-            RFunction function = checkPromise(op.read(callerFrame, functionName, inMethodsTable));
+            RFunction function = checkPromise(op.read(callerFrame, functionName, false));
+
+            if (function == null) {
+                if (methodsTableFrame != null) {
+                    function = checkPromise(op.read(methodsTableFrame, functionName, true));
+                }
 
-            if (function == null && groupName != null) {
-                groupMatch = true;
-                functionName = groupName + RRuntime.RDOT + clazzName;
-                function = checkPromise(op.read(callerFrame, functionName, inMethodsTable));
+                if (function == null && groupName != null) {
+                    groupMatch = true;
+                    functionName = groupName + RRuntime.RDOT + clazzName;
+                    function = checkPromise(op.read(callerFrame, functionName, false));
+                    if (function == null && methodsTableFrame != null) {
+                        function = checkPromise(op.read(methodsTableFrame, functionName, true));
+                    }
+                }
             }
 
             if (function != null) {
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
index 015e5d666b..e9c3209d2d 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
@@ -54902,6 +54902,14 @@ attr(,"class")
 attr(,"class")
 [1] "foo"
 
+##com.oracle.truffle.r.test.functions.TestS3Dispatch.testMethodTableDispatch
+#t <- ts(1:3); class(t) <- c('ts', 'foo'); print.foo <- function(x, ...) 'foo'; print(t)
+Time Series:
+Start = 1
+End = 3
+Frequency = 1
+[1] 1 2 3
+
 ##com.oracle.truffle.r.test.functions.TestS3Dispatch.testNextMethod
 #{ f.default<-function(x, a=7) a; f.foo<-function(x, a=v) { b<-NextMethod("f"); v=42; c(a,b) }; x<-1; class(x)<-"foo"; f<-function(x) UseMethod("f"); f(x) }
 [1] 42  7
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/functions/TestS3Dispatch.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/functions/TestS3Dispatch.java
index 5cd1970bea..2d753d6a97 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/functions/TestS3Dispatch.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/functions/TestS3Dispatch.java
@@ -111,4 +111,11 @@ public class TestS3Dispatch extends TestBase {
     public void testComplexGroupDispatch() {
         assertEval("{x<--7+2i;class(x)<-\"foo\";Complex.foo<-function(z){1;};Im(x);}");
     }
+
+    @Test
+    public void testMethodTableDispatch() {
+        // this test ensures that print.ts is found in the method table before print.foo is found in
+        // the calling environment
+        assertEval("t <- ts(1:3); class(t) <- c('ts', 'foo'); print.foo <- function(x, ...) 'foo'; print(t)");
+    }
 }
-- 
GitLab