[cppyy] Disallow conversion from C++ instance to function pointers

Implicit conversion from C++ instances to function pointers is not
allowed in C++, and it can result in confusing overload resolutions if
we allow it in cppyy.

See:
#20687 (comment)

Author: guitargeek

bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx

@@ -2655,7 +2655,7 @@ static PyMethodDef gWrapperCacheEraserMethodDef = {
 };
 
 static void* PyFunction_AsCPointer(PyObject* pyobject,
-    const std::string& rettype, const std::string& signature)
+    const std::string& rettype, const std::string& signature, bool allowCppInstance)
 {
 // Convert a bound C++ function pointer or callable python object to a C-style
 // function pointer. The former is direct, the latter involves a JIT-ed wrapper.
@@ -2702,8 +2702,12 @@ static void* PyFunction_AsCPointer(PyObject* pyobject,
         return fptr;
     }
 
-    if (PyCallable_Check(pyobject)) {
+
+    if (PyCallable_Check(pyobject) && (allowCppInstance || !CPPInstance_Check(pyobject))) {
     // generic python callable: create a C++ wrapper function
+    // Sometimes we don't want to take this branch if the object is a C++
+    // instance, because C++ doesn't allow converting functor obejcts to
+    // function pointers, but only to std::function.
         void* wpraddress = nullptr;
 
     // re-use existing wrapper if possible
@@ -2808,7 +2812,7 @@ bool CPyCppyy::FunctionPointerConverter::SetArg(
     }
 
 // normal case, get a function pointer
-    void* fptr = PyFunction_AsCPointer(pyobject, fRetType, fSignature);
+    void* fptr = PyFunction_AsCPointer(pyobject, fRetType, fSignature, fAllowCppInstance);
     if (fptr) {
         SetLifeLine(ctxt->fPyContext, pyobject, (intptr_t)this);
         para.fValue.fVoidp = fptr;
@@ -2840,7 +2844,7 @@ bool CPyCppyy::FunctionPointerConverter::ToMemory(
     }
 
 // normal case, get a function pointer
-    void* fptr = PyFunction_AsCPointer(pyobject, fRetType, fSignature);
+    void* fptr = PyFunction_AsCPointer(pyobject, fRetType, fSignature, fAllowCppInstance);
     if (fptr) {
         SetLifeLine(ctxt, pyobject, (intptr_t)address);
         *((void**)address) = fptr;

bindings/pyroot/cppyy/CPyCppyy/src/DeclareConverters.h

@@ -408,13 +408,16 @@ class FunctionPointerConverter : public Converter {
 protected:
     std::string fRetType;
     std::string fSignature;
+    bool fAllowCppInstance = false;
 };
 
 // std::function
 class StdFunctionConverter : public FunctionPointerConverter {
 public:
     StdFunctionConverter(Converter* cnv, const std::string& ret, const std::string& sig) :
-        FunctionPointerConverter(ret, sig), fConverter(cnv) {}
+        FunctionPointerConverter(ret, sig), fConverter(cnv) {
+        fAllowCppInstance = true;
+    }
     StdFunctionConverter(const StdFunctionConverter&) = delete;
     StdFunctionConverter& operator=(const StdFunctionConverter&) = delete;
     virtual ~StdFunctionConverter() { delete fConverter; }

bindings/pyroot/cppyy/cppyy/test/test_overloads.py

@@ -405,5 +405,51 @@ class Derived : public Base {
 
         assert result_instance == result_direct
 
+
+    def test14_disallow_functor_to_function_pointer(self):
+        """Make sure we're no allowing to convert C++ functors to funciton
+        pointers, extending the C++ language in an unnatural way that can lead
+        to wrong overload resolutions."""
+
+        import cppyy
+
+        cppyy.cppdef("""
+        class Test14Functor {
+        public:
+            double operator () (double* args, double*) {
+                return 4.0 * args[0];
+            }
+        };
+
+        int test14_foo(double (*fcn)(double*, double*)) {
+            return 0;
+        }
+
+        template<class T>
+        int test14_foo(T fcn) {
+            return 1;
+        }
+
+        int test14_bar(double (*fcn)(double*, double*)) {
+            return 0;
+        }
+
+        int test14_baz(double (*fcn)(double*, double*)) {
+            return 0;
+        }
+
+        int test14_baz(std::function<double(double*, double*)> const &fcn) {
+            return 2;
+        }
+        """)
+
+        functor = cppyy.gbl.Test14Functor()
+        assert cppyy.gbl.test14_foo(functor) == 1 # should resolve to foo(T fcn)
+        # not allowed, because there is only an overload taking a function pointer
+        raises(TypeError, cppyy.gbl.test14_bar, functor)
+        # The "baz" function has a std::function overload, which should be selected
+        assert cppyy.gbl.test14_baz(functor) == 2 # should resolve to baz(std::function)
+
+
 if __name__ == "__main__":
     exit(pytest.main(args=['-sv', '-ra', __file__]))