Merge pull request #131 from Distributive-Network/Xmader/perf/return-underlying-function

unwrap functions when returned to their original language

Author: zollqir

include/pyTypeFactory.hh

@@ -44,6 +44,6 @@ PyType *pyTypeFactory(JSContext *cx, JS::Rooted<JSObject *> *thisObj, JS::Rooted
  * @param args - Pointer to a PyTupleObject containing the arguments to the python function
  * @return PyObject* - The result of the JSFunction called with args coerced to JS types, coerced back to a PyObject type, or NULL if coercion wasn't possible
  */
-static PyObject *callJSFunc(PyObject *JSFuncAddress, PyObject *args);
+PyObject *callJSFunc(PyObject *JSFuncAddress, PyObject *args);
 
 #endif

src/jsTypeFactory.cc

@@ -117,6 +117,12 @@ JS::Value jsTypeFactory(JSContext *cx, PyObject *object) {
     }
     memoizePyTypeAndGCThing(new StrType(object), returnType);
   }
+  else if (PyCFunction_Check(object) && PyCFunction_GetFunction(object) == callJSFunc) {
+    // If it's a wrapped JS function by us, return the underlying JS function rather than wrapping it again
+    PyObject *jsCxThisFuncTuple = PyCFunction_GetSelf(object);
+    JS::RootedValue *jsFunc = (JS::RootedValue *)PyLong_AsVoidPtr(PyTuple_GetItem(jsCxThisFuncTuple, 2));
+    returnType.set(*jsFunc);
+  }
   else if (PyFunction_Check(object) || PyCFunction_Check(object)) {
     // can't determine number of arguments for PyCFunctions, so just assume potentially unbounded
     uint16_t nargs = 0;

src/pyTypeFactory.cc

@@ -32,6 +32,7 @@
 #include "include/modules/pythonmonkey/pythonmonkey.hh"
 
 #include <jsapi.h>
+#include <jsfriendapi.h>
 #include <js/Object.h>
 #include <js/ValueArray.h>
 
@@ -123,9 +124,16 @@ PyType *pyTypeFactory(JSContext *cx, JS::Rooted<JSObject *> *thisObj, JS::Rooted
         return new ExceptionType(cx, obj);
       }
     case js::ESClass::Function: {
-        // FIXME (Tom Tang): `jsCxThisFuncTuple` and the tuple items are not going to be GCed
-        PyObject *jsCxThisFuncTuple = PyTuple_Pack(3, PyLong_FromVoidPtr(cx), PyLong_FromVoidPtr(thisObj), PyLong_FromVoidPtr(rval));
-        PyObject *pyFunc = PyCFunction_New(&callJSFuncDef, jsCxThisFuncTuple);
+        PyObject *pyFunc;
+        if (JS_IsNativeFunction(obj, callPyFunc)) { // It's a wrapped python function by us
+          // Get the underlying python function from the 0th reserved slot
+          JS::Value pyFuncVal = js::GetFunctionNativeReserved(obj, 0);
+          pyFunc = (PyObject *)(pyFuncVal.toPrivate());
+        } else {
+          // FIXME (Tom Tang): `jsCxThisFuncTuple` and the tuple items are not going to be GCed
+          PyObject *jsCxThisFuncTuple = PyTuple_Pack(3, PyLong_FromVoidPtr(cx), PyLong_FromVoidPtr(thisObj), PyLong_FromVoidPtr(rval));
+          pyFunc = PyCFunction_New(&callJSFuncDef, jsCxThisFuncTuple);
+        }
         FuncType *f = new FuncType(pyFunc);
         memoizePyTypeAndGCThing(f, *rval); // TODO (Caleb Aikens) consider putting this in the FuncType constructor
         return f;
@@ -167,7 +175,7 @@ PyType *pyTypeFactory(JSContext *cx, JS::Rooted<JSObject *> *thisObj, JS::Rooted
   return NULL;
 }
 
-static PyObject *callJSFunc(PyObject *jsCxThisFuncTuple, PyObject *args) {
+PyObject *callJSFunc(PyObject *jsCxThisFuncTuple, PyObject *args) {
   // TODO (Caleb Aikens) convert PyObject *args to JS::Rooted<JS::ValueArray> JSargs
   JSContext *cx = (JSContext *)PyLong_AsVoidPtr(PyTuple_GetItem(jsCxThisFuncTuple, 0));
   JS::RootedObject *thisObj = (JS::RootedObject *)PyLong_AsVoidPtr(PyTuple_GetItem(jsCxThisFuncTuple, 1));

tests/python/test_pythonmonkey_eval.py

@@ -235,6 +235,29 @@ def ident(x):
     # pm.collect() # TODO: to be fixed in BF-59
     assert "YYZ" == js_fn_back()
 
+def test_eval_functions_pyfunction_in_closure():
+    # BF-58 https://github.com/Distributive-Network/PythonMonkey/pull/19
+    def fn1():
+        def fn0(n):
+            return n + 100
+        return fn0
+    assert 101.9 == fn1()(1.9)
+    assert 101.9 == pm.eval("(fn1) => { return fn1 }")(fn1())(1.9)
+    assert 101.9 == pm.eval("(fn1, x) => { return fn1()(x) }")(fn1, 1.9)
+    assert 101.9 == pm.eval("(fn1) => { return fn1() }")(fn1)(1.9)
+
+def test_unwrap_py_function():
+    # https://github.com/Distributive-Network/PythonMonkey/issues/65
+    def pyFunc():
+        pass
+    unwrappedPyFunc = pm.eval("(wrappedPyFunc) => { return wrappedPyFunc }")(pyFunc)
+    assert unwrappedPyFunc is pyFunc
+
+def test_unwrap_js_function():
+    # https://github.com/Distributive-Network/PythonMonkey/issues/65
+    wrappedJSFunc = pm.eval("const JSFunc = () => { return 0 }\nJSFunc")
+    assert pm.eval("(unwrappedJSFunc) => { return unwrappedJSFunc === JSFunc }")(wrappedJSFunc)
+
 def test_eval_functions_pyfunctions_ints():
     caller = pm.eval("(func, param1, param2) => { return func(param1, param2) }")
     def add(a, b):