support function pointer returns and optimize function point variables

wlav · Jun 19, 2022 · dd183a0 · dd183a0
1 parent aa68005
commit dd183a0
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 41 deletions.
diff --git a/src/Converters.cxx b/src/Converters.cxx
@@ -2554,51 +2554,15 @@ bool CPyCppyy::FunctionPointerConverter::SetArg(
     return false;
 }
 
-static std::map<void*, std::string> sFuncWrapperLookup;
 PyObject* CPyCppyy::FunctionPointerConverter::FromMemory(void* address)
 {
 // A function pointer in clang is represented by a Type, not a FunctionDecl and it's
 // not possible to get the latter from the former: the backend will need to support
 // both. Since that is far in the future, we'll use a std::function instead.
-    static int func_count = 0;
-
-    if (!(address && *(void**)address)) {
-        PyErr_SetString(PyExc_TypeError, "can not convert null function pointer");
-        return nullptr;
-    }
-
-    void* faddr = *(void**)address;
-    auto cached = sFuncWrapperLookup.find(faddr);
-    if (cached == sFuncWrapperLookup.end()) {
-        std::ostringstream fname;
-        fname << "ptr2func" << ++func_count;
-
-        std::ostringstream code;
-        code << "namespace __cppyy_internal {\n  std::function<"
-             << fRetType << fSignature << "> " << fname.str()
-             << " = (" << fRetType << "(*)" << fSignature << ")" << (intptr_t)faddr
-             << ";\n}";
-
-        if (!Cppyy::Compile(code.str())) {
-            PyErr_SetString(PyExc_TypeError, "conversion to std::function failed");
-            return nullptr;
-        }
-
-     // cache the new wrapper (TODO: does it make sense to use weakrefs on the data
-     // member?)
-        sFuncWrapperLookup[faddr] = fname.str();
-        cached = sFuncWrapperLookup.find(faddr);
-    }
-
-    static Cppyy::TCppScope_t scope = Cppyy::GetScope("__cppyy_internal");
-    PyObject* pyscope = CreateScopeProxy(scope);
-    PyObject* func = PyObject_GetAttrString(pyscope, cached->second.c_str());
-    Py_DECREF(pyscope);
-
-    if (func)      // prevent moving this func object, since then it can not be reused
-        ((CPPInstance*)func)->fFlags |= CPPInstance::kIsLValue;
-
-    return func;
+    if (address)
+        return Utility::FuncPtr2StdFunction(fRetType, fSignature, *(void**)address);
+    PyErr_SetString(PyExc_TypeError, "can not convert null function pointer");
+    return nullptr;
 }
 
 bool CPyCppyy::FunctionPointerConverter::ToMemory(
@@ -3058,7 +3022,7 @@ CPyCppyy::Converter* CPyCppyy::CreateConverter(const std::string& fullType, cdim
         }
     } else if (resolvedType.find("(*)") != std::string::npos ||
                (resolvedType.find("::*)") != std::string::npos)) {
-    // this is a function function pointer
+    // this is a function pointer
     // TODO: find better way of finding the type
         auto pos1 = resolvedType.find('(');
         auto pos2 = resolvedType.find("*)");

diff --git a/src/DeclareExecutors.h b/src/DeclareExecutors.h
@@ -181,6 +181,18 @@ class InstanceArrayExecutor : public InstancePtrExecutor {
     dim_t fSize;
 };
 
+class FunctionPointerExecutor : public Executor {
+public:
+    FunctionPointerExecutor(const std::string& ret, const std::string& sig) :
+        fRetType(ret), fSignature(sig) {}
+    virtual PyObject* Execute(
+        Cppyy::TCppMethod_t, Cppyy::TCppObject_t, CallContext*);
+
+protected:
+    std::string fRetType;
+    std::string fSignature;
+};
+
 } // unnamed namespace
 
 } // namespace CPyCppyy

diff --git a/src/Executors.cxx b/src/Executors.cxx
@@ -742,6 +742,21 @@ PyObject* CPyCppyy::PyObjectExecutor::Execute(
     return (PyObject*)GILCallR(method, self, ctxt);
 }
 
+//----------------------------------------------------------------------------
+PyObject* CPyCppyy::FunctionPointerExecutor::Execute(
+    Cppyy::TCppMethod_t method, Cppyy::TCppObject_t self, CallContext* ctxt)
+{
+// execute <method> with argument <self, ctxt>, return std::function from func ptr
+
+// A function pointer in clang is represented by a Type, not a FunctionDecl and it's
+// not possible to get the latter from the former: the backend will need to support
+// both. Since that is far in the future, we'll use a std::function instead.
+    void* address = (void*)GILCallR(method, self, ctxt);
+    if (address)
+        return Utility::FuncPtr2StdFunction(fRetType, fSignature, address);
+    PyErr_SetString(PyExc_TypeError, "can not convert null function pointer");
+    return nullptr;
+}
 
 //- factories ----------------------------------------------------------------
 CPyCppyy::Executor* CPyCppyy::CreateExecutor(const std::string& fullType, cdims_t dims)
@@ -826,6 +841,15 @@ CPyCppyy::Executor* CPyCppyy::CreateExecutor(const std::string& fullType, cdims_
                 result = new InstancePtrRefExecutor(klass);
         } else
             result = new InstancePtrExecutor(klass);
+    } else if (resolvedType.find("(*)") != std::string::npos ||
+               (resolvedType.find("::*)") != std::string::npos)) {
+    // this is a function pointer
+    // TODO: find better way of finding the type
+        auto pos1 = resolvedType.find('(');
+        auto pos2 = resolvedType.find("*)");
+        auto pos3 = resolvedType.rfind(')');
+        result = new FunctionPointerExecutor(
+            resolvedType.substr(0, pos1), resolvedType.substr(pos2+2, pos3-pos2-1));
     } else {
     // unknown: void* may work ("user knows best"), void will fail on use of return value
         h = (cpd == "") ? gExecFactories.find("void") : gExecFactories.find("void ptr");

diff --git a/src/Utility.cxx b/src/Utility.cxx
@@ -697,6 +697,70 @@ void CPyCppyy::Utility::ConstructCallbackReturn(const std::string& retType, int
     code << (isVoid ? ";\n  }\n" : " ret;\n  }\n");
 }
 
+
+//----------------------------------------------------------------------------
+static std::map<void*, PyObject*> sStdFuncLookup;
+static std::map<std::string, PyObject*> sStdFuncMakerLookup;
+PyObject* CPyCppyy::Utility::FuncPtr2StdFunction(
+        const std::string& retType, const std::string& signature, void* address)
+{
+// Convert a function pointer to an equivalent std::function<> object.
+    static int maker_count = 0;
+
+    if (!address) {
+        PyErr_SetString(PyExc_TypeError, "can not convert null function pointer");
+        return nullptr;
+    }
+
+    auto pf = sStdFuncLookup.find(address);
+    if (pf != sStdFuncLookup.end()) {
+        Py_INCREF(pf->second);
+        return pf->second;
+    }
+
+    PyObject* maker = nullptr;
+
+    auto pm = sStdFuncMakerLookup.find(retType+signature);
+    if (pm == sStdFuncMakerLookup.end()) {
+        std::ostringstream fname;
+        fname << "ptr2func" << ++maker_count;
+
+        std::ostringstream code;
+        code << "namespace __cppyy_internal { std::function<"
+             << retType << signature << "> " << fname.str()
+             << "(intptr_t faddr) { return (" << retType << "(*)" << signature << ")faddr;} }";
+
+        if (!Cppyy::Compile(code.str())) {
+            PyErr_SetString(PyExc_TypeError, "conversion to std::function failed");
+            return nullptr;
+        }
+
+        PyObject* pyscope = CreateScopeProxy("__cppyy_internal");
+        maker = PyObject_GetAttrString(pyscope, fname.str().c_str());
+        Py_DECREF(pyscope);
+        if (!maker)
+            return nullptr;
+
+    // cache the new maker (TODO: does it make sense to use weakrefs?)
+        sStdFuncMakerLookup[retType+signature] = maker;
+    } else
+        maker = pm->second;
+
+    PyObject* args = PyTuple_New(1);
+    PyTuple_SET_ITEM(args, 0, PyLong_FromLongLong((intptr_t)address));
+    PyObject* func = PyObject_Call(maker, args, NULL);
+    Py_DECREF(args);
+
+    if (func) {    // prevent moving this func object, since then it can not be reused
+        ((CPPInstance*)func)->fFlags |= CPPInstance::kIsLValue;
+        Py_INCREF(func);     // TODO: use weak? The C++ maker doesn't go away either
+        sStdFuncLookup[address] = func;
+    }
+
+    return func;
+}
+
+
 //----------------------------------------------------------------------------
 bool CPyCppyy::Utility::InitProxy(PyObject* module, PyTypeObject* pytype, const char* name)
 {

diff --git a/src/Utility.h b/src/Utility.h
@@ -43,6 +43,9 @@ void ConstructCallbackPreamble(const std::string& retType,
     const std::vector<std::string>& argtypes, std::ostringstream& code);
 void ConstructCallbackReturn(const std::string& retType, int nArgs, std::ostringstream& code);
 
+// helper for function pointer conversions
+PyObject* FuncPtr2StdFunction(const std::string& retType, const std::string& signature, void* address);
+
 // initialize proxy type objects
 bool InitProxy(PyObject* module, PyTypeObject* pytype, const char* name);